2023-09-30 15:17:02

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 18/27] staging: iio: resolver: ad2s1210: convert resolution to devicetree property

On Fri, 29 Sep 2023 12:23:23 -0500
David Lechner <[email protected]> wrote:

> From: David Lechner <[email protected]>
>
> From: David Lechner <[email protected]>
>
> Selecting the resolution was implemented as the `bits` sysfs attribute.
> However, the selection of the resolution depends on how the hardware
> is wired and the specific application, so this is rather a job for
> devicetree to describe.
>
> A new devicetree property `adi,resolution` to specify the resolution
> required for each chip is added and the `bits` sysfs attribute is
> removed.

Description need updating to reflect property having a different name.

Otherwise this LGTM
>
> Since the resolution is now supplied by a devicetree property, the
> resolution-gpios are now optional and we can allow for the case where
> the resolution pins on the AD2S1210 are hard-wired instead of requiring
> them to be connected to gpios.
>
> Signed-off-by: David Lechner <[email protected]>
...

>
> +static int ad2s1210_setup_properties(struct ad2s1210_state *st)
> +{
> + struct device *dev = &st->sdev->dev;
> + u32 val;
> + int ret;
> +
> + ret = device_property_read_u32(dev, "assigned-resolution-bits", &val);
Doesn't match patch description of what this is called.

> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "failed to read assigned-resolution-bits property\n");
> +
> + if (val < 10 || val > 16)
> + return dev_err_probe(dev, -EINVAL,
> + "resolution out of range: %u\n", val);
> +
> + st->resolution = (val - 10) >> 1;
> +
> + return 0;
> +}
> +