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

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 17 Nov 2024 18:26:41 +0000
Lothar Rubusch <l.rubusch@gmail.com> 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@gmail.com>
> ---
>  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,