* [PATCH] iio: common: cros_ec_sensors: Add label attribute
@ 2022-04-12 21:07 Gwendal Grignou
2022-04-16 14:10 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Gwendal Grignou @ 2022-04-12 21:07 UTC (permalink / raw)
To: jic23, enric.balletbo; +Cc: bleung, linux-iio, Gwendal Grignou
When sensor location is known, populate iio sysfs "label" attribute:
* "accel-base" : the sensor is in the base of the convertible (2-1)
device.
* "accel-display" : the sensor is in the lid/display plane of the
device.
It apply to standalone accelerometer and IMU (accelerometer +
gyroscope).
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
.../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
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 989b109a82f47..f86b938b36bf8 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
@@ -325,6 +325,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
indio_dev->name = pdev->name;
if (physical_device) {
+ enum motionsensor_location loc;
+
state->param.cmd = MOTIONSENSE_CMD_INFO;
state->param.info.sensor_num = sensor_platform->sensor_num;
ret = cros_ec_motion_send_host_cmd(state, 0);
@@ -333,7 +335,12 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
return ret;
}
state->type = state->resp->info.type;
- state->loc = state->resp->info.location;
+ loc = state->resp->info.location;
+ if (loc == MOTIONSENSE_LOC_BASE)
+ indio_dev->label = "accel-base";
+ else if (loc == MOTIONSENSE_LOC_LID)
+ indio_dev->label = "accel-display";
+ state->loc = loc;
/* Set sign vector, only used for backward compatibility. */
memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS);
--
2.35.1.1178.g4f1659d476-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: common: cros_ec_sensors: Add label attribute
2022-04-12 21:07 [PATCH] iio: common: cros_ec_sensors: Add label attribute Gwendal Grignou
@ 2022-04-16 14:10 ` Jonathan Cameron
2022-04-27 19:10 ` Gwendal Grignou
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2022-04-16 14:10 UTC (permalink / raw)
To: Gwendal Grignou; +Cc: enric.balletbo, bleung, linux-iio
On Tue, 12 Apr 2022 14:07:03 -0700
Gwendal Grignou <gwendal@chromium.org> wrote:
> When sensor location is known, populate iio sysfs "label" attribute:
>
> * "accel-base" : the sensor is in the base of the convertible (2-1)
> device.
> * "accel-display" : the sensor is in the lid/display plane of the
> device.
>
> It apply to standalone accelerometer and IMU (accelerometer +
> gyroscope).
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
I'm happy with this change, but I think it would
be good for the patch description to include 'why' you want to
add this. I assume that's to make use of the standard ABI rather
than the custom ABI we introduced for this driver. We'd have
used label for this in the first place if it had been an option
at the time, but given we didn't what is the logic for duplicating
this information?
Thanks,
Jonathan
> ---
> .../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> 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 989b109a82f47..f86b938b36bf8 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
> @@ -325,6 +325,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> indio_dev->name = pdev->name;
>
> if (physical_device) {
> + enum motionsensor_location loc;
> +
> state->param.cmd = MOTIONSENSE_CMD_INFO;
> state->param.info.sensor_num = sensor_platform->sensor_num;
> ret = cros_ec_motion_send_host_cmd(state, 0);
> @@ -333,7 +335,12 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> return ret;
> }
> state->type = state->resp->info.type;
> - state->loc = state->resp->info.location;
> + loc = state->resp->info.location;
> + if (loc == MOTIONSENSE_LOC_BASE)
> + indio_dev->label = "accel-base";
> + else if (loc == MOTIONSENSE_LOC_LID)
> + indio_dev->label = "accel-display";
> + state->loc = loc;
>
> /* Set sign vector, only used for backward compatibility. */
> memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: common: cros_ec_sensors: Add label attribute
2022-04-16 14:10 ` Jonathan Cameron
@ 2022-04-27 19:10 ` Gwendal Grignou
0 siblings, 0 replies; 3+ messages in thread
From: Gwendal Grignou @ 2022-04-27 19:10 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: enric.balletbo, bleung, linux-iio
On Sat, Apr 16, 2022 at 7:02 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 12 Apr 2022 14:07:03 -0700
> Gwendal Grignou <gwendal@chromium.org> wrote:
>
> > When sensor location is known, populate iio sysfs "label" attribute:
> >
> > * "accel-base" : the sensor is in the base of the convertible (2-1)
> > device.
> > * "accel-display" : the sensor is in the lid/display plane of the
> > device.
> >
> > It apply to standalone accelerometer and IMU (accelerometer +
> > gyroscope).
> >
> > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
>
> I'm happy with this change, but I think it would
> be good for the patch description to include 'why' you want to
> add this. I assume that's to make use of the standard ABI rather
> than the custom ABI we introduced for this driver. We'd have
> used label for this in the first place if it had been an option
> at the time, but given we didn't what is the logic for duplicating
> this information?
I removed the duplication in v2, I needed to add a define for the
narrow case where a sensor is in a camera subassembly.
Note the location attribute can apply to other sensors than
accelerometers (gyroscope, light sensor).
Thanks,
Gwendal.
>
> Thanks,
>
> Jonathan
>
> > ---
> > .../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > 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 989b109a82f47..f86b938b36bf8 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
> > @@ -325,6 +325,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > indio_dev->name = pdev->name;
> >
> > if (physical_device) {
> > + enum motionsensor_location loc;
> > +
> > state->param.cmd = MOTIONSENSE_CMD_INFO;
> > state->param.info.sensor_num = sensor_platform->sensor_num;
> > ret = cros_ec_motion_send_host_cmd(state, 0);
> > @@ -333,7 +335,12 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > return ret;
> > }
> > state->type = state->resp->info.type;
> > - state->loc = state->resp->info.location;
> > + loc = state->resp->info.location;
> > + if (loc == MOTIONSENSE_LOC_BASE)
> > + indio_dev->label = "accel-base";
> > + else if (loc == MOTIONSENSE_LOC_LID)
> > + indio_dev->label = "accel-display";
> > + state->loc = loc;
> >
> > /* Set sign vector, only used for backward compatibility. */
> > memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-27 19:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-12 21:07 [PATCH] iio: common: cros_ec_sensors: Add label attribute Gwendal Grignou
2022-04-16 14:10 ` Jonathan Cameron
2022-04-27 19:10 ` 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.