lkml.org 
[lkml]   [2017]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] dell-laptop: Fix keyboard led max_brightness property for Dell Latitude E6410
    On Wed, Oct 18, 2017 at 08:06:01PM +0200, Pali Rohár wrote:
    > This machine reports number of keyboard backlight led levels, instead of
    > value of the last led level index. Therefore max_brightness properly needs
    > to be subtracted by 1 to match led max_brightness API.
    >
    > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
    > Reported-by: Gabriel M. Elder <gabriel@tekgnowsys.com>
    > Link: https://bugzilla.kernel.org/show_bug.cgi?id=196913
    > ---
    > Changes since v1:
    > * Update kbd_info.levels at initialization time based on quirk

    This looks much cleaner IMO, thanks.

    > ---
    > drivers/platform/x86/dell-laptop.c | 17 +++++++++++++++++
    > 1 file changed, 17 insertions(+)
    >
    > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
    > index f42159f..3f9be8a 100644
    > --- a/drivers/platform/x86/dell-laptop.c
    > +++ b/drivers/platform/x86/dell-laptop.c
    > @@ -49,6 +49,7 @@
    >
    > struct quirk_entry {
    > u8 touchpad_led;
    > + u8 kbd_led_num_of_levels_instead_of_last_index;

    OK, I forgot to comment on this one :-)

    A name should be descriptive, but that's just too much. So let's use something
    shorter, and add a comment if one is really needed to explain what it is.
    Something like:

    u8 kbd_led_reports_num_levels is shorter, but still awfully long.

    u8 kbd_led_num_levels is shorter, but obviously looks like it should be the
    levels value instead of a bool. So the above is probably best.


    >
    > int needs_kbd_timeouts;
    > /*
    > @@ -79,6 +80,10 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
    > .kbd_timeouts = { 0, 5, 15, 60, 5 * 60, 15 * 60, -1 },
    > };
    >
    > +static struct quirk_entry quirk_dell_latitude_e6410 = {
    > + .kbd_led_num_of_levels_instead_of_last_index = 1,
    > +};
    > +
    > static struct platform_driver platform_driver = {
    > .driver = {
    > .name = "dell-laptop",
    > @@ -280,6 +285,15 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
    > },
    > .driver_data = &quirk_dell_xps13_9333,
    > },
    > + {
    > + .callback = dmi_matched,
    > + .ident = "Dell Latitude E6410",
    > + .matches = {
    > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
    > + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude E6410"),
    > + },
    > + .driver_data = &quirk_dell_latitude_e6410,
    > + },
    > { }
    > };
    >
    > @@ -1200,6 +1214,9 @@ static int kbd_get_info(struct kbd_info *info)
    > units = (buffer->output[2] >> 8) & 0xFF;
    > info->levels = (buffer->output[2] >> 16) & 0xFF;
    >
    > + if (quirks && quirks->kbd_led_num_of_levels_instead_of_last_index && info->levels)
    > + info->levels--;
    > +
    > if (units & BIT(0))
    > info->seconds = (buffer->output[3] >> 0) & 0xFF;
    > if (units & BIT(1))
    > --
    > 1.7.9.5
    >
    >

    --
    Darren Hart
    VMware Open Source Technology Center

    \
     
     \ /
      Last update: 2017-10-22 17:17    [W:5.151 / U:0.224 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site