Re: [PATCH 20/32] dell-wmi: Port to new backlight interface selection API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday 11 June 2015 14:19:11 Hans de Goede wrote:
> Hi,
> 
> On 11-06-15 13:43, Pali Rohár wrote:
> >On Wednesday 10 June 2015 15:01:20 Hans de Goede wrote:
> >>Port the backlight selection logic to the new backlight interface
> >>selection API.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> >>---
> >>  drivers/platform/x86/dell-wmi.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> >>index 6512a06..f2d77fe 100644
> >>--- a/drivers/platform/x86/dell-wmi.c
> >>+++ b/drivers/platform/x86/dell-wmi.c
> >>@@ -35,6 +35,7 @@
> >>  #include <linux/acpi.h>
> >>  #include <linux/string.h>
> >>  #include <linux/dmi.h>
> >>+#include <acpi/video.h>
> >>
> >>  MODULE_AUTHOR("Matthew Garrett <mjg@xxxxxxxxxx>");
> >>  MODULE_DESCRIPTION("Dell laptop WMI hotkeys driver");
> >>@@ -397,7 +398,7 @@ static int __init dell_wmi_init(void)
> >>  	}
> >>
> >>  	dmi_walk(find_hk_type, NULL);
> >>-	acpi_video = acpi_video_backlight_support();
> >>+	acpi_video = acpi_video_get_backlight_type() != acpi_backlight_vendor;
> >>
> >>  	err = dell_wmi_input_setup();
> >>  	if (err)
> >
> >Hello,
> >
> >to make sure that nothing will be broken I will try to explain current
> >code. Variable acpi_video is global boolean (for this module) and when
> >is set to true it is expected that ACPI generate brightness up/down key
> >events via ACPI input device. When is acpi_video boolean variable is set
> >to false then ACPI input device should not send brightness up/down keys
> >to userspace. And dell-wmi.ko driver send those two keys via dell-wmi
> >input device.
> 
> I know.
> 
> >So please check that your change does not change above behaviour.
> 
> I've already checked this.
> 

Ok, then this patch is fine for me. Add my Acked-by or Reviewed-by.

> > Maybe it would make sense to rename "acpi_video" variable to something better.
> 
> There is another driver which has a similar construction. I believe that it
> would be good to add an acpi_video_handles_key_presses() helper to the
> acpi-video code which can be used for this instead of abusing
> acpi_video_get_backlight_type() for this. I've put this on my todo list
> to do as a further cleanup once the initial series is merged.
> 
> Regards,
> 
> Hans

Ok, thanks! acpi_video_handles_key_presses() has really better name.

-- 
Pali Rohár
pali.rohar@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux