This series suggests some simplifications to the ad7476 ADC. It is
currently 100% untested, and shouldn't be merged as is. I'd like to hear
opinions on these changes before adding support to the ROHM BD79105 ADC.

Intention of the patch 1 is pretty trivial. I'd just like to hear if
people think the enum + ID table approach is preferred over direct
pointers to IC specific structs in SPI device's driver_data.

Real reason for the RFC version is the patch 2. It aims to clear the
supply handling logic. I did also an alternate version which requires
the names of the regulators to be provided in the chip_data:
https://github.com/M-Vaittinen/linux/commit/cf5b3078feb17f9a0069b2c7c86f6d980e879356

I believe the version in the link --^
is clearer, but it can potentially help people to add issues with supply
enable ordering.

I can't still say if the patch 2 contained in this series is better, or
if the one behind the link is better way to go. So, RFC it is :)

Matti Vaittinen (2):
  iio: adc: ad7476: Simplify chip type detection
  iio: adc: ad7476: Simplify scale handling

 drivers/iio/adc/ad7476.c | 376 +++++++++++++++++----------------------
 1 file changed, 164 insertions(+), 212 deletions(-)

-- 
2.50.1

On Fri, Aug 01, 2025 at 01:06:46PM +0300, Matti Vaittinen wrote:
> This series suggests some simplifications to the ad7476 ADC. It is
> currently 100% untested, and shouldn't be merged as is. I'd like to hear
> opinions on these changes before adding support to the ROHM BD79105 ADC.
> 
> Intention of the patch 1 is pretty trivial. I'd just like to hear if
> people think the enum + ID table approach is preferred over direct
> pointers to IC specific structs in SPI device's driver_data.
> 
> Real reason for the RFC version is the patch 2. It aims to clear the
> supply handling logic. I did also an alternate version which requires
> the names of the regulators to be provided in the chip_data:
> https://github.com/M-Vaittinen/linux/commit/cf5b3078feb17f9a0069b2c7c86f6d980e879356
> 
> I believe the version in the link --^
> is clearer, but it can potentially help people to add issues with supply
> enable ordering.
> 
> I can't still say if the patch 2 contained in this series is better, or
> if the one behind the link is better way to go. So, RFC it is :)
> 
> Matti Vaittinen (2):
>   iio: adc: ad7476: Simplify chip type detection
>   iio: adc: ad7476: Simplify scale handling
> 
>  drivers/iio/adc/ad7476.c | 376 +++++++++++++++++----------------------
>  1 file changed, 164 insertions(+), 212 deletions(-)
> 
> -- 
> 2.50.1
> 

With the suggestion given by Jonathan on the first patch:
(I also dunno there's someone with variable voltages...)

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

On Fri, 1 Aug 2025 13:06:46 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> This series suggests some simplifications to the ad7476 ADC. It is
> currently 100% untested, and shouldn't be merged as is. I'd like to hear
> opinions on these changes before adding support to the ROHM BD79105 ADC.
> 
> Intention of the patch 1 is pretty trivial. I'd just like to hear if
> people think the enum + ID table approach is preferred over direct
> pointers to IC specific structs in SPI device's driver_data.

Definitely prefer direct pointers as you have here.

> 
> Real reason for the RFC version is the patch 2. It aims to clear the
> supply handling logic. I did also an alternate version which requires
> the names of the regulators to be provided in the chip_data:
> https://github.com/M-Vaittinen/linux/commit/cf5b3078
> 
> I believe the version in the link --^
> is clearer, but it can potentially help people to add issues with supply
> enable ordering.
> 
> I can't still say if the patch 2 contained in this series is better, or
> if the one behind the link is better way to go. So, RFC it is :)

I missed this (who reads cover letters?) in first look.  Anyhow, having
taken a quick look at that alternative I slightly prefer the one you have here.

Even if we have supply ordering issues, it seems like they are unlikely to
vary randomly across supported parts so should be easy to incorporate those
rules with the approach here if needed.

Jonathan


> 
> Matti Vaittinen (2):
>   iio: adc: ad7476: Simplify chip type detection
>   iio: adc: ad7476: Simplify scale handling
> 
>  drivers/iio/adc/ad7476.c | 376 +++++++++++++++++----------------------
>  1 file changed, 164 insertions(+), 212 deletions(-)
>

On 02/08/2025 13:59, Jonathan Cameron wrote:
> On Fri, 1 Aug 2025 13:06:46 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> This series suggests some simplifications to the ad7476 ADC. It is
>> currently 100% untested, and shouldn't be merged as is. I'd like to hear
>> opinions on these changes before adding support to the ROHM BD79105 ADC.
>>
>> Intention of the patch 1 is pretty trivial. I'd just like to hear if
>> people think the enum + ID table approach is preferred over direct
>> pointers to IC specific structs in SPI device's driver_data.
> 
> Definitely prefer direct pointers as you have here.
> 
>>
>> Real reason for the RFC version is the patch 2. It aims to clear the
>> supply handling logic. I did also an alternate version which requires
>> the names of the regulators to be provided in the chip_data:
>> https://github.com/M-Vaittinen/linux/commit/cf5b3078
>>
>> I believe the version in the link --^
>> is clearer, but it can potentially help people to add issues with supply
>> enable ordering.
>>
>> I can't still say if the patch 2 contained in this series is better, or
>> if the one behind the link is better way to go. So, RFC it is :)
> 
> I missed this (who reads cover letters?) in first look.  Anyhow, having
> taken a quick look at that alternative I slightly prefer the one you have here.

I need to ask if the "here" refers to the patch contained in this series 
(let's say it's version 1)

or the
https://github.com/M-Vaittinen/linux/commit/cf5b3078
(which I shall call version 2 from now on).

> Even if we have supply ordering issues, it seems like they are unlikely to
> vary randomly across supported parts so should be easy to incorporate those
> rules with the approach here if needed.

Reason why I mentioned the supply ordering is, that (AFAIK) enabling the 
supplies in wrong order may silently damage IC's in the long run. Nasty 
problems which may randomly manifest themselves only after a longer 
period of time - breaking the hardware with seemingly no reason.

As far as I know, the usual case is that the VCC (or VDD or 
whatchamacallit) should be enabled prior V_IO (Vdrive or 
whatchamacallit) or Vref. The version 1 (as well as the currently merged 
driver) do always enable VCC first. The version 2 does first bulk-enable 
to non VREF supplies and only then enables the VREF. Some ICs use VCC as 
VREF, which might result the VCC being enabled only after other 
supplies, but I didn't notice any in-tree supported ICs having both the 
VCC as VREF and separate Vdrive. Also, I have no proper information 
regarding _how_ unsafe it is to do enabling at wrong order. I suppose 
different ICs can be more or less tolerant towards this.

Hence, I assume this is rather safe. Problem being "assume" and "rather" 
- which is why I wanted to have another opinion as well :)

Thanks for the feedback all!

Yours,
	-- Matti

PS. Anyone planning to attend the ELCE at Amsterdam this autumn?