Regression with dell-rbtn: radio killed on resume after suspend to RAM

Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Andrei Borzenkov]

After installing 4.2 on Dell Latitude E5450 I found that wireless was 
disabled every second resume from suspend to RAM. Blacklisting dell-rbtn 
"fixed" it.

This is probably the same as discussed in this Arch forum and related 
bug report: https://bbs.archlinux.org/viewtopic.php?id=203404. I myself 
hit it on Ubuntu.

I am not familiar with other models, but E5450 does have DELLABCE and so 
is using dell-rbtn driver.

I am happy to test patches if need. Please let me know if formal bug 
report is required.

-andrei

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Pali Rohár]

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

On Saturday 21 November 2015 19:57:18 Andrei Borzenkov wrote:
> After installing 4.2 on Dell Latitude E5450 I found that wireless was
> disabled every second resume from suspend to RAM. Blacklisting
> dell-rbtn "fixed" it.
> 
> This is probably the same as discussed in this Arch forum and related
> bug report: https://bbs.archlinux.org/viewtopic.php?id=203404. I
> myself hit it on Ubuntu.
> 
> I am not familiar with other models, but E5450 does have DELLABCE and
> so is using dell-rbtn driver.
> 
> I am happy to test patches if need. Please let me know if formal bug
> report is required.
> 
> -andrei

Hi Andrei!

About five hours ago Gabriele sent patch which fixing this problem to 
LKML. You can find it on: https://lkml.org/lkml/2015/11/21/57

Can you test that patch and confirm it fix also for you?

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

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

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Andrei Borzenkov]

21.11.2015 22:08, Pali Rohár пишет:
> On Saturday 21 November 2015 19:57:18 Andrei Borzenkov wrote:
>> After installing 4.2 on Dell Latitude E5450 I found that wireless was
>> disabled every second resume from suspend to RAM. Blacklisting
>> dell-rbtn "fixed" it.
>>
>> This is probably the same as discussed in this Arch forum and related
>> bug report: https://bbs.archlinux.org/viewtopic.php?id=203404. I
>> myself hit it on Ubuntu.
>>
>> I am not familiar with other models, but E5450 does have DELLABCE and
>> so is using dell-rbtn driver.
>>
>> I am happy to test patches if need. Please let me know if formal bug
>> report is required.
>>
>> -andrei
>
> Hi Andrei!
>
> About five hours ago Gabriele sent patch which fixing this problem to
> LKML. You can find it on: https://lkml.org/lkml/2015/11/21/57
>
> Can you test that patch and confirm it fix also for you?
>

Yes, it does. Can add Tested-By me. Thank you!

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Andrei Borzenkov]

21.11.2015 23:39, Andrei Borzenkov пишет:
> 21.11.2015 22:08, Pali Rohár пишет:
>> On Saturday 21 November 2015 19:57:18 Andrei Borzenkov wrote:
>>> After installing 4.2 on Dell Latitude E5450 I found that wireless was
>>> disabled every second resume from suspend to RAM. Blacklisting
>>> dell-rbtn "fixed" it.
>>>
>>> This is probably the same as discussed in this Arch forum and related
>>> bug report: https://bbs.archlinux.org/viewtopic.php?id=203404. I
>>> myself hit it on Ubuntu.
>>>
>>> I am not familiar with other models, but E5450 does have DELLABCE and
>>> so is using dell-rbtn driver.
>>>
>>> I am happy to test patches if need. Please let me know if formal bug
>>> report is required.
>>>
>>> -andrei
>>
>> Hi Andrei!
>>
>> About five hours ago Gabriele sent patch which fixing this problem to
>> LKML. You can find it on: https://lkml.org/lkml/2015/11/21/57
>>
>> Can you test that patch and confirm it fix also for you?
>>
>
> Yes, it does.

Unfortunately it was too early. Today after resume I again got disable 
radio, and also several later attempts to suspend/resume. Then last time 
I suddenly got radio working again after resume.

Patch looks racy; there is no guarantee we get notification before 
resetting in suspend flag.

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Pali Rohár]

On Sunday 22 November 2015 09:23:19 Andrei Borzenkov wrote:
> 21.11.2015 23:39, Andrei Borzenkov пишет:
> >21.11.2015 22:08, Pali Rohár пишет:
> >>On Saturday 21 November 2015 19:57:18 Andrei Borzenkov wrote:
> >>>After installing 4.2 on Dell Latitude E5450 I found that wireless was
> >>>disabled every second resume from suspend to RAM. Blacklisting
> >>>dell-rbtn "fixed" it.
> >>>
> >>>This is probably the same as discussed in this Arch forum and related
> >>>bug report: https://bbs.archlinux.org/viewtopic.php?id=203404. I
> >>>myself hit it on Ubuntu.
> >>>
> >>>I am not familiar with other models, but E5450 does have DELLABCE and
> >>>so is using dell-rbtn driver.
> >>>
> >>>I am happy to test patches if need. Please let me know if formal bug
> >>>report is required.
> >>>
> >>>-andrei
> >>
> >>Hi Andrei!
> >>
> >>About five hours ago Gabriele sent patch which fixing this problem to
> >>LKML. You can find it on: https://lkml.org/lkml/2015/11/21/57
> >>
> >>Can you test that patch and confirm it fix also for you?
> >>
> >
> >Yes, it does.
> 
> Unfortunately it was too early. Today after resume I again got disable
> radio, and also several later attempts to suspend/resume. Then last time I
> suddenly got radio working again after resume.
> 
> Patch looks racy; there is no guarantee we get notification before resetting
> in suspend flag.
> 

Gabriele, any idea how to fix this?

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

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Gabriele Mazzotta]

On 23/11/2015 15:50, Pali Rohár wrote:
> On Sunday 22 November 2015 09:23:19 Andrei Borzenkov wrote:
>> 21.11.2015 23:39, Andrei Borzenkov пишет:
>>> 21.11.2015 22:08, Pali Rohár пишет:
>>>> On Saturday 21 November 2015 19:57:18 Andrei Borzenkov wrote:
>>>>> After installing 4.2 on Dell Latitude E5450 I found that wireless was
>>>>> disabled every second resume from suspend to RAM. Blacklisting
>>>>> dell-rbtn "fixed" it.
>>>>>
>>>>> This is probably the same as discussed in this Arch forum and related
>>>>> bug report: https://bbs.archlinux.org/viewtopic.php?id=203404. I
>>>>> myself hit it on Ubuntu.
>>>>>
>>>>> I am not familiar with other models, but E5450 does have DELLABCE and
>>>>> so is using dell-rbtn driver.
>>>>>
>>>>> I am happy to test patches if need. Please let me know if formal bug
>>>>> report is required.
>>>>>
>>>>> -andrei
>>>>
>>>> Hi Andrei!
>>>>
>>>> About five hours ago Gabriele sent patch which fixing this problem to
>>>> LKML. You can find it on: https://lkml.org/lkml/2015/11/21/57
>>>>
>>>> Can you test that patch and confirm it fix also for you?
>>>>
>>>
>>> Yes, it does.
>>
>> Unfortunately it was too early. Today after resume I again got disable
>> radio, and also several later attempts to suspend/resume. Then last time I
>> suddenly got radio working again after resume.
>>
>> Patch looks racy; there is no guarantee we get notification before resetting
>> in suspend flag.
>>
>
> Gabriele, any idea how to fix this?
>

I can't think of anything other than ignoring notifications that arrives
within X seconds after the execution of the resume callback.
Unfortunately, we have no idea on how to detect systems that misbehave
and creating a blacklist is not a feasible solution.

Something such as the following should work, but maybe it's not a nice
solution.

---
  drivers/platform/x86/dell-rbtn.c | 36 ++++++++++++++++++++++++++++++++++++
  1 file changed, 36 insertions(+)

diff --git a/drivers/platform/x86/dell-rbtn.c 
b/drivers/platform/x86/dell-rbtn.c
index cd410e3..c03062d 100644
--- a/drivers/platform/x86/dell-rbtn.c
+++ b/drivers/platform/x86/dell-rbtn.c
@@ -18,6 +18,8 @@
  #include <linux/rfkill.h>
  #include <linux/input.h>

+#define EXTRA_NOTIFICATION_TIMEOUT     (2 * HZ);
+
  enum rbtn_type {
         RBTN_UNKNOWN,
         RBTN_TOGGLE,
@@ -28,6 +30,8 @@ struct rbtn_data {
         enum rbtn_type type;
         struct rfkill *rfkill;
         struct input_dev *input_dev;
+       bool suspended;
+       unsigned long ignore_window;
  };


@@ -220,9 +224,34 @@ static const struct acpi_device_id rbtn_ids[] = {
         { "", 0 },
  };

+#ifdef CONFIG_PM_SLEEP
+static int rbtn_suspend(struct device *dev)
+{
+       struct acpi_device *device = to_acpi_device(dev);
+       struct rbtn_data *rbtn_data = acpi_driver_data(device);
+
+       rbtn_data->suspended = true;
+
+       return 0;
+}
+
+static int rbtn_resume(struct device *dev)
+{
+       struct acpi_device *device = to_acpi_device(dev);
+       struct rbtn_data *rbtn_data = acpi_driver_data(device);
+
+       rbtn_data->suspended = false;
+       rbtn_data->ignore_window = jiffies + EXTRA_NOTIFICATION_TIMEOUT;
+
+       return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(rbtn_pm_ops, rbtn_suspend, rbtn_resume);
+
  static struct acpi_driver rbtn_driver = {
         .name = "dell-rbtn",
         .ids = rbtn_ids,
+       .drv.pm = &rbtn_pm_ops,
         .ops = {
                 .add = rbtn_add,
                 .remove = rbtn_remove,
@@ -383,6 +412,13 @@ static int rbtn_remove(struct acpi_device *device)
  static void rbtn_notify(struct acpi_device *device, u32 event)
  {
         struct rbtn_data *rbtn_data = device->driver_data;
+       bool ignore_notification = rbtn_data->suspended ||
+                       time_before(jiffies, rbtn_data->ignore_window);
+
+       if (ignore_notification) {
+               dev_dbg(&device->dev, "ACPI notification ignored\n");
+               return;
+       }

         if (event != 0x80) {
                 dev_info(&device->dev, "Received unknown event (0x%x)\n",
-- 
2.6.2

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Andrei Borzenkov]

23.11.2015 18:14, Gabriele Mazzotta пишет:
> On 23/11/2015 15:50, Pali Rohár wrote:
>> On Sunday 22 November 2015 09:23:19 Andrei Borzenkov wrote:
>>> 21.11.2015 23:39, Andrei Borzenkov пишет:
>>>> 21.11.2015 22:08, Pali Rohár пишет:
>>>>> On Saturday 21 November 2015 19:57:18 Andrei Borzenkov wrote:
>>>>>> After installing 4.2 on Dell Latitude E5450 I found that wireless was
>>>>>> disabled every second resume from suspend to RAM. Blacklisting
>>>>>> dell-rbtn "fixed" it.
>>>>>>
>>>>>> This is probably the same as discussed in this Arch forum and related
>>>>>> bug report: https://bbs.archlinux.org/viewtopic.php?id=203404. I
>>>>>> myself hit it on Ubuntu.
>>>>>>
>>>>>> I am not familiar with other models, but E5450 does have DELLABCE and
>>>>>> so is using dell-rbtn driver.
>>>>>>
>>>>>> I am happy to test patches if need. Please let me know if formal bug
>>>>>> report is required.
>>>>>>
>>>>>> -andrei
>>>>>
>>>>> Hi Andrei!
>>>>>
>>>>> About five hours ago Gabriele sent patch which fixing this problem to
>>>>> LKML. You can find it on: https://lkml.org/lkml/2015/11/21/57
>>>>>
>>>>> Can you test that patch and confirm it fix also for you?
>>>>>
>>>>
>>>> Yes, it does.
>>>
>>> Unfortunately it was too early. Today after resume I again got disable
>>> radio, and also several later attempts to suspend/resume. Then last
>>> time I
>>> suddenly got radio working again after resume.
>>>
>>> Patch looks racy; there is no guarantee we get notification before
>>> resetting
>>> in suspend flag.
>>>
>>
>> Gabriele, any idea how to fix this?
>>
>
> I can't think of anything other than ignoring notifications that arrives
> within X seconds after the execution of the resume callback.
> Unfortunately, we have no idea on how to detect systems that misbehave
> and creating a blacklist is not a feasible solution.
>
> Something such as the following should work, but maybe it's not a nice
> solution.
>

I looked at what dell-rbtn does. My system (Latitude E5450) has RBTN of 
type TOGGLE. In this case both del-laptop hooks itself into i8042 and 
dell-rbtn delivers KEY_RFKILL event to input susbsystem. IOW dell-rbtn 
looks completely redundant in this configuration.

Can we detect that rfkill toggle is already avaiable via normal keyboard 
and not activate dell-rbtn in this case?

> ---
>   drivers/platform/x86/dell-rbtn.c | 36
> ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>
> diff --git a/drivers/platform/x86/dell-rbtn.c
> b/drivers/platform/x86/dell-rbtn.c
> index cd410e3..c03062d 100644
> --- a/drivers/platform/x86/dell-rbtn.c
> +++ b/drivers/platform/x86/dell-rbtn.c
> @@ -18,6 +18,8 @@
>   #include <linux/rfkill.h>
>   #include <linux/input.h>
>
> +#define EXTRA_NOTIFICATION_TIMEOUT     (2 * HZ);
> +
>   enum rbtn_type {
>          RBTN_UNKNOWN,
>          RBTN_TOGGLE,
> @@ -28,6 +30,8 @@ struct rbtn_data {
>          enum rbtn_type type;
>          struct rfkill *rfkill;
>          struct input_dev *input_dev;
> +       bool suspended;
> +       unsigned long ignore_window;
>   };
>
>
> @@ -220,9 +224,34 @@ static const struct acpi_device_id rbtn_ids[] = {
>          { "", 0 },
>   };
>
> +#ifdef CONFIG_PM_SLEEP
> +static int rbtn_suspend(struct device *dev)
> +{
> +       struct acpi_device *device = to_acpi_device(dev);
> +       struct rbtn_data *rbtn_data = acpi_driver_data(device);
> +
> +       rbtn_data->suspended = true;
> +
> +       return 0;
> +}
> +
> +static int rbtn_resume(struct device *dev)
> +{
> +       struct acpi_device *device = to_acpi_device(dev);
> +       struct rbtn_data *rbtn_data = acpi_driver_data(device);
> +
> +       rbtn_data->suspended = false;
> +       rbtn_data->ignore_window = jiffies + EXTRA_NOTIFICATION_TIMEOUT;
> +
> +       return 0;
> +}
> +#endif
> +static SIMPLE_DEV_PM_OPS(rbtn_pm_ops, rbtn_suspend, rbtn_resume);
> +
>   static struct acpi_driver rbtn_driver = {
>          .name = "dell-rbtn",
>          .ids = rbtn_ids,
> +       .drv.pm = &rbtn_pm_ops,
>          .ops = {
>                  .add = rbtn_add,
>                  .remove = rbtn_remove,
> @@ -383,6 +412,13 @@ static int rbtn_remove(struct acpi_device *device)
>   static void rbtn_notify(struct acpi_device *device, u32 event)
>   {
>          struct rbtn_data *rbtn_data = device->driver_data;
> +       bool ignore_notification = rbtn_data->suspended ||
> +                       time_before(jiffies, rbtn_data->ignore_window);
> +
> +       if (ignore_notification) {
> +               dev_dbg(&device->dev, "ACPI notification ignored\n");
> +               return;
> +       }
>
>          if (event != 0x80) {
>                  dev_info(&device->dev, "Received unknown event (0x%x)\n",

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Darren Hart]

On Mon, Nov 23, 2015 at 07:23:29PM +0300, Andrei Borzenkov wrote:
> 23.11.2015 18:14, Gabriele Mazzotta пишет:
> >On 23/11/2015 15:50, Pali Rohár wrote:
> >>On Sunday 22 November 2015 09:23:19 Andrei Borzenkov wrote:
> >>>21.11.2015 23:39, Andrei Borzenkov пишет:
> >>>>21.11.2015 22:08, Pali Rohár пишет:
> >>>>>On Saturday 21 November 2015 19:57:18 Andrei Borzenkov wrote:
> >>>>>>After installing 4.2 on Dell Latitude E5450 I found that wireless was
> >>>>>>disabled every second resume from suspend to RAM. Blacklisting
> >>>>>>dell-rbtn "fixed" it.
> >>>>>>
> >>>>>>This is probably the same as discussed in this Arch forum and related
> >>>>>>bug report: https://bbs.archlinux.org/viewtopic.php?id=203404. I
> >>>>>>myself hit it on Ubuntu.
> >>>>>>
> >>>>>>I am not familiar with other models, but E5450 does have DELLABCE and
> >>>>>>so is using dell-rbtn driver.
> >>>>>>
> >>>>>>I am happy to test patches if need. Please let me know if formal bug
> >>>>>>report is required.
> >>>>>>
> >>>>>>-andrei
> >>>>>
> >>>>>Hi Andrei!
> >>>>>
> >>>>>About five hours ago Gabriele sent patch which fixing this problem to
> >>>>>LKML. You can find it on: https://lkml.org/lkml/2015/11/21/57
> >>>>>
> >>>>>Can you test that patch and confirm it fix also for you?
> >>>>>
> >>>>
> >>>>Yes, it does.
> >>>
> >>>Unfortunately it was too early. Today after resume I again got disable
> >>>radio, and also several later attempts to suspend/resume. Then last
> >>>time I
> >>>suddenly got radio working again after resume.
> >>>
> >>>Patch looks racy; there is no guarantee we get notification before
> >>>resetting
> >>>in suspend flag.
> >>>
> >>
> >>Gabriele, any idea how to fix this?
> >>
> >
> >I can't think of anything other than ignoring notifications that arrives
> >within X seconds after the execution of the resume callback.
> >Unfortunately, we have no idea on how to detect systems that misbehave
> >and creating a blacklist is not a feasible solution.
> >
> >Something such as the following should work, but maybe it's not a nice
> >solution.
> >
> 
> I looked at what dell-rbtn does. My system (Latitude E5450) has RBTN of type
> TOGGLE. In this case both del-laptop hooks itself into i8042 and dell-rbtn
> delivers KEY_RFKILL event to input susbsystem. IOW dell-rbtn looks
> completely redundant in this configuration.
> 
> Can we detect that rfkill toggle is already avaiable via normal keyboard and
> not activate dell-rbtn in this case?

Also dropping this one until we can arrive at a complete solution.

Pali, Gabriele, this one is in your hands. I will review and provide feedback
where I can - I confess I'm finding it difficult to keep all the pieces straight
in my head, and without any hardware, I have to rely entirely on what I can
piece together (a situation we are all in as this differs across many systems,
nobody has all the necessary bits).

It may be helpful for someone to put together a current status, known issues,
plan of attack to get this moving forward again.

-- 
Darren Hart
Intel Open Source Technology Center

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Gabriele Mazzotta]

On 23/11/2015 20:29, Darren Hart wrote:

[cut]

>> I looked at what dell-rbtn does. My system (Latitude E5450) has RBTN of type
>> TOGGLE. In this case both del-laptop hooks itself into i8042 and dell-rbtn
>> delivers KEY_RFKILL event to input susbsystem. IOW dell-rbtn looks
>> completely redundant in this configuration.
>>
>> Can we detect that rfkill toggle is already avaiable via normal keyboard and
>> not activate dell-rbtn in this case?
>
> Also dropping this one until we can arrive at a complete solution.
>
> Pali, Gabriele, this one is in your hands. I will review and provide feedback
> where I can - I confess I'm finding it difficult to keep all the pieces straight
> in my head, and without any hardware, I have to rely entirely on what I can
> piece together (a situation we are all in as this differs across many systems,
> nobody has all the necessary bits).
>
> It may be helpful for someone to put together a current status, known issues,
> plan of attack to get this moving forward again.
>

I'll try to write a summary that should explain why dell-rbtn exists
and what are the current problems.

There are several mechanisms to control radio devices, so let's
consider different categories/cases.

A) The function keys of new Dell laptops do nothing on their own, but
when pressed, the BIOS sends a notification to an ACPI device
(DELLABCE/DELRBTN). One of the tasks of dell-rbtn is to catch those
notifications and send an input event to userspace. Without dell-rbtn,
the function keys of these laptops wouldn't work.

B) There are then some other laptops which support two different
mechanisms to control radio devices: the one here above (A) and
"the old one", i.e. the BIOS does everything. (These are probably
laptops that were released during the transition Windows 7 -> 8). These
laptops can switch between the two modes through an ACPI method call.
What we do with dell-rbtn is to make them behave like (A) since we
can't tell apart laptops of (A) and (B) and we need to know how each
laptop behaves. The function key of these laptops work perfectly
without dell-rbtn.

C) There are then some other laptops for which it's required to use an
i8042 filter to detect keypresses and do some SMI calls to toggle
the state of radio devices. This filter is created by dell-laptop and
has been there since a long time.

D) This category is something in between (C) and (A) and exists only
because of dell-rbtn. Some of the laptops of category (C) also have a
DELLABCE/DELLRBTN device. When dell-rbtn is loaded, the i8042 filter is
removed and dell-rbtn starts listening for ACPI notification. We do
this instead because the i8042 filter doesn't work properly on some
laptops.


The problem reported in [1] is that some laptops of (A) and (B) send a
notification on resume and some others don't. [2] should fix this
problem, but we need to make sure that the extra notification is always
caught before the resume of dell-rbtn. This seems to be what happens
here on my laptop, but Andrei says otherwise. So we need to figure out
if [2] doesn't fix the problem because it's not guaranteed that ACPI
notifications sent while the system is resuming are received by device
drivers before they are resumed (assumption that I made for the first
patch and that dropped with the updated one I sent in this thread) or
because Andrei's problem is something different.


I hope this was clear enough.


Andrei, as I wrote here above, dell-rbtn _should_ make dell-laptop
remove the i8042 filter. Could you also provide your acpidump so that
I can see it?


[1] https://bugzilla.kernel.org/show_bug.cgi?id=106031
[2] 
http://lkml.kernel.org/r/1448115375-7315-1-git-send-email-gabriele.mzt@gmail.com

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Pali Rohár]

On Tuesday 24 November 2015 07:06:27 Andrei Borzenkov wrote:
> I am not sure whether keyboard key should be treated as hard or soft (i.e. I
> do not know how to verify that radio is switched off without OS running),
> but I still expect the same behavior in both cases.
> 

rbtn is translated to rfkill only if type is HW slider switch, not
toggle key! And HW slider switch cannot be overwritten by SW, so it is
hard rfkill.

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

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Gabriele Mazzotta]

On 24/11/2015 05:06, Andrei Borzenkov wrote:
> 24.11.2015 01:33, Gabriele Mazzotta пишет:
> [...snip...]
>>
>> The problem reported in [1] is that some laptops of (A) and (B) send a
>> notification on resume and some others don't. [2] should fix this
>> problem, but we need to make sure that the extra notification is always
>> caught before the resume of dell-rbtn. This seems to be what happens
>> here on my laptop, but Andrei says otherwise. So we need to figure out
>> if [2] doesn't fix the problem because it's not guaranteed that ACPI
>> notifications sent while the system is resuming are received by device
>> drivers before they are resumed (assumption that I made for the first
>> patch and that dropped with the updated one I sent in this thread) or
>> because Andrei's problem is something different.
>>
>>
>> I hope this was clear enough.
>>
>>
>> Andrei, as I wrote here above, dell-rbtn _should_ make dell-laptop
>> remove the i8042 filter.
>
> No. In dell_rbtn_notifier_register() it checks for RBTN of type SLIDER
> using rbtn_inc_count(). If no SLIDER is found it returns ENODEV and
> dell-laptop proceeds with i8042 hook.
>
> [   16.685944] dell_laptop: Using i8042 filter function for receiving
> events
>
> But that does *not* stop dell-rbtn from actually running and sending
> input events:

I think I see the problem now. Your case (RBTN_TOGGLE + 
Latitude/Precision) wasn't taken into account. I think my laptop and 
yours behave in the same way, but since mine is an XPS, dell-laptop 
doesn't create the rfkill + filter for it. dell-laptop does this only
for Latitude and Precision laptops.

> /*
>   * acpi driver functions
>   */
>
> static int rbtn_add(struct acpi_device *device)
> {
> ...
>          case RBTN_TOGGLE:
>                  ret = rbtn_input_init(rbtn_data);
>                  break;
>
> So as I said, we now have both of them handling the same key press.
>
> Also using dell-laptop (3.13 kernel) and dell-laptop + dell-rbtn (4.2
> kernel with your patch to ignore events during suspend) I see different
> reaction to key press:
>
> 3.13
>
> 0: dell-wifi: Wireless LAN
>      Soft blocked: no
>      Hard blocked: no
> 1: dell-bluetooth: Bluetooth
>      Soft blocked: no
>      Hard blocked: no
> 2: phy0: Wireless LAN
>      Soft blocked: no
>      Hard blocked: no
> 6: hci0: Bluetooth
>      Soft blocked: no
>      Hard blocked: no
>
> Press rfkill button
>
> 0: dell-wifi: Wireless LAN
>      Soft blocked: no
>      Hard blocked: yes
> 1: dell-bluetooth: Bluetooth
>      Soft blocked: no
>      Hard blocked: yes
> 2: phy0: Wireless LAN
>      Soft blocked: no
>      Hard blocked: yes
>
> 4.2
>
> 0: dell-wifi: Wireless LAN
>      Soft blocked: no
>      Hard blocked: no
> 1: dell-bluetooth: Bluetooth
>      Soft blocked: no
>      Hard blocked: no
> 2: hci0: Bluetooth
>      Soft blocked: no
>      Hard blocked: no
> 3: phy0: Wireless LAN
>      Soft blocked: no
>      Hard blocked: no
>
> Press rfkill button
>
> 0: dell-wifi: Wireless LAN
>      Soft blocked: yes
>      Hard blocked: no
> 1: dell-bluetooth: Bluetooth
>      Soft blocked: yes
>      Hard blocked: no
> 2: hci0: Bluetooth
>      Soft blocked: yes
>      Hard blocked: no
> 3: phy0: Wireless LAN
>      Soft blocked: yes
>      Hard blocked: yes
>
> I am not sure whether keyboard key should be treated as hard or soft
> (i.e. I do not know how to verify that radio is switched off without OS
> running), but I still expect the same behavior in both cases.

Unfortunately this is expected. Looking at your DSDT it seems that the
function key of your laptop is not intended to hard block radio devices.

Also, I can see that a notification is sent on resume.

>> Could you also provide your acpidump so that
>> I can see it?
>>
>
> Attached (I thought about adding to bugzilla, but as we are not sure
> whether it is the same bug, I send it here).
>
> I have old Dell XPS M1303 with both slider and button type of rfkill,
> I'll try to see what happens there.

I think the BIOS is sending a notification on resume also on your
laptop, but I could be wrong. The followings should be the ACPI method
calls that happen on resume:
_WAK -> RWAK -> EV2 -> \_SB.RBTN.NRBT (this one notifies RBTN)

So you have two problems: the extra notification + i8042 filter.

> P.S. it seems that RBTN is exposed if _OSI claims sufficiently recent
> Windows is present; I tried booting with acpi_osi=! acpi_osi=Linux and
> got no RBTN device at all. I wonder how Windows handles it.

Windows 8 has changed the way radio function keys work. They are now
stateless buttons that inform the OS when they are pressed. It's up to
the OS handle the state of radio devices.

This is the reason why you see a different output using the rfkill +
dell-rbtn. We are handling radio devices as Windows does, mostly
because it's the only way to handle the function key of some laptops.

Your laptop should actually be one of those, but since it's a Latitude,
dell-laptop gets in the way. I also have issues when I load dell-laptop
(RBTN_TOGGLE here) with the force_rfkill param (mine is an XPS).


Pali, what should we do for the case Latitude/Precision + RBTN_TOGGLE?

>>
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=106031
>> [2]
>> http://lkml.kernel.org/r/1448115375-7315-1-git-send-email-gabriele.mzt@gmail.com
>>
>>
>

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Matthew Garrett]

On Tue, Nov 24, 2015 at 09:49:18AM +0100, Gabriele Mazzotta wrote:
> 
> Pali, what should we do for the case Latitude/Precision + RBTN_TOGGLE?

Are there any systems which claim Windows 8 support via OSI and which 
don't implement the rbtn interface? If not, just add an ACPI dependency 
to dell-laptop and have it skip the rfkill code on Windows 8 and later 
systems. I never trusted that code, but Dell insisted that it would 
always wor on Latitudes and Precisions…

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

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Pali Rohár]

On Tuesday 24 November 2015 08:52:16 Matthew Garrett wrote:
> On Tue, Nov 24, 2015 at 09:49:18AM +0100, Gabriele Mazzotta wrote:
> > 
> > Pali, what should we do for the case Latitude/Precision + RBTN_TOGGLE?
> 
> Are there any systems which claim Windows 8 support via OSI and which 
> don't implement the rbtn interface? If not, just add an ACPI dependency 
> to dell-laptop and have it skip the rfkill code on Windows 8 and later 
> systems. I never trusted that code, but Dell insisted that it would 
> always wor on Latitudes and Precisions…
> 

Matthew, Dell released documentation for that radio device code in
userspace libsmbios project. And Gabriele told me that userspace tools
works fine. Just kernel implementation is somehow broken and does not
work correctly on XPS. I already found some bugs in that kernel
implementation (e.g. garbage was send in buffer via SMI to BIOS). And I
think that SMBIOS is working also on XPS laptops, just there are unfixed
bugs in kernel...

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

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Gabriele Mazzotta]

On 24/11/2015 09:52, Matthew Garrett wrote:
> On Tue, Nov 24, 2015 at 09:49:18AM +0100, Gabriele Mazzotta wrote:
>>
>> Pali, what should we do for the case Latitude/Precision + RBTN_TOGGLE?
>
> Are there any systems which claim Windows 8 support via OSI and which
> don't implement the rbtn interface? If not, just add an ACPI dependency
> to dell-laptop and have it skip the rfkill code on Windows 8 and later
> systems. I never trusted that code, but Dell insisted that it would
> always wor on Latitudes and Precisions…
>

I think that systems that claim Windows 8 support also implement an
rbtn interface, but I can't be sure of this.

Such a solution would work, but it's worth pointing out that this would
move the control of radios from kernel space to user space. Honestly,
I'm fine this, but not everybody agrees.

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Andrei Borzenkov]

24.11.2015 12:42, Gabriele Mazzotta пишет:
> On 24/11/2015 09:52, Matthew Garrett wrote:
>> On Tue, Nov 24, 2015 at 09:49:18AM +0100, Gabriele Mazzotta wrote:
>>>
>>> Pali, what should we do for the case Latitude/Precision + RBTN_TOGGLE?
>>
>> Are there any systems which claim Windows 8 support via OSI and which
>> don't implement the rbtn interface? If not, just add an ACPI dependency
>> to dell-laptop and have it skip the rfkill code on Windows 8 and later
>> systems. I never trusted that code, but Dell insisted that it would
>> always wor on Latitudes and Precisions…

And the code actually worked and still does :)

>>
> 
> I think that systems that claim Windows 8 support also implement an
> rbtn interface, but I can't be sure of this.
> 
> Such a solution would work, but it's worth pointing out that this would
> move the control of radios from kernel space to user space. 

Well, this would be wrong. Button press is still handled by hardware
(BIOS) so we should not pretend it is software. We should rather fix
dell-rbtn to do the right thing when notification is received - i.e.
query current hardware state via dell-laptop.

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Andrei Borzenkov]

24.11.2015 01:33, Gabriele Mazzotta пишет:

> I'll try to write a summary that should explain why dell-rbtn exists
> and what are the current problems.
> 
> There are several mechanisms to control radio devices, so let's
> consider different categories/cases.
> 
> A) The function keys of new Dell laptops do nothing on their own, but
> when pressed, the BIOS sends a notification to an ACPI device
> (DELLABCE/DELRBTN). One of the tasks of dell-rbtn is to catch those
> notifications and send an input event to userspace. Without dell-rbtn,
> the function keys of these laptops wouldn't work.
> 
> B) There are then some other laptops which support two different
> mechanisms to control radio devices: the one here above (A) and
> "the old one", i.e. the BIOS does everything. (These are probably
> laptops that were released during the transition Windows 7 -> 8). These
> laptops can switch between the two modes through an ACPI method call.
> What we do with dell-rbtn is to make them behave like (A) since we
> can't tell apart laptops of (A) and (B) and we need to know how each
> laptop behaves. The function key of these laptops work perfectly
> without dell-rbtn.
> 
> C) There are then some other laptops for which it's required to use an
> i8042 filter to detect keypresses and do some SMI calls to toggle
> the state of radio devices. This filter is created by dell-laptop and
> has been there since a long time.
> 
> D) This category is something in between (C) and (A) and exists only
> because of dell-rbtn. Some of the laptops of category (C) also have a
> DELLABCE/DELLRBTN device.

I'm pretty confident that my Dell Latitude falls into this category.
What happens here apparently is

- user presses hotkey on keyboard
- hotkey is handled by BIOS to toggle radio
- BIOS sends notification to OS via ACPI that something changed and OS
must query current radio state

Adding debug print do dell-laptop and dell-rbtn

[43345.599909] dell-laptop: got keyboard event
[43345.704558] dell-rbtn DELLABCE:00: Received event (0x80)

So ACPI event comes rather later after key press.

This makes those notifications at wake up not spurious at all - they are
actually pretty logical, and serve to make OS ascertain current status
of radio kill switch. But I am pretty much confident that those events
do NOT mean to be used as request to toggle kill switch state.


> When dell-rbtn is loaded, the i8042 filter is
> removed and dell-rbtn starts listening for ACPI notification. We do
> this instead because the i8042 filter doesn't work properly on some
> laptops.
>
>

And here is the actual bug. dell-rbtn handles category D as if it were
category A. For category D it should *not* send input event, but rather
hook and notify dell-laptop to query current kill switch state (because
at least on my system there is no way to do it via ACPI RBTN).

So the actual problem is to differentiate between categories A and D. Is
it possible to do based on ACPI name (DELRBTN vs DELLABCE) or may be on
return value of CRBT? If not we really need black/white list here based
on DMI or something.

And BTW these input events are consumed by kernel in filter installed by
net/rfkill/input.c, so they are not really user space.

> The problem reported in [1] is that some laptops of (A) and (B) send a
> notification on resume and some others don't. [2] should fix this
> problem, but we need to make sure that the extra notification is always
> caught before the resume of dell-rbtn. 

If dell-rbtn did the right thing for my laptop, those notifications were
harmless.

> This seems to be what happens
> here on my laptop, but Andrei says otherwise. So we need to figure out
> if [2] doesn't fix the problem because it's not guaranteed that ACPI
> notifications sent while the system is resuming are received by device
> drivers before they are resumed (assumption that I made for the first
> patch and that dropped with the updated one I sent in this thread) or
> because Andrei's problem is something different.
> 

Are you sure the laptops in bugzilla are actually of type A and not of
type D? That would match what I observe here.

> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=106031
> [2]
> http://lkml.kernel.org/r/1448115375-7315-1-git-send-email-gabriele.mzt@gmail.com
> 

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Andrei Borzenkov]

On Wed, Nov 25, 2015 at 6:53 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>
> This makes those notifications at wake up not spurious at all - they are
> actually pretty logical, and serve to make OS ascertain current status
> of radio kill switch. But I am pretty much confident that those events
> do NOT mean to be used as request to toggle kill switch state.
>

Pretty much what Alex said already :)

https://lkml.org/lkml/2014/12/24/113

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Pali Rohár]

On Wednesday 25 November 2015 06:53:56 Andrei Borzenkov wrote:
> So the actual problem is to differentiate between categories A and D. Is
> it possible to do based on ACPI name (DELRBTN vs DELLABCE) or may be on
> return value of CRBT? If not we really need black/white list here based
> on DMI or something.
> 

I do not trust ACPI device names and ACPI HIDs. Maybe that return value
of CRBT can be useful. From information from Alex we know that 0 and 1
types are toggle buttons and 2 and 3 are HW slider switch. Maybe there
is also difference between 0 and 1?

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

Re: Regression with dell-rbtn: radio killed on resume after suspend to RAM

[Gabriele Mazzotta]

On 25/11/2015 04:53, Andrei Borzenkov wrote:
> 24.11.2015 01:33, Gabriele Mazzotta пишет:
>
>> I'll try to write a summary that should explain why dell-rbtn exists
>> and what are the current problems.
>>
>> There are several mechanisms to control radio devices, so let's
>> consider different categories/cases.
>>
>> A) The function keys of new Dell laptops do nothing on their own, but
>> when pressed, the BIOS sends a notification to an ACPI device
>> (DELLABCE/DELRBTN). One of the tasks of dell-rbtn is to catch those
>> notifications and send an input event to userspace. Without dell-rbtn,
>> the function keys of these laptops wouldn't work.
>>
>> B) There are then some other laptops which support two different
>> mechanisms to control radio devices: the one here above (A) and
>> "the old one", i.e. the BIOS does everything. (These are probably
>> laptops that were released during the transition Windows 7 -> 8). These
>> laptops can switch between the two modes through an ACPI method call.
>> What we do with dell-rbtn is to make them behave like (A) since we
>> can't tell apart laptops of (A) and (B) and we need to know how each
>> laptop behaves. The function key of these laptops work perfectly
>> without dell-rbtn.
>>
>> C) There are then some other laptops for which it's required to use an
>> i8042 filter to detect keypresses and do some SMI calls to toggle
>> the state of radio devices. This filter is created by dell-laptop and
>> has been there since a long time.
>>
>> D) This category is something in between (C) and (A) and exists only
>> because of dell-rbtn. Some of the laptops of category (C) also have a
>> DELLABCE/DELLRBTN device.
>
> I'm pretty confident that my Dell Latitude falls into this category.
> What happens here apparently is
>
> - user presses hotkey on keyboard
> - hotkey is handled by BIOS to toggle radio
> - BIOS sends notification to OS via ACPI that something changed and OS
> must query current radio state

So is the i8042 filter doing anything? What happens if you remove your
laptop from the whitelist of dell-laptop (see dell_setup_rfkill())?
Is dell-rbtn handling your function key presses correctly in this case?

> Adding debug print do dell-laptop and dell-rbtn
>
> [43345.599909] dell-laptop: got keyboard event
> [43345.704558] dell-rbtn DELLABCE:00: Received event (0x80)
>
> So ACPI event comes rather later after key press.

According to some Windows documentation, the ACPI event must come after
the function key is released.

> This makes those notifications at wake up not spurious at all - they are
> actually pretty logical, and serve to make OS ascertain current status
> of radio kill switch. But I am pretty much confident that those events
> do NOT mean to be used as request to toggle kill switch state.

They are spurious in my case.

>
>> When dell-rbtn is loaded, the i8042 filter is
>> removed and dell-rbtn starts listening for ACPI notification. We do
>> this instead because the i8042 filter doesn't work properly on some
>> laptops.
>>
>>
>
> And here is the actual bug. dell-rbtn handles category D as if it were
> category A. For category D it should *not* send input event, but rather
> hook and notify dell-laptop to query current kill switch state (because
> at least on my system there is no way to do it via ACPI RBTN).
>
> So the actual problem is to differentiate between categories A and D. Is
> it possible to do based on ACPI name (DELRBTN vs DELLABCE) or may be on
> return value of CRBT? If not we really need black/white list here based
> on DMI or something.
>
> And BTW these input events are consumed by kernel in filter installed by
> net/rfkill/input.c, so they are not really user space.
>
>> The problem reported in [1] is that some laptops of (A) and (B) send a
>> notification on resume and some others don't. [2] should fix this
>> problem, but we need to make sure that the extra notification is always
>> caught before the resume of dell-rbtn.
>
> If dell-rbtn did the right thing for my laptop, those notifications were
> harmless.
>
>> This seems to be what happens
>> here on my laptop, but Andrei says otherwise. So we need to figure out
>> if [2] doesn't fix the problem because it's not guaranteed that ACPI
>> notifications sent while the system is resuming are received by device
>> drivers before they are resumed (assumption that I made for the first
>> patch and that dropped with the updated one I sent in this thread) or
>> because Andrei's problem is something different.
>>
>
> Are you sure the laptops in bugzilla are actually of type A and not of
> type D? That would match what I observe here.

I have the same laptop, which is of type B. Without dell-rbtn the BIOS
handles radios, with dell-rbtn it simply sends ACPI notifications.

>>
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=106031
>> [2]
>> http://lkml.kernel.org/r/1448115375-7315-1-git-send-email-gabriele.mzt@gmail.com
>>
>