Re: [PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]

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

 



On Thursday 27 October 2016 13:46:33 Hans de Goede wrote:
> Hi,
> 
> On 27-10-16 12:38, Pali Rohár wrote:
> >On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote:
> >>Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider
> >>events, replace dell_rbtn_notifier_register() /
> >>dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used
> >>by dell-laptop to decide whether or not to use the i8042 filter and used
> >>by dell-rbtn to auto-remove its rfkill interface when called.
> >>
> >>This results in a nice cleanup, downside is that the rfkill interface
> >>of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this
> >>now requires rmmod + insmod of dell-rbtn. But people who do not want
> >>dell-laptop for some reason will have it blacklisted anyways, so this
> >>is not an issue and there is a work-around.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> >>---
> >>Changes in v2:
> >>-This is a new patch in v2 of my platform/x86/dell-* notifier set intended
> >> to show how dell_smbios*notifier can be used to improve the dell-rbtn
> >> integration too
> >>Changes in v3:
> >>-Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the
> >> dynamic symbol dance we do to allow loading without dell-rbtn actually works.
> >>---
> >> drivers/platform/x86/Kconfig       |  1 +
> >> drivers/platform/x86/dell-laptop.c | 53 +++++++-------------------------
> >> drivers/platform/x86/dell-rbtn.c   | 63 +++++++++-----------------------------
> >> drivers/platform/x86/dell-rbtn.h   |  5 +--
> >> drivers/platform/x86/dell-smbios.h |  1 +
> >> 5 files changed, 29 insertions(+), 94 deletions(-)
> >
> >Looks like that for preventing sending event that rfkill switch was
> >changed by hardware slider we must always drop atk i8042 keycode...
> >
> >Needs to check if key is really send by both dell-rbtn and also by atk
> >i8042 keyboard driver and if yes then i8042_install_filter() is always
> >needed (if rbtn is there or not)...
> 
> But this is not related to this patch, right? This patch does not change
> any behavior. Other then your concerns about where to put the notifier,
> do you like the approach of this patch?

It is not related, but if above is truth, then another rewrite (also of
those your changes!) is needed. So maybe it would be better to postpone
(or drop this patch from your patch series) so we do not invest time for
something which will be again rewritten...

-- 
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