On Fri, 17 Jan 2025 15:01:36 -0800
Gwendal Grignou <gwendal@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
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>
