Re: [PATCH v3 07/10] iio: adc: ad7476: Conditionally call convstart

[Nuno Sá]

On Fri, Aug 08, 2025 at 11:54:09AM +0300, Matti Vaittinen wrote:
> The ad7476 supports two IC variants which may have a 'convstart' -GPIO
> for starting the conversion. Currently the driver calls a function which
> tries to access the GPIO for all of the IC variants, whether they
> support 'convstart' or not. This is not an error because this function
> returns early if GPIO information is not populated.
> 
> We can do a tad better by calling this function only for the ICs which
> have the 'convstart' by providing a function pointer to the convstart
> function from the chip_info structure, and calling this function only
> for the ICs which have the function pointer set.
> 
> This does also allow to support ICs which require different convstart
> handling than the currently supported ICs.
> 
> Call convstart function only on the ICs which can support it and allow
> IC-specific convstart functions for the ICs which require different
> handling.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
> ---
> Revision history:
>  v2 => v3:
>  - Use indirect call to convstart (via function pointer) also from the
>    ad7476_scan_direct().
>  - Adapt to the change which returned the chip_info pointer back to the
>    driver's state structure.
> 
>  v1 => v2:
>  - Adapt to the change which removed the chip_info pointer from the
>   driver's state structure.
> 
> The follow-up patch adding support for the ROHM BD79105 will bring
> different 'convstart' functions in use. The IC specific pointer will
> also prepare the way for this.
> ---

Reviewed-by: Nuno Sá <nuno.sa@xxxxxxxxxx>

>  drivers/iio/adc/ad7476.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index ad9e629f0cbd..6cb2cbeafbd3 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -31,6 +31,7 @@ struct ad7476_chip_info {
>  	unsigned int			int_vref_mv;
>  	struct iio_chan_spec		channel[2];
>  	void (*reset)(struct ad7476_state *);
> +	void (*conversion_pre_op)(struct ad7476_state *st);
>  	bool				has_vref;
>  	bool				has_vdrive;
>  };
> @@ -70,7 +71,8 @@ static irqreturn_t ad7476_trigger_handler(int irq, void  *p)
>  	struct ad7476_state *st = iio_priv(indio_dev);
>  	int b_sent;
>  
> -	ad7091_convst(st);
> +	if (st->chip_info->conversion_pre_op)
> +		st->chip_info->conversion_pre_op(st);
>  
>  	b_sent = spi_sync(st->spi, &st->msg);
>  	if (b_sent < 0)
> @@ -94,7 +96,8 @@ static int ad7476_scan_direct(struct ad7476_state *st)
>  {
>  	int ret;
>  
> -	ad7091_convst(st);
> +	if (st->chip_info->conversion_pre_op)
> +		st->chip_info->conversion_pre_op(st);
>  
>  	ret = spi_sync(st->spi, &st->msg);
>  	if (ret)
> @@ -160,12 +163,14 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
>  static const struct ad7476_chip_info ad7091_chip_info = {
>  	.channel[0] = AD7091R_CHAN(12),
>  	.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> +	.conversion_pre_op = ad7091_convst,
>  	.reset = ad7091_reset,
>  };
>  
>  static const struct ad7476_chip_info ad7091r_chip_info = {
>  	.channel[0] = AD7091R_CHAN(12),
>  	.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> +	.conversion_pre_op = ad7091_convst,
>  	.int_vref_mv = 2500,
>  	.has_vref = true,
>  	.reset = ad7091_reset,
> -- 
> 2.50.1
>