mbox series

[v2,0/7] HID: lenovo: Mute LED handling fixes and improvements

Message ID 20210220122438.21857-1-hdegoede@redhat.com
Headers show
Series HID: lenovo: Mute LED handling fixes and improvements | expand

Message

Hans de Goede Feb. 20, 2021, 12:24 p.m. UTC
Hi All,

Here is v2 of my series with mute LED handling fixes and improvements
for the hid-lenovo driver.

This time I've added the LED folks to the Cc in case they have any input,
but there is nothing controversial in here wrt use of the LED API.

The following patches were changed or are new in version 2 of the
series, see the individual patches for detaisl:

[PATCH v2 2/7] HID: lenovo: Fix lenovo_led_set_tp10ubkbd() error handling
[PATCH v2 4/7] HID: lenovo: Remove lenovo_led_brightness_get()
[PATCH v2 5/7] HID: lenovo: Set LEDs max_brightness value

Regards,

Hans


Hans de Goede (7):
  HID: lenovo: Use brightness_set_blocking callback for setting LEDs
    brightness
  HID: lenovo: Fix lenovo_led_set_tp10ubkbd() error handling
  HID: lenovo: Check hid_get_drvdata() returns non NULL in
    lenovo_event()
  HID: lenovo: Remove lenovo_led_brightness_get()
  HID: lenovo: Set LEDs max_brightness value
  HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE
  HID: lenovo: Set default_trigger-s for the mute and micmute LEDs

 drivers/hid/hid-lenovo.c | 61 ++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 30 deletions(-)

Comments

Marek Behún Feb. 20, 2021, 4:47 p.m. UTC | #1
On Sat, 20 Feb 2021 17:34:12 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 2/20/21 4:16 PM, Marek Behun wrote:
> > On Sat, 20 Feb 2021 13:24:36 +0100
> > Hans de Goede <hdegoede@redhat.com> wrote:
> >   
> >> +	data->led_mute.max_brightness = LED_ON;  
> > 
> > These constants are obsolete since we now support variable maximum
> > brightness. Just put 1 there.  
> 
> Ok, any other remarks wrt the LED bits before I send out a v3 ?

I will review the remaining patches tonight.
Marek Behún Feb. 21, 2021, 1:26 a.m. UTC | #2
Reviewed-by: Marek Behún <kabel@kernel.org>
Marek Behún Feb. 21, 2021, 1:39 a.m. UTC | #3
Reviewed-by: Marek Behún <kabel@kernel.org>
Marek Behún Feb. 21, 2021, 1:43 a.m. UTC | #4
I would use
  ... Set default_triggers for ...
instead of
  ... Set default_trigger-s for ...
in commit title.

Reviewed-by: Marek Behún <kabel@kernel.org>
Pavel Machek Feb. 23, 2021, 8:59 a.m. UTC | #5
On Sat 2021-02-20 13:24:32, Hans de Goede wrote:
> The lenovo_led_brightness_set function may sleep, so we should have the

> the led_class_dev's brightness_set_blocking callback point to it, rather

> then the regular brightness_set callback.

> 

> When toggle through sysfs this is not a problem, but the brightness_set

> callback may be called from atomic context when using LED-triggers.

> 

> Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support")

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>


Acked-by: Pavel Machek <pavel@ucw.cz>				Pavel


-- 
http://www.livejournal.com/~pavelmachek