On Tue, Jun 12, 2018 at 12:49:35PM -0600, Azael Avalos wrote:
> Second generation keyboard backlight (type 2) laptops can switch
> on the keyboard LED on their own via hardware/firmware, but the
> LED subsystem is unaware of such change since the LED interface
> was only beign created on first generation keyboard backlight
> (type 1) laptops.
>
> This patch creates the LED interface for second gen keyboards
> and calls the *_hw_changed API whenever userspace changes the
> state of the keyboard backlight LED.
>
> Signed-off-by: Azael Avalos <coproscefalo@xxxxxxxxx>
> ---
> drivers/platform/x86/toshiba_acpi.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index eef76bfa5d73..0b645b19865f 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -1836,6 +1836,7 @@ static ssize_t kbd_backlight_mode_store(struct device *dev,
> return ret;
>
> toshiba->kbd_mode = mode;
> + toshiba_acpi->kbd_mode = mode;
I found myself confused within the driver regarding changes to toshiba,
toshiba_acpi, and dev.
We have the global toshiba_acpi, and we sometimes pass around the device
and use the function argument.
But, what is going on in this case when we have two different devices?
Are they different?
...
> @@ -3237,11 +3248,16 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
> pr_info("SATA power event received %x\n", event);
> break;
> case 0x92: /* Keyboard backlight mode changed */
> - toshiba_acpi->kbd_event_generated = true;
> + dev->kbd_event_generated = true;
This is an unrelated cleanup right? Just using the passed in argument
instead of the global? No functional change? Should mention this in the
commit message if so.
If I'm confused about what these two devices do, then some comments in
the driver would be appropriate.
--
Darren Hart
VMware Open Source Technology Center
