Re: [PATCH 7/8] iio: adc: ad9467: cache the sample rate

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 19 Apr 2024 17:36:50 +0200
Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sa <nuno.sa@analog.com>
> 
> Since we allow to change the sampling frequency and do it with
> clk_round_rate(), we can cache it and use on the read_raw() interface.
> This will also be useful in a following patch supporting interface
> calibration.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>

The clk subsystem caches the clock rate in most cases anyway, so
I'm not sure why we need this.  Or it the point that you are going
to temporarily change it in the next patch?

Patch looks fine, but I think a clearer requirements statement is
needed.

Jonathan


> ---
>  drivers/iio/adc/ad9467.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index 7475ec2a56c72..7db87ccc1ea4b 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -117,6 +117,7 @@ struct ad9467_state {
>  	struct iio_backend		*back;
>  	struct spi_device		*spi;
>  	struct clk			*clk;
> +	unsigned long			sample_rate;
>  	unsigned int			output_mode;
>  	unsigned int                    (*scales)[2];
>  
> @@ -331,7 +332,7 @@ static int ad9467_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		return ad9467_get_scale(st, val, val2);
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		*val = clk_get_rate(st->clk);
> +		*val = st->sample_rate;
>  
>  		return IIO_VAL_INT;
>  	default:
> @@ -346,6 +347,7 @@ static int ad9467_write_raw(struct iio_dev *indio_dev,
>  	struct ad9467_state *st = iio_priv(indio_dev);
>  	const struct ad9467_chip_info *info = st->info;
>  	long r_clk;
> +	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SCALE:
> @@ -358,7 +360,12 @@ static int ad9467_write_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		}
>  
> -		return clk_set_rate(st->clk, r_clk);
> +		ret = clk_set_rate(st->clk, r_clk);
> +		if (ret)
> +			return ret;
> +
> +		st->sample_rate = r_clk;
> +		return 0;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -543,6 +550,8 @@ static int ad9467_probe(struct spi_device *spi)
>  	if (IS_ERR(st->clk))
>  		return PTR_ERR(st->clk);
>  
> +	st->sample_rate = clk_get_rate(st->clk);
> +
>  	st->pwrdown_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
>  						   GPIOD_OUT_LOW);
>  	if (IS_ERR(st->pwrdown_gpio))
>