On Fri, Jan 17, 2025 at 7:47 AM Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> wrote: > > On Thu, 16 Jan 2025 20:20:59 -0800 > Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote: > > > 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> > What this is missing is why? > > I'm not against adding tracepoints in drivers, but some statement of > the usecase etc would be helpful. If it's debug only that makes > for different requirements than some tool is going to connect to > this stream. It is for debug only, will update the commit message. > > Jonathan > > > > 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..075196ca804a1 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. > > @@ -423,6 +425,7 @@ 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, (void *)state->msg->data, ret); > > Can we cast to the right type rather than a void *? Fixed in v3: `data` is a char array, but in the context of cros_ec_motion_send_host_cmd(), it is a > > > if (ret < 0) > > return ret; > > > >
