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

[Gabriele Mazzotta <gabriele.mzt@gmail.com>]

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