Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Sat, 30 Sep 2023 16:15:43 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     David Lechner <dlechner@baylibre.com>
Cc:     linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
        linux-staging@lists.linux.dev,
        David Lechner <david@lechnology.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Michael Hennerich <Michael.Hennerich@analog.com>,
        Nuno =?UTF-8?B?U8Oh?= <nuno.sa@analog.com>,
        Axel Haslam <ahaslam@baylibre.com>,
        Philip Molloy <pmolloy@baylibre.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 18/27] staging: iio: resolver: ad2s1210: convert
 resolution to devicetree property
Message-ID: <20230930161543.22c3833e@jic23-huawei>
In-Reply-To: <20230929-ad2s1210-mainline-v3-18-fa4364281745@baylibre.com>
References: <20230929-ad2s1210-mainline-v3-0-fa4364281745@baylibre.com>
        <20230929-ad2s1210-mainline-v3-18-fa4364281745@baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 29 Sep 2023 12:23:23 -0500
David Lechner <dlechner@baylibre.com> wrote:

> From: David Lechner <david@lechnology.com>
> 
> From: David Lechner <dlechner@baylibre.com>
> 
> 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 <dlechner@baylibre.com>
...

>  
> +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;
> +}
> +