Re: [PATCH v4 2/5] iio: imu: inv_icm42607: Simplify IIO channel macros

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 20 Sep 2026 10:55:14 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 09/17, Kanak Shilledar wrote:
> > The INV_ICM42607_ACCEL_CHAN and INV_ICM42607_GYRO_CHAN macro had a third
> > parameter of _ext_info, drop it and just point the .ext_info field to
> > the respective inv_icm42607_*_ext_infos. This reduces the repetitive
> > calling of inv_icm42607_*_ext_infos struct in the IIO channel spec
> > struct and makes the code more easy to follow.
> > 
> > Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
> > ---
> >  drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c | 23 ++++++++++-------------
> >  drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c  | 23 ++++++++++-------------
> >  2 files changed, 20 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> > index 0b3f035c2da0c..8f61bc9014526 100644
> > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> > @@ -17,7 +17,7 @@
> >  #include "inv_icm42607.h"
> >  #include "inv_icm42607_temp.h"
> >  
> > -#define INV_ICM42607_ACCEL_CHAN(_modifier, _index, _ext_info)			\
> > +#define INV_ICM42607_ACCEL_CHAN(_modifier, _index)				\
> >  {										\
> >  	.type = IIO_ACCEL,							\
> >  	.modified = 1,								\
> > @@ -34,9 +34,14 @@
> >  		.storagebits = 16,						\
> >  		.endianness = IIO_BE,						\
> >  	},									\
> > -	.ext_info = _ext_info,							\
> > +	.ext_info = inv_icm42607_accel_ext_infos,				\
> >  }
> >  
> > +static const struct iio_chan_spec_ext_info inv_icm42607_accel_ext_infos[] = {
> > +	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, inv_icm42607_get_mount_matrix),
> > +	{ }
> > +};
> > +  
> Moving iio_chan_spec_ext_info declaration upwards looks like a spurious change.
> If moving the declaration upwards is really needed, the commit message could
> have a phrase or two explaining why.

It would have some sort of logic if they were before the .ext_info
in the macro just on basis we generally expect to look earlier
in a file for things that are used.  Obviously doesn't matter
given that is in a macro called much later.

Fully agree some explanatory comments would be useful!

Jonathan

> 
> >  enum inv_icm42607_accel_scan {
> >  	INV_ICM42607_ACCEL_SCAN_X,
> >  	INV_ICM42607_ACCEL_SCAN_Y,
> > @@ -44,18 +49,10 @@ enum inv_icm42607_accel_scan {
> >  	INV_ICM42607_ACCEL_SCAN_TEMP,
> >  };
> >  
> > -static const struct iio_chan_spec_ext_info inv_icm42607_accel_ext_infos[] = {
> > -	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, inv_icm42607_get_mount_matrix),
> > -	{ }
> > -};
> > -
> >  static const struct iio_chan_spec inv_icm42607_accel_channels[] = {
> > -	INV_ICM42607_ACCEL_CHAN(IIO_MOD_X, INV_ICM42607_ACCEL_SCAN_X,
> > -				inv_icm42607_accel_ext_infos),
> > -	INV_ICM42607_ACCEL_CHAN(IIO_MOD_Y, INV_ICM42607_ACCEL_SCAN_Y,
> > -				inv_icm42607_accel_ext_infos),
> > -	INV_ICM42607_ACCEL_CHAN(IIO_MOD_Z, INV_ICM42607_ACCEL_SCAN_Z,
> > -				inv_icm42607_accel_ext_infos),
> > +	INV_ICM42607_ACCEL_CHAN(IIO_MOD_X, INV_ICM42607_ACCEL_SCAN_X),
> > +	INV_ICM42607_ACCEL_CHAN(IIO_MOD_Y, INV_ICM42607_ACCEL_SCAN_Y),
> > +	INV_ICM42607_ACCEL_CHAN(IIO_MOD_Z, INV_ICM42607_ACCEL_SCAN_Z),
> >  	INV_ICM42607_TEMP_CHAN(INV_ICM42607_ACCEL_SCAN_TEMP),
> >  };
> >  
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> > index 5b4683c2dd1e4..8e8d36461e515 100644
> > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> > @@ -17,7 +17,7 @@
> >  #include "inv_icm42607.h"
> >  #include "inv_icm42607_temp.h"
> >  
> > -#define INV_ICM42607_GYRO_CHAN(_modifier, _index, _ext_info)			\
> > +#define INV_ICM42607_GYRO_CHAN(_modifier, _index)				\
> >  {										\
> >  	.type = IIO_ANGL_VEL,							\
> >  	.modified = 1,								\
> > @@ -34,9 +34,14 @@
> >  		.storagebits = 16,						\
> >  		.endianness = IIO_BE,						\
> >  	},									\
> > -	.ext_info = _ext_info,							\
> > +	.ext_info = inv_icm42607_gyro_ext_infos,				\
> >  }
> >  
> > +static const struct iio_chan_spec_ext_info inv_icm42607_gyro_ext_infos[] = {
> > +	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, inv_icm42607_get_mount_matrix),
> > +	{ }
> > +};
> > +  
> Same here, why moving inv_icm42607_gyro_ext_infos declarations up is necessary?
> 
> >  enum inv_icm42607_gyro_scan {
> >  	INV_ICM42607_GYRO_SCAN_X,
> >  	INV_ICM42607_GYRO_SCAN_Y,
> > @@ -44,18 +49,10 @@ enum inv_icm42607_gyro_scan {
> >  	INV_ICM42607_GYRO_SCAN_TEMP,
> >  };
> >  
> > -static const struct iio_chan_spec_ext_info inv_icm42607_gyro_ext_infos[] = {
> > -	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, inv_icm42607_get_mount_matrix),
> > -	{ }
> > -};
> > -
> >  static const struct iio_chan_spec inv_icm42607_gyro_channels[] = {
> > -	INV_ICM42607_GYRO_CHAN(IIO_MOD_X, INV_ICM42607_GYRO_SCAN_X,
> > -			       inv_icm42607_gyro_ext_infos),
> > -	INV_ICM42607_GYRO_CHAN(IIO_MOD_Y, INV_ICM42607_GYRO_SCAN_Y,
> > -			       inv_icm42607_gyro_ext_infos),
> > -	INV_ICM42607_GYRO_CHAN(IIO_MOD_Z, INV_ICM42607_GYRO_SCAN_Z,
> > -			       inv_icm42607_gyro_ext_infos),
> > +	INV_ICM42607_GYRO_CHAN(IIO_MOD_X, INV_ICM42607_GYRO_SCAN_X),
> > +	INV_ICM42607_GYRO_CHAN(IIO_MOD_Y, INV_ICM42607_GYRO_SCAN_Y),
> > +	INV_ICM42607_GYRO_CHAN(IIO_MOD_Z, INV_ICM42607_GYRO_SCAN_Z),
> >  	INV_ICM42607_TEMP_CHAN(INV_ICM42607_GYRO_SCAN_TEMP),
> >  };
> >  
> > 
> > -- 
> > 2.43.0
> >