Re: [PATCH v2 12/22] iio: accel: adxl345: elaborate iio channel definition

From: Jonathan Cameron
Date: Sun Nov 24 2024 - 14:18:55 EST


On Sun, 17 Nov 2024 18:26:41 +0000
Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote:

> Make the channel definition ready to allow for feature implementation
> for this accelerometer sensor.

Add what you need for each patch that makes use of the new data.
A separate change like this is not easy to review without seeing those
usecases and a reviewer should not need to go look for them in other
patches.

>
> Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx>
> ---
> drivers/iio/accel/adxl345_core.c | 36 +++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index b16887ec1c..f98ddef4c5 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -141,21 +141,33 @@ struct adxl34x_state {
> u8 intio;
> };
>
> -#define ADXL345_CHANNEL(index, axis) { \
> - .type = IIO_ACCEL, \
> - .modified = 1, \
> - .channel2 = IIO_MOD_##axis, \
> - .address = index, \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> - BIT(IIO_CHAN_INFO_CALIBBIAS), \
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> - BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +#define ADXL34x_CHANNEL(index, reg, axis) { \
> + .type = IIO_ACCEL, \

No reason to change the indentation.
That just makes this patch hard to read as I can't immeidately see what changed.

> + .address = (reg), \
> + .modified = 1, \
> + .channel2 = IIO_MOD_##axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_CALIBBIAS), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_index = (index), \
> + .scan_type = { \
Bring the scan stuff in as part of a patch that makes use of it. Not
beofre that.

> + .sign = 's', \
> + .realbits = 13, \
> + .storagebits = 16, \
> + .shift = 0, \
> + .endianness = IIO_LE, \
> + }, \
> }
>
> +enum adxl34x_chans {
> + chan_x, chan_y, chan_z,
> +};
> +
> static const struct iio_chan_spec adxl34x_channels[] = {
> - ADXL345_CHANNEL(0, X),
> - ADXL345_CHANNEL(1, Y),
> - ADXL345_CHANNEL(2, Z),
> + ADXL34x_CHANNEL(0, chan_x, X),
> + ADXL34x_CHANNEL(1, chan_y, Y),
> + ADXL34x_CHANNEL(2, chan_z, Z),
> };
>
> static int adxl345_read_raw(struct iio_dev *indio_dev,