Re: [Bug 106031] Regression in 4.2.x: in airplane mode each time I open my laptop lid

["Pali Rohár" <pali.rohar@gmail.com>]

On Friday 23 October 2015 20:03:19 Gabriele Mazzotta wrote:
> On 23/10/2015 13:14, Pali Rohár wrote:
> >On Friday 23 October 2015 11:47:25 Gabriele Mazzotta wrote:
> >>>In my opinion it is better to ignore user key press after resume, if it
> >>>fix our problem. Better as false-positive event.
> >>
> >>The following appears to work really well. The notification arrives
> >>before rbtn_resume() has been executed, so the extra event is ignored.
> >>
> >>diff --git a/drivers/platform/x86/dell-rbtn.c
> >>b/drivers/platform/x86/dell-rbtn.c
> >>index cd410e3..1d64b72 100644
> >>--- a/drivers/platform/x86/dell-rbtn.c
> >>+++ b/drivers/platform/x86/dell-rbtn.c
> >>@@ -28,6 +28,7 @@ struct rbtn_data {
> >>  	enum rbtn_type type;
> >>  	struct rfkill *rfkill;
> >>  	struct input_dev *input_dev;
> >>+	bool suspended;
> >>  };
> >>
> >>
> >>@@ -220,9 +221,33 @@ 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;
> >>+
> >>+	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,
> >>@@ -384,6 +409,9 @@ static void rbtn_notify(struct acpi_device *device, u32
> >>event)
> >>  {
> >>  	struct rbtn_data *rbtn_data = device->driver_data;
> >>
> >>+	if (rbtn_data->suspended)
> >>+		return;
> >>+
> >>  	if (event != 0x80) {
> >>  		dev_info(&device->dev, "Received unknown event (0x%x)\n",
> >>  			 event);
> >>
> >
> >Great, but is not there a better way to turn off .notify ACPI function
> >when that ACPI device is suspended?
> >
> >Is not this ACPI device driver bug that it allows to call .notify method
> >even if device is suspended?
> 
> I was surprised this worked, I was assuming that nothing could run
> before the resume callback, but I was wrong. I think it makes sense to
> treat ACPI devices in a special way, but I really don't know, we need
> someone more knowledgeable to answer these questions. However, while I
> was trying to figure things out, I stumbled upon the following:
> e71eeb2a6bcc ("ACPI / button: Do not propagate wakeup-from-suspend events").

Gabriele, are you going to send this patch?

I think that patch should be OK as it drop events when device is in
suspend state (when it should not receive events)...

Darren, what do you think about it?

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