Re: [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 28 Oct 2024 08:11:15 +0100
ahaslam@baylibre.com wrote:

> From: Axel Haslam <ahaslam@baylibre.com>
> 
> Include a chip info struct in device SPI and device OF match tables to
> provide channel definitions for each particular ADC model and drop
> device enum.
> 
> Suggested-by: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> ---
>  drivers/iio/dac/ad5791.c | 107 +++++++++++++++++++--------------------
>  1 file changed, 51 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
> index 553431bf0232..a11e81211669 100644
> --- a/drivers/iio/dac/ad5791.c
> +++ b/drivers/iio/dac/ad5791.c
> @@ -65,7 +65,9 @@
>   */
Whilst adding the docs for the missing entries, please delete this
blank line!

Otherwise patch looks good to me.

Jonathan
	
>  
>  struct ad5791_chip_info {
> -	int (*get_lin_comp)	(unsigned int span);
> +	const char *name;
> +	const struct iio_chan_spec channel;
> +	int (*get_lin_comp)(unsigned int span);
>  };
>  
>  /**
> @@ -98,13 +100,6 @@ struct ad5791_state {
>  	} data[3] __aligned(IIO_DMA_MINALIGN);
>  };
>  
> -enum ad5791_supported_device_ids {
> -	ID_AD5760,
> -	ID_AD5780,
> -	ID_AD5781,
> -	ID_AD5791,
> -};
> -
>  static int ad5791_spi_write(struct ad5791_state *st, u8 addr, u32 val)
>  {
>  	st->data[0].d32 = cpu_to_be32(AD5791_CMD_WRITE |
> @@ -228,20 +223,6 @@ static int ad5780_get_lin_comp(unsigned int span)
>  	else
>  		return AD5780_LINCOMP_10_20;
>  }
> -static const struct ad5791_chip_info ad5791_chip_info_tbl[] = {
> -	[ID_AD5760] = {
> -		.get_lin_comp = ad5780_get_lin_comp,
> -	},
> -	[ID_AD5780] = {
> -		.get_lin_comp = ad5780_get_lin_comp,
> -	},
> -	[ID_AD5781] = {
> -		.get_lin_comp = ad5791_get_lin_comp,
> -	},
> -	[ID_AD5791] = {
> -		.get_lin_comp = ad5791_get_lin_comp,
> -	},
> -};
>  
>  static int ad5791_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
> @@ -289,30 +270,34 @@ static const struct iio_chan_spec_ext_info ad5791_ext_info[] = {
>  	{ },
>  };
>  
> -#define AD5791_CHAN(bits, _shift) {			\
> -	.type = IIO_VOLTAGE,				\
> -	.output = 1,					\
> -	.indexed = 1,					\
> -	.address = AD5791_ADDR_DAC0,			\
> -	.channel = 0,					\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> -		BIT(IIO_CHAN_INFO_OFFSET),		\
> -	.scan_type = {					\
> -		.sign = 'u',				\
> -		.realbits = (bits),			\
> -		.storagebits = 24,			\
> -		.shift = (_shift),			\
> -	},						\
> -	.ext_info = ad5791_ext_info,			\
> +#define AD5791_DEFINE_CHIP_INFO(_name, bits, _shift, _lin_comp)		\
> +static const struct ad5791_chip_info _name##_chip_info = {		\
> +	.name = #_name,							\
> +	.get_lin_comp = &(_lin_comp),					\
> +	.channel = {							\
> +			.type = IIO_VOLTAGE,				\
> +			.output = 1,					\
> +			.indexed = 1,					\
> +			.address = AD5791_ADDR_DAC0,			\
> +			.channel = 0,					\
> +			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> +			.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_OFFSET),		\
> +			.scan_type = {					\
> +				.sign = 'u',				\
> +				.realbits = (bits),			\
> +				.storagebits = 24,			\
> +				.shift = (_shift),			\
> +			},						\
> +			.ext_info = ad5791_ext_info,			\
> +	},								\
>  }
>  
> -static const struct iio_chan_spec ad5791_channels[] = {
> -	[ID_AD5760] = AD5791_CHAN(16, 4),
> -	[ID_AD5780] = AD5791_CHAN(18, 2),
> -	[ID_AD5781] = AD5791_CHAN(18, 2),
> -	[ID_AD5791] = AD5791_CHAN(20, 0)
> -};
> +AD5791_DEFINE_CHIP_INFO(ad5760, 16, 4, ad5780_get_lin_comp);
> +AD5791_DEFINE_CHIP_INFO(ad5780, 18, 2, ad5780_get_lin_comp);
> +AD5791_DEFINE_CHIP_INFO(ad5781, 18, 2, ad5791_get_lin_comp);
> +AD5791_DEFINE_CHIP_INFO(ad5790, 20, 0, ad5791_get_lin_comp);
> +AD5791_DEFINE_CHIP_INFO(ad5791, 20, 0, ad5791_get_lin_comp);