Re: [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6

[Pali RohÃr]

On Saturday 27 May 2017 18:07:14 Andy Lutomirski wrote:
> On Sat, May 27, 2017 at 4:01 AM, Pali RohÃr <pali.rohar@xxxxxxxxx>
> wrote:
> > On Saturday 27 May 2017 07:16:19 Darren Hart wrote:
> >> From: Andy Lutomirski <luto@xxxxxxxxxx>
> >> 
> >> According to Mario at Dell, the DELLABC6 device should not be used
> >> on a Linux system. It also conflicts with Intel-HID and its
> >> interactions with Network Manager. Document that we are aware of
> >> the device, but that we are intentionally ignoring it.
> >> 
> >> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
> >> [dvhart: New commit message and minor comment wording fixes]
> >> Cc: Mario Limonciello <mario_limonciello@xxxxxxxx>
> >> Cc: "Pali RohÃr" <pali.rohar@xxxxxxxxx>
> >> Signed-off-by: Darren Hart (VMware) <dvhart@xxxxxxxxxxxxx>
> >> ---
> >> 
> >>  drivers/platform/x86/dell-rbtn.c | 26 +++++++++++++++++++-------
> >>  1 file changed, 19 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/drivers/platform/x86/dell-rbtn.c
> >> b/drivers/platform/x86/dell-rbtn.c index dcd9f40..2eeef03 100644
> >> --- a/drivers/platform/x86/dell-rbtn.c
> >> +++ b/drivers/platform/x86/dell-rbtn.c
> >> @@ -223,14 +223,26 @@ static const struct acpi_device_id
> >> rbtn_ids[] = { * This driver can also handle the "DELLABC6"
> >> device that
> >> 
> >>        * appears on the XPS 13 9350, but that device is disabled
> >>        * by the DSDT unless booted with acpi_osi="!Windows 2012"
> >> 
> >> -      * acpi_osi="!Windows 2013".  Even if we boot that and bind
> >> -      * the driver, we seem to have inconsistent behavior in
> >> -      * which NetworkManager can get out of sync with the rfkill
> >> -      * state.
> >> +      * acpi_osi="!Windows 2013".
> >> 
> >>        *
> >> 
> >> -      * On the XPS 13 9350 and similar laptops, we're not
> >> supposed to -      * use DELLABC6 at all.  Instead, we handle the
> >> rfkill button -      * via the intel-hid driver.
> >> +      * According to Mario at Dell:
> >> +      *
> >> +      *  DELLABC6 is a custom interface that was created solely
> >> to +      *  have airplane mode support for Windows 7.  For
> >> Windows 10 +      *  the proper interface is to use that which is
> >> handled by +      *  intel-hid.  A OEM airplane mode driver is
> >> not used. +      *
> >> +      *  Since the kernel doesn't identify as Windows 7 it would
> >> be +      *  incorrect to do attempt to use that interface.
> >> +      *
> >> +      * Even if we override _OSI and bind to DELLABC6, we end up
> >> +      * with inconsistent behavior in which NetworkManager can
> >> get +      * out of sync with the rfkill state.  This happens
> >> because +      * NetworkManager receives events from intel-hid
> >> and fights with +      * dell-rbtn for control.
> >> +      *
> >> +      * The upshot is that it's better to just ignore DELLABC6
> >> +      * devices.
> >> 
> >>        */
> >>       
> >>       { "", 0 },
> > 
> > Just one note: Kernel code should not depend on one particular
> > software which implements networking (in userspace). Either
> > behaviour is independent of used software and therefore comment
> > does not apply only to Network Manager OR behaviour is strictly
> > bounded to Network Manager which is IMHO not a kernel bug, but
> > rather userspace software application bug. If there is a bug in
> > userspace, then userspace should be fixed instead of adding
> > hacks/workarounds in kernel.
> 
> Fair enough.  NetworkManager is just an example here.  The general
> kernel behavior is that, if the kernel sends KEY_RFKILL or similar,
> that means "the button was pressed and it's up to userspace to handle
> it".  Sending KEY_RFKILL *and* handling it in the kernel is not going
> to go well.  This should be true with any other reasonably modern
> userspace (connmgr or whatever it's called, perhaps?).

Agree, we already had a discussion that KEY_RFKILL is sent only when 
kernel/firmware does not change internal hardware state. Internal 
hardware change is notified to userspace via /dev/rfkill and not via 
input subsystem.

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.