Re: [PATCH v3] dell-rbtn: Ignore ACPI notifications if device is suspended

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

On 24/05/2016 00:22, Pali Rohár wrote:
> On Tuesday 24 May 2016 00:17:15 Darren Hart wrote:
>> On Tue, May 24, 2016 at 12:06:03AM +0200, Pali Rohár wrote:
>>> On Monday 23 May 2016 23:26:55 Darren Hart wrote:
>>>> I've queued this. Thanks for your patience.
>>>
>>> Ok, In that case I would update comments in patch to try it more
>>> clear what code is doing.
>>
>> I thought I had your approval on this one Pali. Apologies if that was
>> not the case. Did I miss a change request from you?
>>
>> If so, please point me at it, and I'll dequeue this one and wait for
>> an updated one.
> 
> I just wanted to review that code from somebody else and decide if 
> accept it or not. Because I was not sure if it is OK...
> 
> But there was no objection, so patch is OK.
> 
> And I pointed that patch could have better comments to describe what it 
> is doing as at first time I was confused.
> 
> So I believe that you can update patch in your queue with new version 
> which just change comments in source code (without functional changes).
> 

Something such as the following?
Feel free to reword the comments if you have something better in mind.

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

diff --git a/drivers/platform/x86/dell-rbtn.c b/drivers/platform/x86/dell-rbtn.c
index 331d63c..e0208ba 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;
 };
 
 
@@ -235,9 +236,55 @@ static const struct acpi_device_id rbtn_ids[] = {
 	{ "", 0 },
 };
 
+#ifdef CONFIG_PM_SLEEP
+static void ACPI_SYSTEM_XFACE rbtn_acpi_clear_flag(void *context)
+{
+	struct rbtn_data *rbtn_data = context;
+
+	rbtn_data->suspended = false;
+}
+
+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);
+	acpi_status status;
+
+	/*
+	 * Upon resume, some BIOSes autonomously send an ACPI notification
+	 * that triggers an unwanted input event. In order to ignore it,
+	 * we use a flag that we set at suspend and clear once we have
+	 * received the extra notification. Since ACPI notifications are
+	 * delivered asynchronously to drivers, we clear the flag from the
+	 * workqueue used to deliver the notifications. This should be enough
+	 * to guarantee that the flag is cleared only after we received the
+	 * extra notification, if any.
+	 */
+	status = acpi_os_execute(OSL_NOTIFY_HANDLER,
+			 rbtn_acpi_clear_flag, rbtn_data);
+	if (ACPI_FAILURE(status))
+		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,
@@ -399,6 +446,15 @@ static void rbtn_notify(struct acpi_device *device, u32 event)
 {
 	struct rbtn_data *rbtn_data = device->driver_data;
 
+	/*
+	 * Some BIOSes send autonomously a notification at resume.
+	 * Ignore it to prevent unwanted input events.
+	 */
+	if (rbtn_data->suspended) {
+		dev_dbg(&device->dev, "ACPI notification ignored\n");
+		return;
+	}
+
 	if (event != 0x80) {
 		dev_info(&device->dev, "Received unknown event (0x%x)\n",
 			 event);
-- 
2.8.1