[PATCH] dell-laptop: Add hwswitch_only module parameter

[PATCH] dell-laptop: Add hwswitch_only module parameter

[Keng-Yu Lin]

Some BIOS does not report the correct state of the wireless killswitch
in the return value of SMI Class 17, Select 11.

This causes the killswitch to always stay as soft-blocked once the rfkill
hotkey is pressed.

This patch adds a module parameter to work around this by disregarding
the software rfkill and using only Bit 16 in the return value of
SMI Class 17, Select 11 as the hardware killswitch.

Signed-off-by: Keng-Yu Lin <keng-yu.lin@canonical.com>
---
 drivers/platform/x86/dell-laptop.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

 This patch is intended to work around the buggy BIOS.

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 4413975..a105d7e 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -74,6 +74,8 @@ static struct rfkill *wifi_rfkill;
 static struct rfkill *bluetooth_rfkill;
 static struct rfkill *wwan_rfkill;
 
+static bool hwswitch_only;
+
 static const struct dmi_system_id __initdata dell_device_table[] = {
 	{
 		.ident = "Dell laptop",
@@ -314,7 +316,8 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	status = buffer->output[1];
 	release_buffer();
 
-	rfkill_set_sw_state(rfkill, !!(status & BIT(bit)));
+	if (!hwswitch_only)
+		rfkill_set_sw_state(rfkill, !!(status & BIT(bit)));
 
 	if (hwswitch_state & (BIT(hwswitch_bit)))
 		rfkill_set_hw_state(rfkill, !(status & BIT(16)));
@@ -630,6 +633,9 @@ static void __exit dell_exit(void)
 module_init(dell_init);
 module_exit(dell_exit);
 
+module_param(hwswitch_only, bool, 0644);
+MODULE_PARM_DESC(hwswitch_only, "Use the hardware rfkill only");
+
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
 MODULE_DESCRIPTION("Dell laptop driver");
 MODULE_LICENSE("GPL");
-- 
1.7.1

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Matthew Garrett]

On Thu, Oct 07, 2010 at 11:30:07AM +0800, Keng-Yu Lin wrote:
> Some BIOS does not report the correct state of the wireless killswitch
> in the return value of SMI Class 17, Select 11.
> 
> This causes the killswitch to always stay as soft-blocked once the rfkill
> hotkey is pressed.
> 
> This patch adds a module parameter to work around this by disregarding
> the software rfkill and using only Bit 16 in the return value of
> SMI Class 17, Select 11 as the hardware killswitch.

I don't like "Fix my hardware" options. Is there any way we can identify 
this failure case?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Matthew Garrett]

On Thu, Oct 07, 2010 at 08:30:57AM -0500, Mario Limonciello wrote:

>    In this specific scenario that Ken Yu submitted this patch for, the
>    firmware will return the same return codes for the SMI's as a functional
>    machine will.  Machines affected by this will fail to recover after rfkill
>    is toggled twice.  The symptom will be that the soft block always appears
>    set to the same setting with continued keypresses.
>    During the prototype phase this option is quite useful to help debug the
>    problem it addresses.

Is the kernel able to unblock it under those circumstances?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Mario Limonciello]

Hi Matthew:

On Thu, Oct 7, 2010 at 08:35, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>
> On Thu, Oct 07, 2010 at 08:30:57AM -0500, Mario Limonciello wrote:
>
> >    In this specific scenario that Ken Yu submitted this patch for, the
> >    firmware will return the same return codes for the SMI's as a functional
> >    machine will.  Machines affected by this will fail to recover after rfkill
> >    is toggled twice.  The symptom will be that the soft block always appears
> >    set to the same setting with continued keypresses.
> >    During the prototype phase this option is quite useful to help debug the
> >    problem it addresses.
>
> Is the kernel able to unblock it under those circumstances?

Manually running rfkill unblock will unblock it in this broken
firmware scenario in question.

--
Mario Limonciello
superm1@gmail.com

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Matthew Garrett]

On Thu, Oct 07, 2010 at 03:49:05PM -0500, Mario Limonciello wrote:
> Hi Matthew:
> > Is the kernel able to unblock it under those circumstances?
> 
> Manually running rfkill unblock will unblock it in this broken
> firmware scenario in question.

So the issue is in the firmware's response to the keystroke? Ok. I'd 
rather have a DMI list of the broken machines than a module parameter.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Mario Limonciello]

Hi Matthew:

On Thu, Oct 7, 2010 at 15:50, Matthew Garrett <mjg@redhat.com> wrote:
> On Thu, Oct 07, 2010 at 03:49:05PM -0500, Mario Limonciello wrote:
>> Hi Matthew:
>> > Is the kernel able to unblock it under those circumstances?
>>
>> Manually running rfkill unblock will unblock it in this broken
>> firmware scenario in question.
>
> So the issue is in the firmware's response to the keystroke? Ok. I'd
> rather have a DMI list of the broken machines than a module parameter.

Yes the issue is the firmware's response to the keystroke.  The
intention here is to individual submit machines in future patches as
they're discovered.  The parameter's primary purpose is to assist in
building the list.  If someone reports a bug with these symptoms, they
can add the parameter to their kernel command line, and report if it
helps them.  If it helps, their machine can be added to the DMI table.

-- 
Mario Limonciello
superm1@gmail.com

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Matthew Garrett]

On Thu, Oct 07, 2010 at 03:53:37PM -0500, Mario Limonciello wrote:

> Yes the issue is the firmware's response to the keystroke.  The
> intention here is to individual submit machines in future patches as
> they're discovered.  The parameter's primary purpose is to assist in
> building the list.  If someone reports a bug with these symptoms, they
> can add the parameter to their kernel command line, and report if it
> helps them.  If it helps, their machine can be added to the DMI table.

Sorry, I'm not adding this as a module option. How can you not know 
which machines are affected?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Dmitry Torokhov]

On Thu, Oct 07, 2010 at 03:53:37PM -0500, Mario Limonciello wrote:
> Hi Matthew:
> 
> On Thu, Oct 7, 2010 at 15:50, Matthew Garrett <mjg@redhat.com> wrote:
> > On Thu, Oct 07, 2010 at 03:49:05PM -0500, Mario Limonciello wrote:
> >> Hi Matthew:
> >> > Is the kernel able to unblock it under those circumstances?
> >>
> >> Manually running rfkill unblock will unblock it in this broken
> >> firmware scenario in question.
> >
> > So the issue is in the firmware's response to the keystroke? Ok. I'd
> > rather have a DMI list of the broken machines than a module parameter.
> 
> Yes the issue is the firmware's response to the keystroke.  The
> intention here is to individual submit machines in future patches as
> they're discovered.  The parameter's primary purpose is to assist in
> building the list.  If someone reports a bug with these symptoms, they
> can add the parameter to their kernel command line, and report if it
> helps them.  If it helps, their machine can be added to the DMI table.
> 

Mario, Matthew,

Since the fix is not essential for boot purposes can we keep module
parameter only (without adding DMI entries) and push the task of
properly setting the module parmeter to udev?

We have this strategy for bunch of input stuff (force release, keymap)
and I think it works better than dding more and more DMI quirks into
kernel itself.

Thanks.

-- 
Dmitry

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Mario Limonciello]

Hi Dmitry:

On Thu, Oct 7, 2010 at 16:30, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Thu, Oct 07, 2010 at 03:53:37PM -0500, Mario Limonciello wrote:
>> Hi Matthew:
>>
>> On Thu, Oct 7, 2010 at 15:50, Matthew Garrett <mjg@redhat.com> wrote:
>> > On Thu, Oct 07, 2010 at 03:49:05PM -0500, Mario Limonciello wrote:
>> >> Hi Matthew:
>> >> > Is the kernel able to unblock it under those circumstances?
>> >>
>> >> Manually running rfkill unblock will unblock it in this broken
>> >> firmware scenario in question.
>> >
>> > So the issue is in the firmware's response to the keystroke? Ok. I'd
>> > rather have a DMI list of the broken machines than a module parameter.
>>
>> Yes the issue is the firmware's response to the keystroke.  The
>> intention here is to individual submit machines in future patches as
>> they're discovered.  The parameter's primary purpose is to assist in
>> building the list.  If someone reports a bug with these symptoms, they
>> can add the parameter to their kernel command line, and report if it
>> helps them.  If it helps, their machine can be added to the DMI table.
>>
>
> Mario, Matthew,
>
> Since the fix is not essential for boot purposes can we keep module
> parameter only (without adding DMI entries) and push the task of
> properly setting the module parmeter to udev?
>
> We have this strategy for bunch of input stuff (force release, keymap)
> and I think it works better than dding more and more DMI quirks into
> kernel itself.
>
> Thanks.
>
> --
> Dmitry
>

I think this is a fair alternative.  A majority of the machines will
work properly as is - no parameter, and the few in between that don't
can have entries added to udev then.  It's even easier for users to
diagnose and submit feedback when they're modifying a udev conffile.

-- 
Mario Limonciello
superm1@gmail.com

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Matthew Garrett]

On Thu, Oct 07, 2010 at 02:30:28PM -0700, Dmitry Torokhov wrote:

> We have this strategy for bunch of input stuff (force release, keymap)
> and I think it works better than dding more and more DMI quirks into
> kernel itself.

It's limited to Dell hardware, so I think keeping a static list in the 
Dell laptop driver is reasonable. All we need is a list of hardware, and 
I'd really hope that Dell know which BIOS versions contain this code!

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Mario Limonciello]

Matthew:

On Thu, Oct 7, 2010 at 16:37, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, Oct 07, 2010 at 02:30:28PM -0700, Dmitry Torokhov wrote:
>
>> We have this strategy for bunch of input stuff (force release, keymap)
>> and I think it works better than dding more and more DMI quirks into
>> kernel itself.
>
> It's limited to Dell hardware, so I think keeping a static list in the
> Dell laptop driver is reasonable. All we need is a list of hardware, and
> I'd really hope that Dell know which BIOS versions contain this code!
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>

All Dell laptops are /supposed/ to adhere to the specification in
question.  Unfortunately, not all machines are tested with Linux
during their development and on the OS they ship with that
functionality isn't always used in that specific way for rfkill.


-- 
Mario Limonciello
superm1@gmail.com

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Matthew Garrett]

On Thu, Oct 07, 2010 at 04:41:12PM -0500, Mario Limonciello wrote:

> All Dell laptops are /supposed/ to adhere to the specification in
> question.  Unfortunately, not all machines are tested with Linux
> during their development and on the OS they ship with that
> functionality isn't always used in that specific way for rfkill.

I don't understand this. You know which code is broken - surely you're 
able to determine which products shipped with BIOSes derived from the 
broken code?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Mario Limonciello]

Matthew:

On Thu, Oct 7, 2010 at 16:42, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, Oct 07, 2010 at 04:41:12PM -0500, Mario Limonciello wrote:
>
>> All Dell laptops are /supposed/ to adhere to the specification in
>> question.  Unfortunately, not all machines are tested with Linux
>> during their development and on the OS they ship with that
>> functionality isn't always used in that specific way for rfkill.
>
> I don't understand this. You know which code is broken - surely you're
> able to determine which products shipped with BIOSes derived from the
> broken code?
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>

Not all product's BIOS are developed in-house.  The ones that are, yes
it quite possible to identify.  Matter of fact, this issue tends to
not exist on the in-house developed BIOS codebase.


-- 
Mario Limonciello
superm1@gmail.com

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Dmitry Torokhov]

On Thu, Oct 07, 2010 at 10:37:54PM +0100, Matthew Garrett wrote:
> On Thu, Oct 07, 2010 at 02:30:28PM -0700, Dmitry Torokhov wrote:
> 
> > We have this strategy for bunch of input stuff (force release, keymap)
> > and I think it works better than dding more and more DMI quirks into
> > kernel itself.
> 
> It's limited to Dell hardware, so I think keeping a static list in the 
> Dell laptop driver is reasonable. All we need is a list of hardware, and 
> I'd really hope that Dell know which BIOS versions contain this code!
> 

Unfortunately this kind of crap tends to flow from one BIOS version to
another.. How many DMI entries do we know about so far? Did Dell issue
firmware updates correcting the issue so new boxes will not exhibit the
problem?

-- 
Dmitry

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Matthew Garrett]

On Thu, Oct 07, 2010 at 04:45:22PM -0500, Mario Limonciello wrote:
> Matthew:
> 
> On Thu, Oct 7, 2010 at 16:42, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > I don't understand this. You know which code is broken - surely you're
> > able to determine which products shipped with BIOSes derived from the
> > broken code?
> 
> Not all product's BIOS are developed in-house.  The ones that are, yes
> it quite possible to identify.  Matter of fact, this issue tends to
> not exist on the in-house developed BIOS codebase.

Ok, so let's approach this differently. What is it that results in this 
issue not affecting Windows?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Mario Limonciello]

Hi Dmitry:

On Thu, Oct 7, 2010 at 16:46, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Thu, Oct 07, 2010 at 10:37:54PM +0100, Matthew Garrett wrote:
>> On Thu, Oct 07, 2010 at 02:30:28PM -0700, Dmitry Torokhov wrote:
>>
>> > We have this strategy for bunch of input stuff (force release, keymap)
>> > and I think it works better than dding more and more DMI quirks into
>> > kernel itself.
>>
>> It's limited to Dell hardware, so I think keeping a static list in the
>> Dell laptop driver is reasonable. All we need is a list of hardware, and
>> I'd really hope that Dell know which BIOS versions contain this code!
>>
>
> Unfortunately this kind of crap tends to flow from one BIOS version to
> another.. How many DMI entries do we know about so far? Did Dell issue
> firmware updates correcting the issue so new boxes will not exhibit the
> problem?
>
> --
> Dmitry
>

I've seen this on 2 machines thus far.  One of them is still under
development and this was caught in time to be fixed properly in the
firmware (thanks to the patch above).  The other will not be receiving
a firmware update to resolve it.

There are likely others out there, but I've not encountered them yet.

-- 
Mario Limonciello
superm1@gmail.com

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Matthew Garrett]

On Thu, Oct 07, 2010 at 02:46:28PM -0700, Dmitry Torokhov wrote:
> On Thu, Oct 07, 2010 at 10:37:54PM +0100, Matthew Garrett wrote:
> > Dell laptop driver is reasonable. All we need is a list of hardware, and 
> > I'd really hope that Dell know which BIOS versions contain this code!
> > 
> 
> Unfortunately this kind of crap tends to flow from one BIOS version to
> another.. How many DMI entries do we know about so far? Did Dell issue
> firmware updates correcting the issue so new boxes will not exhibit the
> problem?

Pushing this out to userspace doesn't help. If it's sufficiently 
prevelant that it's impossible to have a comprehensive list, then a 
module option makes it too easy for people to just post the fix on a 
forum somewhere and never actually get it upstream.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Mario Limonciello]

On Thu, Oct 7, 2010 at 16:49, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, Oct 07, 2010 at 04:45:22PM -0500, Mario Limonciello wrote:
>> Matthew:
>>
>> On Thu, Oct 7, 2010 at 16:42, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>> > I don't understand this. You know which code is broken - surely you're
>> > able to determine which products shipped with BIOSes derived from the
>> > broken code?
>>
>> Not all product's BIOS are developed in-house.  The ones that are, yes
>> it quite possible to identify.  Matter of fact, this issue tends to
>> not exist on the in-house developed BIOS codebase.
>
> Ok, so let's approach this differently. What is it that results in this
> issue not affecting Windows?
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>

Depending on the third-party vendor that worked on the BIOS, they may
have an additional method for modifying the wireless and another tool
in addition to the standard one used on a majority of the machines.
That method is not necessarily documented.

-- 
Mario Limonciello
superm1@gmail.com

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Mario Limonciello]

On Thu, Oct 7, 2010 at 16:50, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, Oct 07, 2010 at 02:46:28PM -0700, Dmitry Torokhov wrote:
>> On Thu, Oct 07, 2010 at 10:37:54PM +0100, Matthew Garrett wrote:
>> > Dell laptop driver is reasonable. All we need is a list of hardware, and
>> > I'd really hope that Dell know which BIOS versions contain this code!
>> >
>>
>> Unfortunately this kind of crap tends to flow from one BIOS version to
>> another.. How many DMI entries do we know about so far? Did Dell issue
>> firmware updates correcting the issue so new boxes will not exhibit the
>> problem?
>
> Pushing this out to userspace doesn't help. If it's sufficiently
> prevelant that it's impossible to have a comprehensive list, then a
> module option makes it too easy for people to just post the fix on a
> forum somewhere and never actually get it upstream.
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>

Making it more difficult to enable the alternative functionality
wouldn't necessarily make the list any more comprehensive, it just
raises the bar for users who want to try to help.  If users report the
bugs to their distros, distros will help to forward them upstream.
You can't expect to fix a social problem here just by making it more
difficult to fix the original problem at hand for individuals.


-- 
Mario Limonciello
superm1@gmail.com

Re: [PATCH] dell-laptop: Add hwswitch_only module parameter

[Dmitry Torokhov]

On Thu, Oct 07, 2010 at 10:50:28PM +0100, Matthew Garrett wrote:
> On Thu, Oct 07, 2010 at 02:46:28PM -0700, Dmitry Torokhov wrote:
> > On Thu, Oct 07, 2010 at 10:37:54PM +0100, Matthew Garrett wrote:
> > > Dell laptop driver is reasonable. All we need is a list of hardware, and 
> > > I'd really hope that Dell know which BIOS versions contain this code!
> > > 
> > 
> > Unfortunately this kind of crap tends to flow from one BIOS version to
> > another.. How many DMI entries do we know about so far? Did Dell issue
> > firmware updates correcting the issue so new boxes will not exhibit the
> > problem?
> 
> Pushing this out to userspace doesn't help. If it's sufficiently 
> prevelant that it's impossible to have a comprehensive list, then a 
> module option makes it too easy for people to just post the fix on a 
> forum somewhere and never actually get it upstream.
> 

Forcing users patching the kernel reduces chances that it will be fixed
at all. Also there is load of patches that never make into distributions
and even less in mainline. Or if they make it it takes looong time...
so I do not think it is a valid argument here.

-- 
Dmitry