All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely.
@ 2021-02-17 19:07 Ronald Tschalär
  2021-02-17 19:07 ` [PATCH 2/3] Input: applespi: Fix occasional crc errors under load Ronald Tschalär
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Ronald Tschalär @ 2021-02-17 19:07 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel

The response to a command may never arrive or it may be corrupted (and
hence dropped) for some reason. While exceedingly rare, when it did
happen it blocked all further commands. One way to fix this was to
do a suspend/resume. However, recovering automatically seems like a
nicer option. Hence this puts a time limit (1 sec) on how long we're
willing to wait for a response, after which we assume it got lost.

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 drivers/input/keyboard/applespi.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index d22223154177f..8494bf610fd70 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -48,6 +48,7 @@
 #include <linux/efi.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
+#include <linux/ktime.h>
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
@@ -409,7 +410,7 @@ struct applespi_data {
 	unsigned int			cmd_msg_cntr;
 	/* lock to protect the above parameters and flags below */
 	spinlock_t			cmd_msg_lock;
-	bool				cmd_msg_queued;
+	ktime_t				cmd_msg_queued;
 	enum applespi_evt_type		cmd_evt_type;
 
 	struct led_classdev		backlight_info;
@@ -729,7 +730,7 @@ static void applespi_msg_complete(struct applespi_data *applespi,
 		wake_up_all(&applespi->drain_complete);
 
 	if (is_write_msg) {
-		applespi->cmd_msg_queued = false;
+		applespi->cmd_msg_queued = 0;
 		applespi_send_cmd_msg(applespi);
 	}
 
@@ -771,8 +772,16 @@ static int applespi_send_cmd_msg(struct applespi_data *applespi)
 		return 0;
 
 	/* check whether send is in progress */
-	if (applespi->cmd_msg_queued)
-		return 0;
+	if (applespi->cmd_msg_queued) {
+		if (ktime_ms_delta(ktime_get(), applespi->cmd_msg_queued) < 1000)
+			return 0;
+
+		dev_warn(&applespi->spi->dev, "Command %d timed out\n",
+			 applespi->cmd_evt_type);
+
+		applespi->cmd_msg_queued = 0;
+		applespi->write_active = false;
+	}
 
 	/* set up packet */
 	memset(packet, 0, APPLESPI_PACKET_SIZE);
@@ -869,7 +878,7 @@ static int applespi_send_cmd_msg(struct applespi_data *applespi)
 		return sts;
 	}
 
-	applespi->cmd_msg_queued = true;
+	applespi->cmd_msg_queued = ktime_get();
 	applespi->write_active = true;
 
 	return 0;
@@ -1921,7 +1930,7 @@ static int __maybe_unused applespi_resume(struct device *dev)
 	applespi->drain = false;
 	applespi->have_cl_led_on = false;
 	applespi->have_bl_level = 0;
-	applespi->cmd_msg_queued = false;
+	applespi->cmd_msg_queued = 0;
 	applespi->read_active = false;
 	applespi->write_active = false;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] Input: applespi: Fix occasional crc errors under load.
  2021-02-17 19:07 [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely Ronald Tschalär
@ 2021-02-17 19:07 ` Ronald Tschalär
  2021-02-19 19:12   ` Dmitry Torokhov
  2021-02-17 19:07 ` [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing Ronald Tschalär
  2021-02-17 20:23 ` [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely Dmitry Torokhov
  2 siblings, 1 reply; 15+ messages in thread
From: Ronald Tschalär @ 2021-02-17 19:07 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel

For some reason, when the system is under heavy CPU load, the read
following the write sometimes occurs unusually quickly, resulting in
the read data not being quite ready and hence a bad packet getting read.
Adding another delay after reading the status message appears to fix
this.

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 drivers/input/keyboard/applespi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index 8494bf610fd70..f0a0067c48ff6 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -749,6 +749,8 @@ static void applespi_async_write_complete(void *context)
 					 applespi->tx_status,
 					 APPLESPI_STATUS_SIZE);
 
+	udelay(SPI_RW_CHG_DELAY_US);
+
 	if (!applespi_check_write_status(applespi, applespi->wr_m.status)) {
 		/*
 		 * If we got an error, we presumably won't get the expected
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
  2021-02-17 19:07 [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely Ronald Tschalär
  2021-02-17 19:07 ` [PATCH 2/3] Input: applespi: Fix occasional crc errors under load Ronald Tschalär
@ 2021-02-17 19:07 ` Ronald Tschalär
  2021-02-17 20:26   ` Dmitry Torokhov
                     ` (2 more replies)
  2021-02-17 20:23 ` [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely Dmitry Torokhov
  2 siblings, 3 replies; 15+ messages in thread
From: Ronald Tschalär @ 2021-02-17 19:07 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel

The problem is that tracing can't be set via sysfs until the module is
loaded, at which point the keyboard and trackpad initialization commands
have already been run and hence tracing can't be used to debug problems
here.

Adding this param allows tracing to be enabled for messages sent and
received during module probing. It takes comma-separated list of events,
e.g.

  trace_event=applespi_tp_ini_cmd,applespi_bad_crc

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 drivers/input/keyboard/applespi.c | 32 +++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index f0a0067c48ff6..03f9ad0f83967 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -53,6 +53,8 @@
 #include <linux/module.h>
 #include <linux/spinlock.h>
 #include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/trace_events.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 
@@ -110,6 +112,10 @@ module_param_string(touchpad_dimensions, touchpad_dimensions,
 		    sizeof(touchpad_dimensions), 0444);
 MODULE_PARM_DESC(touchpad_dimensions, "The pixel dimensions of the touchpad, as XxY+W+H .");
 
+static char *trace_event;
+module_param(trace_event, charp, 0444);
+MODULE_PARM_DESC(trace_event, "Enable early event tracing. It takes the form of a comma-separated list of events to enable.");
+
 /**
  * struct keyboard_protocol - keyboard message.
  * message.type = 0x0110, message.length = 0x000a
@@ -1645,6 +1651,30 @@ static void applespi_save_bl_level(struct applespi_data *applespi,
 			 "Error saving backlight level to EFI vars: %d\n", sts);
 }
 
+static void applespi_enable_early_event_tracing(struct device *dev)
+{
+	char *buf, *event;
+	int sts;
+
+	if (trace_event && trace_event[0]) {
+		buf = kstrdup(trace_event, GFP_KERNEL);
+		if (!buf)
+			return;
+
+		while ((event = strsep(&buf, ","))) {
+			if (event[0]) {
+				sts = trace_set_clr_event("applespi", event, 1);
+				if (sts)
+					dev_warn(dev,
+						 "Error setting trace event '%s': %d\n",
+						 event, sts);
+			}
+		}
+
+		kfree(buf);
+	}
+}
+
 static int applespi_probe(struct spi_device *spi)
 {
 	struct applespi_data *applespi;
@@ -1653,6 +1683,8 @@ static int applespi_probe(struct spi_device *spi)
 	int sts, i;
 	unsigned long long gpe, usb_status;
 
+	applespi_enable_early_event_tracing(&spi->dev);
+
 	/* check if the USB interface is present and enabled already */
 	acpi_sts = acpi_evaluate_integer(spi_handle, "UIST", NULL, &usb_status);
 	if (ACPI_SUCCESS(acpi_sts) && usb_status) {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely.
  2021-02-17 19:07 [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely Ronald Tschalär
  2021-02-17 19:07 ` [PATCH 2/3] Input: applespi: Fix occasional crc errors under load Ronald Tschalär
  2021-02-17 19:07 ` [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing Ronald Tschalär
@ 2021-02-17 20:23 ` Dmitry Torokhov
  2021-02-17 20:45   ` Life is hard, and then you die
  2 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2021-02-17 20:23 UTC (permalink / raw)
  To: Ronald Tschalär
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel

Hi Ronald,

On Wed, Feb 17, 2021 at 11:07:16AM -0800, Ronald Tschalär wrote:
> @@ -869,7 +878,7 @@ static int applespi_send_cmd_msg(struct applespi_data *applespi)
>  		return sts;
>  	}
>  
> -	applespi->cmd_msg_queued = true;
> +	applespi->cmd_msg_queued = ktime_get();

Will it be OK if I change this to ktime_get_coarse()? I do not think we
need exact time here, and the coarse variant is cheaper I believe.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
  2021-02-17 19:07 ` [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing Ronald Tschalär
@ 2021-02-17 20:26   ` Dmitry Torokhov
  2021-02-17 20:52     ` Life is hard, and then you die
  2021-02-18  0:18     ` kernel test robot
  2021-02-18  4:41     ` kernel test robot
  2 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2021-02-17 20:26 UTC (permalink / raw)
  To: Ronald Tschalär
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel

Hi Ronald,

On Wed, Feb 17, 2021 at 11:07:18AM -0800, Ronald Tschalär wrote:
> The problem is that tracing can't be set via sysfs until the module is
> loaded, at which point the keyboard and trackpad initialization commands
> have already been run and hence tracing can't be used to debug problems
> here.
> 
> Adding this param allows tracing to be enabled for messages sent and
> received during module probing. It takes comma-separated list of events,
> e.g.
> 
>   trace_event=applespi_tp_ini_cmd,applespi_bad_crc

You can unbind and rebind a device to a driver via sysfs as many times
as needed (see bind and unbind driver sysfs attributes), so I believe

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely.
  2021-02-17 20:23 ` [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely Dmitry Torokhov
@ 2021-02-17 20:45   ` Life is hard, and then you die
  2021-02-19 19:12     ` Dmitry Torokhov
  0 siblings, 1 reply; 15+ messages in thread
From: Life is hard, and then you die @ 2021-02-17 20:45 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel


  Hi Dmitry,

On Wed, Feb 17, 2021 at 12:23:23PM -0800, Dmitry Torokhov wrote:
> 
> On Wed, Feb 17, 2021 at 11:07:16AM -0800, Ronald Tschalär wrote:
> > @@ -869,7 +878,7 @@ static int applespi_send_cmd_msg(struct applespi_data *applespi)
> >  		return sts;
> >  	}
> >  
> > -	applespi->cmd_msg_queued = true;
> > +	applespi->cmd_msg_queued = ktime_get();
> 
> Will it be OK if I change this to ktime_get_coarse()? I do not think we
> need exact time here, and the coarse variant is cheaper I believe.

Certainly - I just wasn't aware of the coarse variant.


  Cheers,

  Ronald


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
  2021-02-17 20:26   ` Dmitry Torokhov
@ 2021-02-17 20:52     ` Life is hard, and then you die
  2021-02-17 21:06       ` Dmitry Torokhov
  0 siblings, 1 reply; 15+ messages in thread
From: Life is hard, and then you die @ 2021-02-17 20:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel


  Hi Dmitry,

On Wed, Feb 17, 2021 at 12:26:18PM -0800, Dmitry Torokhov wrote:
> 
> On Wed, Feb 17, 2021 at 11:07:18AM -0800, Ronald Tschalär wrote:
> > The problem is that tracing can't be set via sysfs until the module is
> > loaded, at which point the keyboard and trackpad initialization commands
> > have already been run and hence tracing can't be used to debug problems
> > here.
> > 
> > Adding this param allows tracing to be enabled for messages sent and
> > received during module probing. It takes comma-separated list of events,
> > e.g.
> > 
> >   trace_event=applespi_tp_ini_cmd,applespi_bad_crc
> 
> You can unbind and rebind a device to a driver via sysfs as many times
> as needed (see bind and unbind driver sysfs attributes), so I believe

Hmm, I'm going to have to play with that a bit, but one place it still
won't help I think is something we ran into in practise: init was
failing during boot, but was successfull later on.


  Cheers,

  Ronald


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
  2021-02-17 20:52     ` Life is hard, and then you die
@ 2021-02-17 21:06       ` Dmitry Torokhov
  2021-02-17 22:34         ` Life is hard, and then you die
  0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2021-02-17 21:06 UTC (permalink / raw)
  To: Life is hard, and then you die
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel

On Wed, Feb 17, 2021 at 12:52:57PM -0800, Life is hard, and then you die wrote:
> 
>   Hi Dmitry,
> 
> On Wed, Feb 17, 2021 at 12:26:18PM -0800, Dmitry Torokhov wrote:
> > 
> > On Wed, Feb 17, 2021 at 11:07:18AM -0800, Ronald Tschalär wrote:
> > > The problem is that tracing can't be set via sysfs until the module is
> > > loaded, at which point the keyboard and trackpad initialization commands
> > > have already been run and hence tracing can't be used to debug problems
> > > here.
> > > 
> > > Adding this param allows tracing to be enabled for messages sent and
> > > received during module probing. It takes comma-separated list of events,
> > > e.g.
> > > 
> > >   trace_event=applespi_tp_ini_cmd,applespi_bad_crc
> > 
> > You can unbind and rebind a device to a driver via sysfs as many times
> > as needed (see bind and unbind driver sysfs attributes), so I believe
> 
> Hmm, I'm going to have to play with that a bit, but one place it still
> won't help I think is something we ran into in practise: init was
> failing during boot, but was successfull later on.

Maybe compiling module as a built-in and then using kernel command line
option to initiate the trace would work?

If this facility is really needed, it would be beneficial for other
modules as well, and thus better implemented in the module loading code
to activate desired tracing after loading the module but before invoking
module init code.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
  2021-02-17 21:06       ` Dmitry Torokhov
@ 2021-02-17 22:34         ` Life is hard, and then you die
  0 siblings, 0 replies; 15+ messages in thread
From: Life is hard, and then you die @ 2021-02-17 22:34 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel


  Hi Dmitry,

On Wed, Feb 17, 2021 at 01:06:27PM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 17, 2021 at 12:52:57PM -0800, Life is hard, and then you die wrote:
> > 
> > On Wed, Feb 17, 2021 at 12:26:18PM -0800, Dmitry Torokhov wrote:
> > > 
> > > On Wed, Feb 17, 2021 at 11:07:18AM -0800, Ronald Tschalär wrote:
> > > > The problem is that tracing can't be set via sysfs until the module is
> > > > loaded, at which point the keyboard and trackpad initialization commands
> > > > have already been run and hence tracing can't be used to debug problems
> > > > here.
> > > > 
> > > > Adding this param allows tracing to be enabled for messages sent and
> > > > received during module probing. It takes comma-separated list of events,
> > > > e.g.
> > > > 
> > > >   trace_event=applespi_tp_ini_cmd,applespi_bad_crc
> > > 
> > > You can unbind and rebind a device to a driver via sysfs as many times
> > > as needed (see bind and unbind driver sysfs attributes), so I believe

Ok yes, that works well, except for the boot-debug scenario.

> > Hmm, I'm going to have to play with that a bit, but one place it still
> > won't help I think is something we ran into in practise: init was
> > failing during boot, but was successfull later on.
> 
> Maybe compiling module as a built-in and then using kernel command line
> option to initiate the trace would work?

My personal issue with this is the fact that most folks reporting
issues are running their distro's standard kernel, which invariably
has this (and most others) compiled as a loadable module; and asking
folks to rebuild their kernel is actually quite a hurdle for them, in
particular compared to asking them to just add some boot params or
manipulating some sysfs entries. So I prefer to try to provide easy
ways for folks to be able to generate and report info back that work
and are enabled out-of-the-box on most distros.

> If this facility is really needed, it would be beneficial for other
> modules as well, and thus better implemented in the module loading code
> to activate desired tracing after loading the module but before invoking
> module init code.

I don't know if it rises to the level of "really needed" - I certainly
needed something like this to debug an issue and hence the module
param. And I figured if somebody adds/debugs additional init commands
they could find it useful too. But this may not be commonly needed
after all, or folks are using some other solution.

If there's interest, I might be able to take a stab a this in the near
future, but not sure.


  Cheers,

  Ronald


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
  2021-02-17 19:07 ` [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing Ronald Tschalär
@ 2021-02-18  0:18     ` kernel test robot
  2021-02-18  0:18     ` kernel test robot
  2021-02-18  4:41     ` kernel test robot
  2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-02-18  0:18 UTC (permalink / raw)
  To: Ronald Tschalär, Dmitry Torokhov
  Cc: kbuild-all, Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones,
	linux-input, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2362 bytes --]

Hi "Ronald,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on input/next]
[also build test ERROR on v5.11 next-20210217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ronald-Tschal-r/Input-applespi-Don-t-wait-for-responses-to-commands-indefinitely/20210218-032205
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-s001-20210217 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-215-g0fb77bb6-dirty
        # https://github.com/0day-ci/linux/commit/838cd23e96675132bdd30878d1b24a59b8a534e1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ronald-Tschal-r/Input-applespi-Don-t-wait-for-responses-to-commands-indefinitely/20210218-032205
        git checkout 838cd23e96675132bdd30878d1b24a59b8a534e1
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/input/keyboard/applespi.o: in function `applespi_enable_early_event_tracing':
>> drivers/input/keyboard/applespi.c:1666: undefined reference to `trace_set_clr_event'


vim +1666 drivers/input/keyboard/applespi.c

  1653	
  1654	static void applespi_enable_early_event_tracing(struct device *dev)
  1655	{
  1656		char *buf, *event;
  1657		int sts;
  1658	
  1659		if (trace_event && trace_event[0]) {
  1660			buf = kstrdup(trace_event, GFP_KERNEL);
  1661			if (!buf)
  1662				return;
  1663	
  1664			while ((event = strsep(&buf, ","))) {
  1665				if (event[0]) {
> 1666					sts = trace_set_clr_event("applespi", event, 1);
  1667					if (sts)
  1668						dev_warn(dev,
  1669							 "Error setting trace event '%s': %d\n",
  1670							 event, sts);
  1671				}
  1672			}
  1673	
  1674			kfree(buf);
  1675		}
  1676	}
  1677	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29308 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
@ 2021-02-18  0:18     ` kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-02-18  0:18 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2428 bytes --]

Hi "Ronald,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on input/next]
[also build test ERROR on v5.11 next-20210217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ronald-Tschal-r/Input-applespi-Don-t-wait-for-responses-to-commands-indefinitely/20210218-032205
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-s001-20210217 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-215-g0fb77bb6-dirty
        # https://github.com/0day-ci/linux/commit/838cd23e96675132bdd30878d1b24a59b8a534e1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ronald-Tschal-r/Input-applespi-Don-t-wait-for-responses-to-commands-indefinitely/20210218-032205
        git checkout 838cd23e96675132bdd30878d1b24a59b8a534e1
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/input/keyboard/applespi.o: in function `applespi_enable_early_event_tracing':
>> drivers/input/keyboard/applespi.c:1666: undefined reference to `trace_set_clr_event'


vim +1666 drivers/input/keyboard/applespi.c

  1653	
  1654	static void applespi_enable_early_event_tracing(struct device *dev)
  1655	{
  1656		char *buf, *event;
  1657		int sts;
  1658	
  1659		if (trace_event && trace_event[0]) {
  1660			buf = kstrdup(trace_event, GFP_KERNEL);
  1661			if (!buf)
  1662				return;
  1663	
  1664			while ((event = strsep(&buf, ","))) {
  1665				if (event[0]) {
> 1666					sts = trace_set_clr_event("applespi", event, 1);
  1667					if (sts)
  1668						dev_warn(dev,
  1669							 "Error setting trace event '%s': %d\n",
  1670							 event, sts);
  1671				}
  1672			}
  1673	
  1674			kfree(buf);
  1675		}
  1676	}
  1677	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29308 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
  2021-02-17 19:07 ` [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing Ronald Tschalär
@ 2021-02-18  4:41     ` kernel test robot
  2021-02-18  0:18     ` kernel test robot
  2021-02-18  4:41     ` kernel test robot
  2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-02-18  4:41 UTC (permalink / raw)
  To: Ronald Tschalär, Dmitry Torokhov
  Cc: kbuild-all, Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones,
	linux-input, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2103 bytes --]

Hi "Ronald,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on input/next]
[also build test ERROR on v5.11 next-20210217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ronald-Tschal-r/Input-applespi-Don-t-wait-for-responses-to-commands-indefinitely/20210218-032205
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/838cd23e96675132bdd30878d1b24a59b8a534e1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ronald-Tschal-r/Input-applespi-Don-t-wait-for-responses-to-commands-indefinitely/20210218-032205
        git checkout 838cd23e96675132bdd30878d1b24a59b8a534e1
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ia64-linux-ld: drivers/input/keyboard/applespi.o: in function `applespi_probe':
>> applespi.c:(.text+0xf22): undefined reference to `trace_set_clr_event'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66945 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.
@ 2021-02-18  4:41     ` kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-02-18  4:41 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2146 bytes --]

Hi "Ronald,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on input/next]
[also build test ERROR on v5.11 next-20210217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ronald-Tschal-r/Input-applespi-Don-t-wait-for-responses-to-commands-indefinitely/20210218-032205
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/838cd23e96675132bdd30878d1b24a59b8a534e1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ronald-Tschal-r/Input-applespi-Don-t-wait-for-responses-to-commands-indefinitely/20210218-032205
        git checkout 838cd23e96675132bdd30878d1b24a59b8a534e1
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ia64-linux-ld: drivers/input/keyboard/applespi.o: in function `applespi_probe':
>> applespi.c:(.text+0xf22): undefined reference to `trace_set_clr_event'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 66945 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely.
  2021-02-17 20:45   ` Life is hard, and then you die
@ 2021-02-19 19:12     ` Dmitry Torokhov
  0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2021-02-19 19:12 UTC (permalink / raw)
  To: Life is hard, and then you die
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel

On Wed, Feb 17, 2021 at 12:45:51PM -0800, Life is hard, and then you die wrote:
> 
>   Hi Dmitry,
> 
> On Wed, Feb 17, 2021 at 12:23:23PM -0800, Dmitry Torokhov wrote:
> > 
> > On Wed, Feb 17, 2021 at 11:07:16AM -0800, Ronald Tschalär wrote:
> > > @@ -869,7 +878,7 @@ static int applespi_send_cmd_msg(struct applespi_data *applespi)
> > >  		return sts;
> > >  	}
> > >  
> > > -	applespi->cmd_msg_queued = true;
> > > +	applespi->cmd_msg_queued = ktime_get();
> > 
> > Will it be OK if I change this to ktime_get_coarse()? I do not think we
> > need exact time here, and the coarse variant is cheaper I believe.
> 
> Certainly - I just wasn't aware of the coarse variant.

Applied, thank you.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] Input: applespi: Fix occasional crc errors under load.
  2021-02-17 19:07 ` [PATCH 2/3] Input: applespi: Fix occasional crc errors under load Ronald Tschalär
@ 2021-02-19 19:12   ` Dmitry Torokhov
  0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2021-02-19 19:12 UTC (permalink / raw)
  To: Ronald Tschalär
  Cc: Gustavo A. R. Silva, Sergiu Cuciurean, Lee Jones, linux-input,
	linux-kernel

On Wed, Feb 17, 2021 at 11:07:17AM -0800, Ronald Tschalär wrote:
> For some reason, when the system is under heavy CPU load, the read
> following the write sometimes occurs unusually quickly, resulting in
> the read data not being quite ready and hence a bad packet getting read.
> Adding another delay after reading the status message appears to fix
> this.
> 
> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-02-19 19:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-17 19:07 [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely Ronald Tschalär
2021-02-17 19:07 ` [PATCH 2/3] Input: applespi: Fix occasional crc errors under load Ronald Tschalär
2021-02-19 19:12   ` Dmitry Torokhov
2021-02-17 19:07 ` [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing Ronald Tschalär
2021-02-17 20:26   ` Dmitry Torokhov
2021-02-17 20:52     ` Life is hard, and then you die
2021-02-17 21:06       ` Dmitry Torokhov
2021-02-17 22:34         ` Life is hard, and then you die
2021-02-18  0:18   ` kernel test robot
2021-02-18  0:18     ` kernel test robot
2021-02-18  4:41   ` kernel test robot
2021-02-18  4:41     ` kernel test robot
2021-02-17 20:23 ` [PATCH 1/3] Input: applespi: Don't wait for responses to commands indefinitely Dmitry Torokhov
2021-02-17 20:45   ` Life is hard, and then you die
2021-02-19 19:12     ` Dmitry Torokhov

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.