Received-SPF: pass (google.com: domain of linux-kernel+bounces-183813-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <11c97f46-3881-4274-8c36-ec88218d3e60@baylibre.com>
Date: Mon, 20 May 2024 08:59:11 -0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RFC 4/4] iio: adc: ad7380: add support for multiple scan
 type
To: Jonathan Cameron <jic23@kernel.org>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>,
 Michael Hennerich <Michael.Hennerich@analog.com>,
 =?UTF-8?Q?Nuno_S=C3=A1?= <nuno.sa@analog.com>,
 Julien Stephan <jstephan@baylibre.com>, Esteban Blanc <eblanc@baylibre.com>,
 linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240507-iio-add-support-for-multiple-scan-types-v1-0-95ac33ee51e9@baylibre.com>
 <20240507-iio-add-support-for-multiple-scan-types-v1-4-95ac33ee51e9@baylibre.com>
 <20240508124049.00001661@Huawei.com>
 <CAMknhBFob4Wd8Gm8W8NKSuL9UbBCY8+fAN_voGPhb4Fy1tAT-w@mail.gmail.com>
 <20240519202039.5a70157d@jic23-huawei>
Content-Language: en-US
From: David Lechner <dlechner@baylibre.com>
In-Reply-To: <20240519202039.5a70157d@jic23-huawei>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 5/19/24 2:20 PM, Jonathan Cameron wrote:
> On Wed, 8 May 2024 12:21:09 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
>> On Wed, May 8, 2024 at 6:40 AM Jonathan Cameron
>> <Jonathan.Cameron@huawei.com> wrote:
>>>
>>> On Tue,  7 May 2024 14:02:08 -0500
>>> David Lechner <dlechner@baylibre.com> wrote:
>>>  
>>>> The AD783x chips have a resolution boost feature that allows for 2
>>>> extra bits of resolution. Previously, we had to choose a scan type to
>>>> fit the largest resolution and manipulate the raw data to fit when the
>>>> resolution was lower. This patch adds support for multiple scan types
>>>> for the voltage input channels so that we can support both resolutions
>>>> without having to manipulate the raw data.
>>>>
>>>> Signed-off-by: David Lechner <dlechner@baylibre.com>  
>>>
>>> I'm wondering about the control mechanism.  I was thinking we'd poke
>>> the scan type directly but this may well make more sense.
>>>
>>> This is relying on _scale change to trigger the change in the scan type.
>>> That may well be sufficient and I've been over thinking this for far too many
>>> years :)
>>>
>>> It will get messy though in some cases as the device might have a PGA on the
>>> front end so we will have a trade off between actual scaling control and
>>> resolution related scale changes. We've had a few device where the scale
>>> calculation is already complex and involves various different hardware
>>> controls, but none have affected the storage format like this.
>>>
>>> I'll think some more.
>>>  
>>
>> Here is some more food for thought. The AD4630 family of chips we are
>> working on is similar to this one in that it also has oversampling
>> with increased resolution. Except in that case, they are strictly tied
>> together. With oversampling disabled, we must only read 24-bits (or 16
>> depending on the exact model) and when oversampling is enabled, we
>> must read 32-bits (30 real bits with 2-bit shift). So in that case,
>> the scan_type would depend only on oversampling ratio > 0. (Writing
>> the oversampling ratio attribute would affect scale, but scale
>> wouldn't be writable like on ad7380.)
>>
>> It seems more intuitive to me that to enable oversampling, we would
>> just write to the oversampling ratio attribute rather than having to
>> write to a buffer _type attribute to enable oversampling in the first
>> place. And other than requiring reading the documentation it would be
>> pretty hard to guess that writing le:s30/32>>2 is what you need to do
>> to enable oversampling.
>>
> 
> Ok. Few weeks thinking and I've no better ideas.  Generally I'm fine
> with how you did this but I wouldn't have a 'special / default'
> scan_type.  Just put them all in the array and pick between them.
> That avoids fun of people trying to work out on what basis to
> prefer one over another. 
> 
> So tidy the loose ends up and I'd be delighted to see a non RFC version.
> It 'might' be worth waiting until we have a couple of suitable drivers
> though and then show the feature works well for them all.
> Whilst I think I'd take it with just one though as can see how it fits
> together, but more than one driver would boost my confidence level.
> 
> Jonathan

Great! I'll do a v2 with only the core code. Julian, Esteban and I
are all working on drivers that should make use of this. So if all goes
well, we should have 3 drivers for you this release cycle.