On 5/19/24 2:20 PM, Jonathan Cameron wrote: > On Wed, 8 May 2024 12:21:09 -0500 > David Lechner <dlechner@xxxxxxxxxxxx> wrote: > >> On Wed, May 8, 2024 at 6:40 AM Jonathan Cameron >> <Jonathan.Cameron@xxxxxxxxxx> wrote: >>> >>> On Tue, 7 May 2024 14:02:08 -0500 >>> David Lechner <dlechner@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> >>> >>> 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.
