All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] iio: cros_ec: Trace EC sensors command
@ 2025-01-17 23:01 Gwendal Grignou
  2025-01-18 16:33 ` Jonathan Cameron
  0 siblings, 1 reply; 6+ messages in thread
From: Gwendal Grignou @ 2025-01-17 23:01 UTC (permalink / raw)
  To: jic23; +Cc: tzungbi, linux-iio, Gwendal Grignou

For debugging, add tracing for EC_CMD_MOTION_SENSE_CMD command:
- decode the name of the subcommand
- provide internal information for the most common sub-commands:
  setting range, frequency, EC probing frequency, ...
- display return status.

When enabled, the tracing output is similar to:
/sys/kernel/debug/tracing # echo 1 > events/cros_ec/enable ; echo 1 > tracing_on ; cat trace_pipe | grep MOTIONSENSE_CMD_SENSOR_ODR
 SensorDeviceImp-814     [003] .....   686.176782: cros_ec_motion_host_cmd: MOTIONSENSE_CMD_SENSOR_ODR, id: 1, data: 200000, result: 4, return: 12500

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes from v2 (https://patchwork.kernel.org/patch/13942819/):
- Improve casting
- Add intent of the patch in commit message.
Changes from v1 (https://patchwork.kernel.org/patch/13942368/):
- fix merging issue with iio.git/togreg.

 drivers/iio/common/cros_ec_sensors/Makefile   |  3 +-
 .../cros_ec_sensors/cros_ec_sensors_core.c    |  9 ++-
 .../cros_ec_sensors/cros_ec_sensors_trace.c   | 32 +++++++++++
 .../cros_ec_sensors/cros_ec_sensors_trace.h   | 56 +++++++++++++++++++
 4 files changed, 96 insertions(+), 4 deletions(-)
 create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c
 create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h

diff --git a/drivers/iio/common/cros_ec_sensors/Makefile b/drivers/iio/common/cros_ec_sensors/Makefile
index e0a33ab66d21a..c358fa0328abd 100644
--- a/drivers/iio/common/cros_ec_sensors/Makefile
+++ b/drivers/iio/common/cros_ec_sensors/Makefile
@@ -3,6 +3,7 @@
 # Makefile for sensors seen through the ChromeOS EC sensor hub.
 #
 
-obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
+cros-ec-sensors-core-objs += cros_ec_sensors_core.o cros_ec_sensors_trace.o
+obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros-ec-sensors-core.o
 obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
 obj-$(CONFIG_IIO_CROS_EC_SENSORS_LID_ANGLE) += cros_ec_lid_angle.o
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 9fc71a73caa12..7751d6f69b124 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -23,6 +23,8 @@
 #include <linux/platform_data/cros_ec_sensorhub.h>
 #include <linux/platform_device.h>
 
+#include "cros_ec_sensors_trace.h"
+
 /*
  * Hard coded to the first device to support sensor fifo.  The EC has a 2048
  * byte fifo and will trigger an interrupt when fifo is 2/3 full.
@@ -413,6 +415,7 @@ EXPORT_SYMBOL_GPL(cros_ec_sensors_core_register);
 int cros_ec_motion_send_host_cmd(struct cros_ec_sensors_core_state *state,
 				 u16 opt_length)
 {
+	struct ec_response_motion_sense *resp = (struct ec_response_motion_sense *)state->msg->data;
 	int ret;
 
 	if (opt_length)
@@ -423,12 +426,12 @@ int cros_ec_motion_send_host_cmd(struct cros_ec_sensors_core_state *state,
 	memcpy(state->msg->data, &state->param, sizeof(state->param));
 
 	ret = cros_ec_cmd_xfer_status(state->ec, state->msg);
+	trace_cros_ec_motion_host_cmd(&state->param, resp, ret);
 	if (ret < 0)
 		return ret;
 
-	if (ret &&
-	    state->resp != (struct ec_response_motion_sense *)state->msg->data)
-		memcpy(state->resp, state->msg->data, ret);
+	if (ret && state->resp != resp)
+		memcpy(state->resp, resp, ret);
 
 	return 0;
 }
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c
new file mode 100644
index 0000000000000..c4db949fa7750
--- /dev/null
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0
+// Trace events for the ChromeOS Embedded Controller
+//
+// Copyright 2025 Google LLC.
+
+#define TRACE_SYMBOL(a) {a, #a}
+
+// Generate the list using the following script:
+// sed -n 's/^.*\(MOTIONSENSE_CMD.*\) = .*,$/\tTRACE_SYMBOL(\1), \\/p' include/linux/platform_data/cros_ec_commands.h
+#define MOTIONSENSE_CMDS \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_DUMP), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_INFO), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_EC_RATE), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_SENSOR_ODR), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_SENSOR_RANGE), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_KB_WAKE_ANGLE), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_DATA), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_FIFO_INFO), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_FIFO_FLUSH), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_FIFO_READ), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_PERFORM_CALIB), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_SENSOR_OFFSET), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_LIST_ACTIVITIES), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_SET_ACTIVITY), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_LID_ANGLE), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_FIFO_INT_ENABLE), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_SPOOF), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_TABLET_MODE_LID_ANGLE), \
+	TRACE_SYMBOL(MOTIONSENSE_CMD_SENSOR_SCALE)
+
+#define CREATE_TRACE_POINTS
+#include "cros_ec_sensors_trace.h"
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h
new file mode 100644
index 0000000000000..61853e410e96c
--- /dev/null
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Trace events for the ChromeOS Embedded Controller
+ *
+ * Copyright 2025 Google LLC.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM cros_ec
+
+#if !defined(_CROS_EC_SENSORS_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
+#define _CROS_EC_SENSORS_TRACE_H_
+
+#include <linux/bits.h>
+#include <linux/types.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(cros_ec_motion_host_cmd,
+	    TP_PROTO(struct ec_params_motion_sense *param,
+		     struct ec_response_motion_sense *resp,
+		     int retval),
+	    TP_ARGS(param, resp, retval),
+	    TP_STRUCT__entry(__field(uint8_t, cmd)
+			     __field(uint8_t, sensor_id)
+			     __field(uint32_t, data)
+			     __field(int, retval)
+			     __field(int32_t, ret)
+	    ),
+	    TP_fast_assign(__entry->cmd = param->cmd;
+			   __entry->sensor_id = param->sensor_odr.sensor_num;
+			   __entry->data = param->sensor_odr.data;
+			   __entry->retval = retval;
+			   __entry->ret = retval > 0 ? resp->sensor_odr.ret : -1;
+	    ),
+	    TP_printk("%s, id: %d, data: %u, result: %u, return: %d",
+		      __print_symbolic(__entry->cmd, MOTIONSENSE_CMDS),
+		      __entry->sensor_id,
+		      __entry->data,
+		      __entry->retval,
+		      __entry->ret)
+);
+
+#endif /* _CROS_EC_SENSORS_TRACE_H_ */
+
+/* this part must be outside header guard */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE cros_ec_sensors_trace
+
+#include <trace/define_trace.h>
-- 
2.48.0.rc2.279.g1de40edade-goog


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

* Re: [PATCH v3] iio: cros_ec: Trace EC sensors command
  2025-01-17 23:01 [PATCH v3] iio: cros_ec: Trace EC sensors command Gwendal Grignou
@ 2025-01-18 16:33 ` Jonathan Cameron
  2025-01-20  8:50   ` Tzung-Bi Shih
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2025-01-18 16:33 UTC (permalink / raw)
  To: Gwendal Grignou; +Cc: tzungbi, linux-iio

On Fri, 17 Jan 2025 15:01:36 -0800
Gwendal Grignou <gwendal@chromium.org> wrote:

> For debugging, add tracing for EC_CMD_MOTION_SENSE_CMD command:
> - decode the name of the subcommand
> - provide internal information for the most common sub-commands:
>   setting range, frequency, EC probing frequency, ...
> - display return status.
> 
> When enabled, the tracing output is similar to:
> /sys/kernel/debug/tracing # echo 1 > events/cros_ec/enable ; echo 1 > tracing_on ; cat trace_pipe | grep MOTIONSENSE_CMD_SENSOR_ODR
>  SensorDeviceImp-814     [003] .....   686.176782: cros_ec_motion_host_cmd: MOTIONSENSE_CMD_SENSOR_ODR, id: 1, data: 200000, result: 4, return: 12500
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

I briefly applied this, but...

[jic23@jic23-huawei iio]$ make LOCALVERSION= W=1 -j12  C=1
mkdir -p /home/jic23/src/kernel/iio/tools/objtool && make O=/home/jic23/src/kernel/iio subdir=tools/objtool --no-print-directory -C objtool 
  CALL    scripts/checksyscalls.sh
  INSTALL libsubcmd_headers
  CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.o
  CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.o
In file included from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h:56,
                 from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c:32:
./include/trace/define_trace.h:106:42: fatal error: ./cros_ec_sensors_trace.h: No such file or directory
  106 | #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
      |                                          ^

Despite dealing with trace points a lot in the day job
I still find them hard to actually add to a subsystem because
of fun things like this one. 

Jonathan

> ---
> Changes from v2 (https://patchwork.kernel.org/patch/13942819/):
> - Improve casting
> - Add intent of the patch in commit message.
> Changes from v1 (https://patchwork.kernel.org/patch/13942368/):
> - fix merging issue with iio.git/togreg.
> 
>  drivers/iio/common/cros_ec_sensors/Makefile   |  3 +-
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |  9 ++-
>  .../cros_ec_sensors/cros_ec_sensors_trace.c   | 32 +++++++++++
>  .../cros_ec_sensors/cros_ec_sensors_trace.h   | 56 +++++++++++++++++++
>  4 files changed, 96 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile b/drivers/iio/common/cros_ec_sensors/Makefile
> index e0a33ab66d21a..c358fa0328abd 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -3,6 +3,7 @@
>  # Makefile for sensors seen through the ChromeOS EC sensor hub.
>  #
>  
> -obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
> +cros-ec-sensors-core-objs += cros_ec_sensors_core.o cros_ec_sensors_trace.o
> +obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros-ec-sensors-core.o
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS_LID_ANGLE) += cros_ec_lid_angle.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 9fc71a73caa12..7751d6f69b124 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -23,6 +23,8 @@
>  #include <linux/platform_data/cros_ec_sensorhub.h>
>  #include <linux/platform_device.h>
>  
> +#include "cros_ec_sensors_trace.h"
> +
>  /*
>   * Hard coded to the first device to support sensor fifo.  The EC has a 2048
>   * byte fifo and will trigger an interrupt when fifo is 2/3 full.
> @@ -413,6 +415,7 @@ EXPORT_SYMBOL_GPL(cros_ec_sensors_core_register);
>  int cros_ec_motion_send_host_cmd(struct cros_ec_sensors_core_state *state,
>  				 u16 opt_length)
>  {
> +	struct ec_response_motion_sense *resp = (struct ec_response_motion_sense *)state->msg->data;
>  	int ret;
>  
>  	if (opt_length)
> @@ -423,12 +426,12 @@ int cros_ec_motion_send_host_cmd(struct cros_ec_sensors_core_state *state,
>  	memcpy(state->msg->data, &state->param, sizeof(state->param));
>  
>  	ret = cros_ec_cmd_xfer_status(state->ec, state->msg);
> +	trace_cros_ec_motion_host_cmd(&state->param, resp, ret);
>  	if (ret < 0)
>  		return ret;
>  
> -	if (ret &&
> -	    state->resp != (struct ec_response_motion_sense *)state->msg->data)
> -		memcpy(state->resp, state->msg->data, ret);
> +	if (ret && state->resp != resp)
> +		memcpy(state->resp, resp, ret);
>  
>  	return 0;
>  }
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c
> new file mode 100644
> index 0000000000000..c4db949fa7750
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Trace events for the ChromeOS Embedded Controller
> +//
> +// Copyright 2025 Google LLC.
> +
> +#define TRACE_SYMBOL(a) {a, #a}
> +
> +// Generate the list using the following script:
> +// sed -n 's/^.*\(MOTIONSENSE_CMD.*\) = .*,$/\tTRACE_SYMBOL(\1), \\/p' include/linux/platform_data/cros_ec_commands.h
> +#define MOTIONSENSE_CMDS \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_DUMP), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_INFO), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_EC_RATE), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_SENSOR_ODR), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_SENSOR_RANGE), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_KB_WAKE_ANGLE), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_DATA), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_FIFO_INFO), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_FIFO_FLUSH), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_FIFO_READ), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_PERFORM_CALIB), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_SENSOR_OFFSET), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_LIST_ACTIVITIES), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_SET_ACTIVITY), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_LID_ANGLE), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_FIFO_INT_ENABLE), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_SPOOF), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_TABLET_MODE_LID_ANGLE), \
> +	TRACE_SYMBOL(MOTIONSENSE_CMD_SENSOR_SCALE)
> +
> +#define CREATE_TRACE_POINTS
> +#include "cros_ec_sensors_trace.h"
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h
> new file mode 100644
> index 0000000000000..61853e410e96c
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Trace events for the ChromeOS Embedded Controller
> + *
> + * Copyright 2025 Google LLC.
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM cros_ec
> +
> +#if !defined(_CROS_EC_SENSORS_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
> +#define _CROS_EC_SENSORS_TRACE_H_
> +
> +#include <linux/bits.h>
> +#include <linux/types.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(cros_ec_motion_host_cmd,
> +	    TP_PROTO(struct ec_params_motion_sense *param,
> +		     struct ec_response_motion_sense *resp,
> +		     int retval),
> +	    TP_ARGS(param, resp, retval),
> +	    TP_STRUCT__entry(__field(uint8_t, cmd)
> +			     __field(uint8_t, sensor_id)
> +			     __field(uint32_t, data)
> +			     __field(int, retval)
> +			     __field(int32_t, ret)
> +	    ),
> +	    TP_fast_assign(__entry->cmd = param->cmd;
> +			   __entry->sensor_id = param->sensor_odr.sensor_num;
> +			   __entry->data = param->sensor_odr.data;
> +			   __entry->retval = retval;
> +			   __entry->ret = retval > 0 ? resp->sensor_odr.ret : -1;
> +	    ),
> +	    TP_printk("%s, id: %d, data: %u, result: %u, return: %d",
> +		      __print_symbolic(__entry->cmd, MOTIONSENSE_CMDS),
> +		      __entry->sensor_id,
> +		      __entry->data,
> +		      __entry->retval,
> +		      __entry->ret)
> +);
> +
> +#endif /* _CROS_EC_SENSORS_TRACE_H_ */
> +
> +/* this part must be outside header guard */
> +
> +#undef TRACE_INCLUDE_PATH
> +#define TRACE_INCLUDE_PATH .
> +
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE cros_ec_sensors_trace
> +
> +#include <trace/define_trace.h>


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

* Re: [PATCH v3] iio: cros_ec: Trace EC sensors command
  2025-01-18 16:33 ` Jonathan Cameron
@ 2025-01-20  8:50   ` Tzung-Bi Shih
  2025-01-21 21:54     ` Gwendal Grignou
  0 siblings, 1 reply; 6+ messages in thread
From: Tzung-Bi Shih @ 2025-01-20  8:50 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Gwendal Grignou, linux-iio

On Sat, Jan 18, 2025 at 04:33:52PM +0000, Jonathan Cameron wrote:
> I briefly applied this, but...
> 
> [jic23@jic23-huawei iio]$ make LOCALVERSION= W=1 -j12  C=1
> mkdir -p /home/jic23/src/kernel/iio/tools/objtool && make O=/home/jic23/src/kernel/iio subdir=tools/objtool --no-print-directory -C objtool 
>   CALL    scripts/checksyscalls.sh
>   INSTALL libsubcmd_headers
>   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.o
>   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.o
> In file included from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h:56,
>                  from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c:32:
> ./include/trace/define_trace.h:106:42: fatal error: ./cros_ec_sensors_trace.h: No such file or directory
>   106 | #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>       |                                          ^
> 
> Despite dealing with trace points a lot in the day job
> I still find them hard to actually add to a subsystem because
> of fun things like this one. 

Weird, I did the same test without seeing the error:

$ make O=build_dir W=1 allmodconfig \
        drivers/iio/common/cros_ec_sensors/cros-ec-sensors-core.o
...
  CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.o
  CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.o
  LD [M]  drivers/iio/common/cros_ec_sensors/cros-ec-sensors-core.o

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

* Re: [PATCH v3] iio: cros_ec: Trace EC sensors command
  2025-01-20  8:50   ` Tzung-Bi Shih
@ 2025-01-21 21:54     ` Gwendal Grignou
  2025-01-22  6:06       ` Tzung-Bi Shih
  0 siblings, 1 reply; 6+ messages in thread
From: Gwendal Grignou @ 2025-01-21 21:54 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: Jonathan Cameron, linux-iio

On Mon, Jan 20, 2025 at 12:50 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Sat, Jan 18, 2025 at 04:33:52PM +0000, Jonathan Cameron wrote:
> > I briefly applied this, but...
> >
> > [jic23@jic23-huawei iio]$ make LOCALVERSION= W=1 -j12  C=1
> > mkdir -p /home/jic23/src/kernel/iio/tools/objtool && make O=/home/jic23/src/kernel/iio subdir=tools/objtool --no-print-directory -C objtool
> >   CALL    scripts/checksyscalls.sh
> >   INSTALL libsubcmd_headers
> >   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.o
> >   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.o
> > In file included from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h:56,
> >                  from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c:32:
> > ./include/trace/define_trace.h:106:42: fatal error: ./cros_ec_sensors_trace.h: No such file or directory
> >   106 | #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
> >       |                                          ^
> >
> > Despite dealing with trace points a lot in the day job
> > I still find them hard to actually add to a subsystem because
> > of fun things like this one.
I reproduced this error on a fresh kernel source tree. I suspect an
issue with sparse, as other trace.c files use __CHECKER__ macros. For
instance `drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c`:

 10 /* sparse doesn't like tracepoint macros */
 11 #ifndef __CHECKER__
...

Will test more and send a V4.
Gwendal.
>
> Weird, I did the same test without seeing the error:
>
> $ make O=build_dir W=1 allmodconfig \
>         drivers/iio/common/cros_ec_sensors/cros-ec-sensors-core.o
> ...
>   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.o
>   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.o
>   LD [M]  drivers/iio/common/cros_ec_sensors/cros-ec-sensors-core.o

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

* Re: [PATCH v3] iio: cros_ec: Trace EC sensors command
  2025-01-21 21:54     ` Gwendal Grignou
@ 2025-01-22  6:06       ` Tzung-Bi Shih
  2025-01-22 19:03         ` Gwendal Grignou
  0 siblings, 1 reply; 6+ messages in thread
From: Tzung-Bi Shih @ 2025-01-22  6:06 UTC (permalink / raw)
  To: Gwendal Grignou; +Cc: Jonathan Cameron, linux-iio

On Tue, Jan 21, 2025 at 01:54:31PM -0800, Gwendal Grignou wrote:
> On Mon, Jan 20, 2025 at 12:50 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On Sat, Jan 18, 2025 at 04:33:52PM +0000, Jonathan Cameron wrote:
> > > I briefly applied this, but...
> > >
> > > [jic23@jic23-huawei iio]$ make LOCALVERSION= W=1 -j12  C=1
> > > mkdir -p /home/jic23/src/kernel/iio/tools/objtool && make O=/home/jic23/src/kernel/iio subdir=tools/objtool --no-print-directory -C objtool
> > >   CALL    scripts/checksyscalls.sh
> > >   INSTALL libsubcmd_headers
> > >   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.o
> > >   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.o
> > > In file included from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h:56,
> > >                  from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c:32:
> > > ./include/trace/define_trace.h:106:42: fatal error: ./cros_ec_sensors_trace.h: No such file or directory
> > >   106 | #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
> > >       |                                          ^
> > >
> > > Despite dealing with trace points a lot in the day job
> > > I still find them hard to actually add to a subsystem because
> > > of fun things like this one.
> I reproduced this error on a fresh kernel source tree. I suspect an
> issue with sparse, as other trace.c files use __CHECKER__ macros. For
> instance `drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c`:
> 
>  10 /* sparse doesn't like tracepoint macros */
>  11 #ifndef __CHECKER__
> ...

I still can't reproduce the error by using a just-cloned kernel source tree
with C=1.  Could you provide some more details about your steps?  I'm
wondering about which steps I may miss.

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

* Re: [PATCH v3] iio: cros_ec: Trace EC sensors command
  2025-01-22  6:06       ` Tzung-Bi Shih
@ 2025-01-22 19:03         ` Gwendal Grignou
  0 siblings, 0 replies; 6+ messages in thread
From: Gwendal Grignou @ 2025-01-22 19:03 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: Jonathan Cameron, linux-iio

On Tue, Jan 21, 2025 at 10:06 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, Jan 21, 2025 at 01:54:31PM -0800, Gwendal Grignou wrote:
> > On Mon, Jan 20, 2025 at 12:50 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > On Sat, Jan 18, 2025 at 04:33:52PM +0000, Jonathan Cameron wrote:
> > > > I briefly applied this, but...
> > > >
> > > > [jic23@jic23-huawei iio]$ make LOCALVERSION= W=1 -j12  C=1
> > > > mkdir -p /home/jic23/src/kernel/iio/tools/objtool && make O=/home/jic23/src/kernel/iio subdir=tools/objtool --no-print-directory -C objtool
> > > >   CALL    scripts/checksyscalls.sh
> > > >   INSTALL libsubcmd_headers
> > > >   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.o
> > > >   CC [M]  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.o
> > > > In file included from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.h:56,
> > > >                  from drivers/iio/common/cros_ec_sensors/cros_ec_sensors_trace.c:32:
> > > > ./include/trace/define_trace.h:106:42: fatal error: ./cros_ec_sensors_trace.h: No such file or directory
> > > >   106 | #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
> > > >       |                                          ^
> > > >
> > > > Despite dealing with trace points a lot in the day job
> > > > I still find them hard to actually add to a subsystem because
> > > > of fun things like this one.
> > I reproduced this error on a fresh kernel source tree. I suspect an
> > issue with sparse, as other trace.c files use __CHECKER__ macros. For
> > instance `drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c`:
> >
> >  10 /* sparse doesn't like tracepoint macros */
> >  11 #ifndef __CHECKER__
> > ...
>
> I still can't reproduce the error by using a just-cloned kernel source tree
> with C=1.  Could you provide some more details about your steps?  I'm
> wondering about which steps I may miss.
first, I checked sparse is new enough:
sparse --version
0.6.4 (Debian: 0.6.4-4)

Then
mkdir linux-iio
cd linux-iio
git init
git remote add iio  git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
git fetch iio
git checkout -b togreg iio/togreg
git am  ...-sensors-command.patch
make menuconfig  # accept the default
make LOCALVERSION= W=1 -j12  C=1

But there is still a mystery: looking around, there is no consensus
for TRACE_INCLUDE_PATH. Out of 157 definitions, 107 uses `.`, 39 uses
`../../drivers/....`, and 10 something else:
find drivers/ -name \*trace\*h -exec grep -re "^#define
TRACE_INCLUDE_PATH " {} \+ | wc -l
156
find drivers/ -name \*trace\*h -exec grep -re "^#define
TRACE_INCLUDE_PATH .$" {} \+ | wc -l
107
find drivers/ -name \*trace\*h -exec grep -re "^#define
TRACE_INCLUDE_PATH ../../drivers" {} \+ | wc -l
39

I am guessing it depends on the included paths added to the command
line used to invoke the compiler.

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

end of thread, other threads:[~2025-01-22 19:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-17 23:01 [PATCH v3] iio: cros_ec: Trace EC sensors command Gwendal Grignou
2025-01-18 16:33 ` Jonathan Cameron
2025-01-20  8:50   ` Tzung-Bi Shih
2025-01-21 21:54     ` Gwendal Grignou
2025-01-22  6:06       ` Tzung-Bi Shih
2025-01-22 19:03         ` Gwendal Grignou

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.