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@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
