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]

 



Hi,

On 27-10-16 13:59, Pali Rohár wrote:
> 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...

Ok, I'm fine with delaying this cleanup till your questions are answered.

Maybe we should then also move ahead with the notifier in dell-smbios,
because without this patch it is only used by dell-wmi and dell-laptop
both of which already depend on dell-smbios.

Regards,

Hans

------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive. 
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel




[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux