On Wed, Jun 13, 2018 at 10:28:39AM -0600, Azael Avalos wrote: > Hi Darren, > El mar., 12 de jun. de 2018 a la(s) 17:09, Darren Hart > (dvhart@xxxxxxxxxxxxx) escribió: > > > > 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? > > They're supposed to be identical, however, there are some variables that > are not being updated, and in this case "kbd_mode" is only updated in the > main toshiba struct and not in the global and we need it updated to access > it on the kbd backlight work queue. > > My thought is that the global struct was added as a convenience to > access the contents where we cannot pass the main struct as an > argument. > > I can send another patch with a variable name cleanup, so we only end up > using either dev or toshiba as the name. This sounds fragile. If for convenience, wouldn't a pointer to the same data suffice, rather than attempting to keep to copies in sync? -- Darren Hart VMware Open Source Technology Center
