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

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

[Darren Hart]

From: Andy Lutomirski <luto@kernel.org>

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@kernel.org>
[dvhart: New commit message and minor comment wording fixes]
Cc: Mario Limonciello <mario_limonciello@dell.com>
Cc: "Pali Rohár" <pali.rohar@gmail.com>
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
 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 },
-- 
2.9.4

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

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 3362 bytes --]

On Saturday 27 May 2017 07:16:19 Darren Hart wrote:
> From: Andy Lutomirski <luto@kernel.org>
> 
> 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@kernel.org>
> [dvhart: New commit message and minor comment wording fixes]
> Cc: Mario Limonciello <mario_limonciello@dell.com>
> Cc: "Pali Rohár" <pali.rohar@gmail.com>
> Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
> ---
>  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.

Currently from comment it is hard for non platform/x86/dell people to 
decide if problem is in userspace or kernel/acpi. It could be understood 
as two kernel drivers "dell-rbtn" and "intel-hid" are fighting, but also 
as one particular software (network manager) is unable to handle state 
with more then on driver.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Andy Lutomirski]

On Sat, May 27, 2017 at 4:01 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Saturday 27 May 2017 07:16:19 Darren Hart wrote:
>> From: Andy Lutomirski <luto@kernel.org>
>>
>> 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@kernel.org>
>> [dvhart: New commit message and minor comment wording fixes]
>> Cc: Mario Limonciello <mario_limonciello@dell.com>
>> Cc: "Pali Rohár" <pali.rohar@gmail.com>
>> Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
>> ---
>>  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?).

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

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 4199 bytes --]

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@gmail.com>
> wrote:
> > On Saturday 27 May 2017 07:16:19 Darren Hart wrote:
> >> From: Andy Lutomirski <luto@kernel.org>
> >> 
> >> 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@kernel.org>
> >> [dvhart: New commit message and minor comment wording fixes]
> >> Cc: Mario Limonciello <mario_limonciello@dell.com>
> >> Cc: "Pali Rohár" <pali.rohar@gmail.com>
> >> Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
> >> ---
> >> 
> >>  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@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Andy Shevchenko]

On Sat, May 27, 2017 at 8:16 AM, Darren Hart <dvhart@infradead.org> wrote:
> From: Andy Lutomirski <luto@kernel.org>
>
> 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.
>

Pali made a good point.
Otherwise, FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> [dvhart: New commit message and minor comment wording fixes]
> Cc: Mario Limonciello <mario_limonciello@dell.com>
> Cc: "Pali Rohár" <pali.rohar@gmail.com>
> Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
> ---
>  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 },
> --
> 2.9.4
>



-- 
With Best Regards,
Andy Shevchenko

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

[Darren Hart]

On Sat, May 27, 2017 at 07:27:19PM +0300, Andy Shevchenko wrote:
> On Sat, May 27, 2017 at 8:16 AM, Darren Hart <dvhart@infradead.org> wrote:
> > From: Andy Lutomirski <luto@kernel.org>
> >
> > 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.
> >
> 
> Pali made a good point.
> Otherwise, FWIW:
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Agreed, updated to remove specific mention of NetworkManager.

Thanks.

-- 
Darren Hart
VMware Open Source Technology Center