On Fri, 6 Dec 2024 17:39:28 +0100
Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> wrote:
> The commit in Fixes adds a special case when only one possible scale is
> available.
> If several scales are available, it sets the .read_avail field of the
> struct iio_info to ad9467_read_avail().
>
> However, this field already holds this function pointer, so the code is a
> no-op.
>
> Use another struct iio_info instead to actually reflect the intent
> described in the commit message. This way, the structure to use is selected
> at runtime and they can be kept as const.
>
> This is safer because modifying static structs that are shared between all
> instances like this, based on the properties of a single instance, is
> asking for trouble down the road.
>
> Fixes: b92f94f74826 ("iio: adc: ad9467: don't allow reading vref if not available")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> ---
> This patch is compile tested only and is completely speculative.
>
> Changes in v2:
> - use another struct iio_info to keep the structure const
Agree entirely with David that this is the way to go.
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
>
> v1: https://lore.kernel.org/linux-kernel/556f87c8931d7d7cdf56ebc79f974f8bef045b0d.1733431628.git.christophe.jaillet@xxxxxxxxxx/
> ---
> drivers/iio/adc/ad9467.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index d358958ab310..f30119b42ba0 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -895,7 +895,7 @@ static int ad9467_update_scan_mode(struct iio_dev *indio_dev,
> return 0;
> }
>
> -static struct iio_info ad9467_info = {
> +static const struct iio_info ad9467_info = {
> .read_raw = ad9467_read_raw,
> .write_raw = ad9467_write_raw,
> .update_scan_mode = ad9467_update_scan_mode,
> @@ -903,6 +903,14 @@ static struct iio_info ad9467_info = {
> .read_avail = ad9467_read_avail,
> };
>
> +/* Same as above, but without .read_avail */
> +static const struct iio_info ad9467_info_no_read_avail = {
> + .read_raw = ad9467_read_raw,
> + .write_raw = ad9467_write_raw,
> + .update_scan_mode = ad9467_update_scan_mode,
> + .debugfs_reg_access = ad9467_reg_access,
> +};
> +
> static int ad9467_scale_fill(struct ad9467_state *st)
> {
> const struct ad9467_chip_info *info = st->info;
> @@ -1214,11 +1222,12 @@ static int ad9467_probe(struct spi_device *spi)
> }
>
> if (st->info->num_scales > 1)
> - ad9467_info.read_avail = ad9467_read_avail;
> + indio_dev->info = &ad9467_info;
> + else
> + indio_dev->info = &ad9467_info_no_read_avail;
> indio_dev->name = st->info->name;
> indio_dev->channels = st->info->channels;
> indio_dev->num_channels = st->info->num_channels;
> - indio_dev->info = &ad9467_info;
>
> ret = ad9467_iio_backend_get(st);
> if (ret)
