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

[Marcelo Schmitt <marcelo.schmitt1@gmail.com>]

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.

>  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
>