On Friday 11 November 2016 15:33:48 Hans de Goede wrote:
> Hi,
>
> On 11-11-16 15:12, Pali Rohár wrote:
> > On Tuesday 01 November 2016 14:37:46 Hans de Goede wrote:
> >> Make thinkpad_acpi call led_notify_brightness_change on the
> >> kbd_led led_classdev registered by thinkpad_acpi when the kbd
> >> backlight brightness changes.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> >> ---
> >> Changes in v3:
> >> -This is a new patch in v3 of this patch-set
> >> Changes in v4:
> >> -No Changes
> >> ---
> >>
> >> drivers/platform/x86/thinkpad_acpi.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> >> b/drivers/platform/x86/thinkpad_acpi.c index b65ce75..5dcd7d8b
> >> 100644
> >> --- a/drivers/platform/x86/thinkpad_acpi.c
> >> +++ b/drivers/platform/x86/thinkpad_acpi.c
> >> @@ -162,6 +162,7 @@ enum tpacpi_hkey_event_t {
> >>
> >> TP_HKEY_EV_HOTKEY_BASE = 0x1001, /* first hotkey (FN+F1) */
> >> TP_HKEY_EV_BRGHT_UP = 0x1010, /* Brightness up */
> >> TP_HKEY_EV_BRGHT_DOWN = 0x1011, /* Brightness down */
> >>
> >> + TP_HKEY_EV_THINKLIGHT = 0x1012, /* Thinklight/kbd backlight
*/
> >
> > My question remains. Is this for Thinklight or keyboard backlight?
> > Because Thinklinght has led device "tpacpi_led_thinklight" and
> > keyboard backlight has led device "tpacpi_led_kbdlight".
>
> I would say both, this matches with the pre-existing
> TP_ACPI_HKEY_THNKLGHT_MASK (they have a 1:1 mapping),
> keep in mind that there are no thinkpads with both
> a thinklight and a backlit keyboard
That is not truth. X230 is in variant with both Thinklight (light above
display) and keyboard backlight. Plus in BIOS it is possible to
configure if you want to enable only Thinkligh, only keyboard backlight
or both. Probably T430 and other 30s models can be found with both. But
I do not own those modules...
> as those both
> serve the same purpose so it looks like the re-used
> the scancode.
So I could guess that it reports event when one of those leds are
changed...
> I've tried this code on both a W541 and a P50 and both
> emit TP_ACPI_HOTKEYSCAN_FNPAGEUP for the backlight
> keyboard hotkey (fn + space), I'm naming the event for
> this TP_HKEY_EV_THINKLIGHT for consistency with the
> existing TP_ACPI_HKEY_THNKLGHT_MASK which corresponds
> to TP_ACPI_HOTKEYSCAN_FNPAGEUP.
>
> >> TP_HKEY_EV_VOL_UP = 0x1015, /* Volume up or unmute */
> >> TP_HKEY_EV_VOL_DOWN = 0x1016, /* Volume down or unmute */
> >> TP_HKEY_EV_VOL_MUTE = 0x1017, /* Mixer output mute */
> >>
> >> @@ -5167,6 +5168,8 @@ static int __init kbdlight_init(struct
> >> ibm_init_struct *iibm) return rc;
> >>
> >> }
> >>
> >> + tpacpi_hotkey_driver_mask_set(hotkey_driver_mask |
> >> + TP_ACPI_HKEY_THNKLGHT_MASK);
> >>
> >> return 0;
> >>
> >> }
> >>
> >> @@ -9114,6 +9117,8 @@ static void tpacpi_driver_event(const
> >> unsigned int hkey_event) volume_alsa_notify_change();
> >>
> >> }
> >>
> >> }
> >>
> >> + if (tp_features.kbdlight && hkey_event == TP_HKEY_EV_THINKLIGHT)
> >> +
> >> led_notify_brightness_change(&tpacpi_led_kbdlight.led_classdev)
> >> ;
> >
> > This looks incorrect. You are trying to inform tpacpi_led_kbdlight
> > when tpacpi_led_thinklight change led status?
>
> No as said the scancode / event is shared, notice the check for
> tp_features.kbdlight, so the notify will only happen on laptops which
> actually have a backlit keyboard.
If that event is send when either Thinklight or keyb-backlight is
changed, should not we inform both led devices about level change? Do
you have a chance to test it on notebook with Thinklight?
And funny question is what would happen on those machines which have
both Thinklight and keyboard backlight?
Anyway, I would suggest different name, not TP_HKEY_EV_THINKLIGHT as it
is not thinklight-only and could be misleading in keyboard backlight
context...
> Regards,
>
> Hans
>
> >> }
> >>
> >> static void hotkey_driver_event(const unsigned int scancode)
--
Pali Rohár
pali.rohar@xxxxxxxxx
Attachment:
signature.asc
Description: This is a digitally signed message part.
------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi
_______________________________________________ ibm-acpi-devel mailing list ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
