| Message ID | 1433941292-21530-21-git-send-email-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
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@redhat.com> > --- > 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@redhat.com>"); > 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. So please check that your change does not change above behaviour. Maybe it would make sense to rename "acpi_video" variable to something better.
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@redhat.com> >> --- >> 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@redhat.com>"); >> 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. > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@redhat.com> > >>--- > >> 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@redhat.com>"); > >> 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.
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@redhat.com>"); 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)
Port the backlight selection logic to the new backlight interface selection API. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/dell-wmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)