All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michał Kępień" <kernel@kempniu.pl>
To: Jonathan Woithe <jwoithe@just42.net>,
	Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for storing hotkey scancodes
Date: Fri, 16 Jun 2017 06:40:52 +0200	[thread overview]
Message-ID: <20170616044058.30443-2-kernel@kempniu.pl> (raw)
In-Reply-To: <20170616044058.30443-1-kernel@kempniu.pl>

All ACPI device notify callbacks are invoked using acpi_os_execute(),
which causes the supplied callback to be queued to a static workqueue
which always executes on CPU 0.  This means that there is no possibility
for any ACPI device notify callback to be concurrently executed on
multiple CPUs, which in the case of fujitsu-laptop means that using a
locked kfifo for handling hotkeys is redundant: as hotkey scancodes are
only pushed and popped from within acpi_fujitsu_laptop_notify(), no risk
of concurrent pushing and popping exists.

Instead of a kfifo, use a fixed-size integer table for storing pushed
scancodes and an associated variable holding the number of scancodes
currently stored in that table.  Change debug messages so that they no
longer contain the term "ringbuffer".

In order to simplify implementation, hotkey input device behavior is
slightly changed (from FIFO to LIFO).  The order of release events
generated by the input device should not matter from end user
perspective as upon releasing any hotkey the firmware only produces a
single event which means "all hotkeys were released".

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/fujitsu-laptop.c | 54 +++++++++--------------------------
 1 file changed, 14 insertions(+), 40 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 1c6fdd952c75..54cb7ae541d4 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -58,7 +58,6 @@
 #include <linux/fb.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
-#include <linux/kfifo.h>
 #include <linux/leds.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -110,7 +109,6 @@
 #define KEY5_CODE	0x420
 
 #define MAX_HOTKEY_RINGBUFFER_SIZE 100
-#define RINGBUFFERSIZE 40
 
 /* Debugging */
 #define FUJLAPTOP_DBG_ERROR	  0x0001
@@ -146,8 +144,8 @@ struct fujitsu_laptop {
 	struct input_dev *input;
 	char phys[32];
 	struct platform_device *pf_device;
-	struct kfifo fifo;
-	spinlock_t fifo_lock;
+	int scancode_buf[40];
+	int scancode_count;
 	int flags_supported;
 	int flags_state;
 };
@@ -813,23 +811,14 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 	sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS);
 	device->driver_data = priv;
 
-	/* kfifo */
-	spin_lock_init(&priv->fifo_lock);
-	error = kfifo_alloc(&priv->fifo, RINGBUFFERSIZE * sizeof(int),
-			    GFP_KERNEL);
-	if (error) {
-		pr_err("kfifo_alloc failed\n");
-		goto err_stop;
-	}
-
 	error = acpi_fujitsu_laptop_input_setup(device);
 	if (error)
-		goto err_free_fifo;
+		return error;
 
 	error = acpi_bus_update_power(device->handle, &state);
 	if (error) {
 		pr_err("Error reading power state\n");
-		goto err_free_fifo;
+		return error;
 	}
 
 	pr_info("ACPI: %s [%s] (%s)\n",
@@ -877,62 +866,47 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 
 	error = acpi_fujitsu_laptop_leds_register(device);
 	if (error)
-		goto err_free_fifo;
+		return error;
 
 	error = fujitsu_laptop_platform_add(device);
 	if (error)
-		goto err_free_fifo;
+		return error;
 
 	return 0;
-
-err_free_fifo:
-	kfifo_free(&priv->fifo);
-err_stop:
-	return error;
 }
 
 static int acpi_fujitsu_laptop_remove(struct acpi_device *device)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
-
 	fujitsu_laptop_platform_remove(device);
 
-	kfifo_free(&priv->fifo);
-
 	return 0;
 }
 
 static void acpi_fujitsu_laptop_press(struct acpi_device *device, int scancode)
 {
 	struct fujitsu_laptop *priv = acpi_driver_data(device);
-	int status;
 
-	status = kfifo_in_locked(&priv->fifo, (unsigned char *)&scancode,
-				 sizeof(scancode), &priv->fifo_lock);
-	if (status != sizeof(scancode)) {
+	if (priv->scancode_count == ARRAY_SIZE(priv->scancode_buf)) {
 		vdbg_printk(FUJLAPTOP_DBG_WARN,
 			    "Could not push scancode [0x%x]\n", scancode);
 		return;
 	}
+	priv->scancode_buf[priv->scancode_count++] = scancode;
 	sparse_keymap_report_event(priv->input, scancode, 1, false);
 	vdbg_printk(FUJLAPTOP_DBG_TRACE,
-		    "Push scancode into ringbuffer [0x%x]\n", scancode);
+		    "Push scancode into buffer [0x%x]\n", scancode);
 }
 
 static void acpi_fujitsu_laptop_release(struct acpi_device *device)
 {
 	struct fujitsu_laptop *priv = acpi_driver_data(device);
-	int scancode, status;
-
-	while (true) {
-		status = kfifo_out_locked(&priv->fifo,
-					  (unsigned char *)&scancode,
-					  sizeof(scancode), &priv->fifo_lock);
-		if (status != sizeof(scancode))
-			return;
+	int scancode;
+
+	while (priv->scancode_count > 0) {
+		scancode = priv->scancode_buf[--priv->scancode_count];
 		sparse_keymap_report_event(priv->input, scancode, 0, false);
 		vdbg_printk(FUJLAPTOP_DBG_TRACE,
-			    "Pop scancode from ringbuffer [0x%x]\n", scancode);
+			    "Pop scancode from buffer [0x%x]\n", scancode);
 	}
 }
 
-- 
2.13.1

  reply	other threads:[~2017-06-16  4:40 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-16  4:40 [PATCH 0/7] fujitsu-laptop: ACPI-related cleanups Michał Kępień
2017-06-16  4:40 ` Michał Kępień [this message]
2017-06-21 18:15   ` [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for storing hotkey scancodes Darren Hart
2017-06-21 23:50     ` Jonathan Woithe
2017-06-22  2:44       ` Darren Hart
2017-06-22  3:01         ` Jonathan Woithe
2017-06-22 20:46           ` Michał Kępień
2017-06-22 23:58             ` Darren Hart
2017-06-23  0:14               ` Jonathan Woithe
2017-06-23  5:54                 ` Darren Hart
2017-06-22 20:08     ` Michał Kępień
2017-06-24  0:25     ` Rafael J. Wysocki
2017-06-27  0:07       ` Darren Hart
2017-06-27 12:16         ` Jonathan Woithe
2017-06-28  4:30         ` Michał Kępień
2017-06-28 16:03           ` Darren Hart
2017-06-16  4:40 ` [PATCH 2/7] platform/x86: fujitsu-laptop: remove redundant safety checks Michał Kępień
2017-06-16  4:40 ` [PATCH 3/7] platform/x86: fujitsu-laptop: use strcpy to set ACPI device names and classes Michał Kępień
2017-06-16  4:40 ` [PATCH 4/7] platform/x86: fujitsu-laptop: sanitize hotkey input device identification Michał Kępień
2017-06-16  4:40 ` [PATCH 5/7] platform/x86: fujitsu-laptop: do not update ACPI device power status Michał Kępień
2017-06-21 20:17   ` Darren Hart
2017-06-22 21:02     ` Michał Kępień
2017-06-22 23:58       ` Darren Hart
2017-06-23  0:16         ` Jonathan Woithe
2017-06-23  5:52           ` Darren Hart
2017-06-16  4:40 ` [PATCH 6/7] platform/x86: fujitsu-laptop: do not evaluate ACPI _INI methods Michał Kępień
2017-06-16  4:40 ` [PATCH 7/7] platform/x86: fujitsu-laptop: rework debugging Michał Kępień
2017-06-18 10:35 ` [PATCH 0/7] fujitsu-laptop: ACPI-related cleanups Jonathan Woithe
2017-06-22 23:57 ` Darren Hart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170616044058.30443-2-kernel@kempniu.pl \
    --to=kernel@kempniu.pl \
    --cc=andy@infradead.org \
    --cc=dvhart@infradead.org \
    --cc=jwoithe@just42.net \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.