linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6
@ 2017-05-27  5:16 Darren Hart
  2017-05-27 11:01 ` Pali Rohár
  2017-05-27 16:27 ` Andy Shevchenko
  0 siblings, 2 replies; 6+ messages in thread
From: Darren Hart @ 2017-05-27  5:16 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: LKML, Andy Lutomirski, Mario Limonciello, Pali Rohár, 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6
  2017-05-27  5:16 [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6 Darren Hart
@ 2017-05-27 11:01 ` Pali Rohár
  2017-05-27 16:07   ` Andy Lutomirski
  2017-05-27 16:27 ` Andy Shevchenko
  1 sibling, 1 reply; 6+ messages in thread
From: Pali Rohár @ 2017-05-27 11:01 UTC (permalink / raw)
  To: Darren Hart; +Cc: platform-driver-x86, LKML, Andy Lutomirski, Mario Limonciello

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6
  2017-05-27 11:01 ` Pali Rohár
@ 2017-05-27 16:07   ` Andy Lutomirski
  2017-05-27 16:38     ` Pali Rohár
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Lutomirski @ 2017-05-27 16:07 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Darren Hart, platform-driver-x86, LKML, Andy Lutomirski,
	Mario Limonciello

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?).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6
  2017-05-27  5:16 [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6 Darren Hart
  2017-05-27 11:01 ` Pali Rohár
@ 2017-05-27 16:27 ` Andy Shevchenko
  2017-06-03 19:25   ` Darren Hart
  1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2017-05-27 16:27 UTC (permalink / raw)
  To: Darren Hart
  Cc: Platform Driver, LKML, Andy Lutomirski, Mario Limonciello,
	Pali Rohár

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6
  2017-05-27 16:07   ` Andy Lutomirski
@ 2017-05-27 16:38     ` Pali Rohár
  0 siblings, 0 replies; 6+ messages in thread
From: Pali Rohár @ 2017-05-27 16:38 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Darren Hart, platform-driver-x86, LKML, Mario Limonciello

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6
  2017-05-27 16:27 ` Andy Shevchenko
@ 2017-06-03 19:25   ` Darren Hart
  0 siblings, 0 replies; 6+ messages in thread
From: Darren Hart @ 2017-06-03 19:25 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Platform Driver, LKML, Andy Lutomirski, Mario Limonciello,
	Pali Rohár

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-06-03 19:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-27  5:16 [PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6 Darren Hart
2017-05-27 11:01 ` Pali Rohár
2017-05-27 16:07   ` Andy Lutomirski
2017-05-27 16:38     ` Pali Rohár
2017-05-27 16:27 ` Andy Shevchenko
2017-06-03 19:25   ` Darren Hart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).