linux-kernel - Re: [PATCH] platform/chrome: Add Wilco EC keyboard backlight LEDs support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20190311215224.GA11869@amd>
Date:   Mon, 11 Mar 2019 22:52:24 +0100
From:   Pavel Machek <pavel@....cz>
To:     Nick Crews <ncrews@...omium.org>
Cc:     kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/chrome: Add Wilco EC keyboard backlight LEDs
 support

On Mon 2019-03-11 09:36:20, Nick Crews wrote:
> On Fri, Mar 8, 2019 at 3:13 PM Pavel Machek <pavel@....cz> wrote:
> >
> > Hi!
> >
> > > > This patch is meant to be applied on top of the for-next
> > > > branch of the platform/chrome repository, as it uses some of
> > > > the code staged there.
> > > >
> > > > The EC is in charge of controlling the keyboard backlight on
> > > > the Wilco platform. We expose a standard LED class device at
> > > > /sys/class/leds/wilco::kbd_backlight. This driver is modeled
> > > > after the standard Chrome OS keyboard backlight driver at
> > > > drivers/platform/chrome/cros_kbd_led_backlight.c
> > > >
> > > > Some Wilco devices do not support a keyboard backlight. This
> > > > is checked in probe(), and in this case the sysfs entry will
> > > > not appear, and everything will behave normally.
> > >
> > > It would be even better if we did not register platform device if EC
> > > does not support backlight.
> 
> Good point Dmitry. That would require making the core driver
> dependent upon this keyboard backlight driver, though. Do you
> think that added complexity is worth it? I don't see performance
> concerns with adding one unused platform device, so is your rationale
> that it would just be cleaner code?
> 

You probably wanted to mail dmitry. You should have probably cc-ed
Dmitry and the lists.

> > > > +       data->led.brightness_set = keyboard_led_set_brightness;
> > > > +       data->led.brightness_get = keyboard_led_get_brightness;
> > >
> > > wilco_ec_mailbox() may sleep, so you need to assign it to
> > > led.brightness_set_blocking.
> >
> > Hmm. Seeing get method there... can the EC change the brightness
> > without command from kernel?
> 
> No, the EC won't change the brightness without getting told to by
> the kernel. Do you think that means that we could get away with just
> caching the brightness from the previous set_brightness(), and not
> actually sending a mailbox) command to the EC on get_brightness()?

Something like that. Even better ... don't implement brightness_get at
all, and check that core does the right thing checking it.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)