FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 01-15-2009, 01:51 PM
Stefan Bader
 
Default panasonic-laptop: add Panasonic Let's Note laptop extras driver v0.94

Andy Whitcroft wrote:
> On Mon, Jan 05, 2009 at 03:31:11PM +0100, Stefan Bader wrote:
>> Since it is well isolated to a single driver, ACK. Two small notes, see below)
>>
>> Andy Whitcroft wrote:
>>> Bug: #222324
>>> commit 709ee531c153038d81b30649b9eeed3c44a4d5cc
>>> This is a driver for ACPI extras such as hotkeys and backlight
>>> brightness control on various Panasonic "Let's Note" series laptop
>>> computers.
>>>
>>> It exports the backlight via the backlight class device API,
>>> and the hotkeys as input event device. Some more esoteric
>>> items like number of installed batteries are exported via sysfs
>>> device attributes.
>>>
>>> Hotkey events also generate old-style ACPI enents through
>>> /proc/acpi/event to interoperate with current versions of acpid.
>>>
>>> --- a/drivers/misc/Kconfig
>>> +++ b/drivers/misc/Kconfig
>>> @@ -245,6 +245,17 @@ config MSI_LAPTOP
>>> If you have an MSI S270 laptop, say Y or M here.
>>> +config PANASONIC_LAPTOP
>>> + tristate "Panasonic Laptop Extras"
>>> + depends on X86 && INPUT
>> Add ACPI here (see commit 7ba427c2363d91d4221843a9ae601f90f8d928b9)
>
> Added.
>
>> In the driver one minor
>>> +
>>> +static int bl_set_status(struct backlight_device *bd)
>>> +{
>>> + struct pcc_acpi *pcc = bl_get_data(bd);
>>> + int bright = bd->props.brightness;
>>> + int rc;
>>> +
>>> + if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf))
>>> + return -EIO;
>>> +
>>> + if (bright < pcc->sinf[SINF_AC_MIN_BRIGHT])
>>> + bright = pcc->sinf[SINF_AC_MIN_BRIGHT];
>>> +
>>> + if (bright < pcc->sinf[SINF_DC_MIN_BRIGHT])
>>> + bright = pcc->sinf[SINF_DC_MIN_BRIGHT];
>>> +
>>> + if (bright < pcc->sinf[SINF_AC_MIN_BRIGHT] ||
>>> + bright > pcc->sinf[SINF_AC_MAX_BRIGHT])
>>> + return -EINVAL;
>>> +
>> Isn't bright always at least pcc->sinf[SINF_AC_MIN_BRIGHT] as it is set
>> above? Might this be meant to check for > SINF_DC_MAX_BRIGHT?
>
> Yeah, it does seems pointless, though it might just represent belt and
> braces checks. I have checked with the upstream version and it has not
> changed as yet. I will report it to the maintainer, but propose to

> leave it as it is for the moment.

Ok, agreed.


> -apw


--

When all other means of communication fail, try words!



--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 01-15-2009, 05:30 PM
Tim Gardner
 
Default panasonic-laptop: add Panasonic Let's Note laptop extras driver v0.94

Andy Whitcroft wrote:
> On Mon, Jan 05, 2009 at 03:31:11PM +0100, Stefan Bader wrote:
>> Since it is well isolated to a single driver, ACK. Two small notes, see below)
>>
>> Andy Whitcroft wrote:
>>> Bug: #222324
>>> commit 709ee531c153038d81b30649b9eeed3c44a4d5cc
>>> This is a driver for ACPI extras such as hotkeys and backlight
>>> brightness control on various Panasonic "Let's Note" series laptop
>>> computers.
>>>
>>> It exports the backlight via the backlight class device API,
>>> and the hotkeys as input event device. Some more esoteric
>>> items like number of installed batteries are exported via sysfs
>>> device attributes.
>>>
>>> Hotkey events also generate old-style ACPI enents through
>>> /proc/acpi/event to interoperate with current versions of acpid.
>>>
>>> --- a/drivers/misc/Kconfig
>>> +++ b/drivers/misc/Kconfig
>>> @@ -245,6 +245,17 @@ config MSI_LAPTOP
>>> If you have an MSI S270 laptop, say Y or M here.
>>> +config PANASONIC_LAPTOP
>>> + tristate "Panasonic Laptop Extras"
>>> + depends on X86 && INPUT
>> Add ACPI here (see commit 7ba427c2363d91d4221843a9ae601f90f8d928b9)
>
> Added.
>
>> In the driver one minor
>>> +
>>> +static int bl_set_status(struct backlight_device *bd)
>>> +{
>>> + struct pcc_acpi *pcc = bl_get_data(bd);
>>> + int bright = bd->props.brightness;
>>> + int rc;
>>> +
>>> + if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf))
>>> + return -EIO;
>>> +
>>> + if (bright < pcc->sinf[SINF_AC_MIN_BRIGHT])
>>> + bright = pcc->sinf[SINF_AC_MIN_BRIGHT];
>>> +
>>> + if (bright < pcc->sinf[SINF_DC_MIN_BRIGHT])
>>> + bright = pcc->sinf[SINF_DC_MIN_BRIGHT];
>>> +
>>> + if (bright < pcc->sinf[SINF_AC_MIN_BRIGHT] ||
>>> + bright > pcc->sinf[SINF_AC_MAX_BRIGHT])
>>> + return -EINVAL;
>>> +
>> Isn't bright always at least pcc->sinf[SINF_AC_MIN_BRIGHT] as it is set
>> above? Might this be meant to check for > SINF_DC_MAX_BRIGHT?
>
> Yeah, it does seems pointless, though it might just represent belt and
> braces checks. I have checked with the upstream version and it has not
> changed as yet. I will report it to the maintainer, but propose to
> leave it as it is for the moment.
>
> -apw
>

ACK for both patches

--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 06:39 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org