[PATCH 0/2] Support ROHM BU79100G ADC

[PATCH 0/2] Support ROHM BU79100G ADC

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

Add support for the ROHM BU79100G ADC.

The BU79100G is a simple 12-bit, 1 channel ADC which can be read using
SPI. The BU79100G does not have MOSI pin, and has only VCC supply pin
which does also work as a Vref.

Series was based on v6.15-rc1. I can rebase if needed.

Matti Vaittinen (2):
  dt-bindings: iio: adc: Add ROHM BD79100G
  iio: adc: ad7476: Support ROHM BU79100G

 Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml | 2 ++
 drivers/iio/adc/ad7476.c                                  | 8 ++++++++
 2 files changed, 10 insertions(+)


base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
-- 
2.49.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 1/2] dt-bindings: iio: adc: Add ROHM BD79100G

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]

The ROHM BD79100G is a 12-bit ADC which can be read over SPI. Device has
no MOSI pin. ADC results can be read from MISO by clocking in 16 bits.
The 4 leading bits will be zero, last 12 containig the data.

Device has only VCC supply pin, which acts also as a VFS, determining the
voltage for full 12-bits. Specifying it is mandatory.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
index 44c671eeda73..10fb6e14c2d0 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
@@ -46,6 +46,7 @@ properties:
       - ti,ads7867
       - ti,ads7868
       - lltc,ltc2314-14
+      - rohm,bu79100g
 
   reg:
     maxItems: 1
@@ -96,6 +97,7 @@ allOf:
               - ti,adc121s
               - ti,ads7866
               - ti,ads7868
+              - rohm,bu79100g
     then:
       required:
         - vcc-supply
-- 
2.49.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 2/2] iio: adc: ad7476: Support ROHM BU79100G

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

ROHM BU79100G is a 12-bit, single channel ADC. Support reading ADC
measurements using the ad7476.c

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/iio/adc/ad7476.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index 37b0515cf4fc..6c5c57de6a58 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -2,6 +2,7 @@
 /*
  * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
  * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
+ * ROHM BU79100G 12-bit SPI ADC driver
  *
  * Copyright 2010 Analog Devices Inc.
  */
@@ -72,6 +73,7 @@ enum ad7476_supported_device_ids {
 	ID_ADS7866,
 	ID_ADS7867,
 	ID_ADS7868,
+	ID_BU79100G,
 	ID_LTC2314_14,
 };
 
@@ -281,6 +283,10 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
 		.channel[0] = ADS786X_CHAN(8),
 		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
 	},
+	[ID_BU79100G] = {
+		.channel[0] = ADS786X_CHAN(12),
+		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+	},
 	[ID_LTC2314_14] = {
 		.channel[0] = AD7940_CHAN(14),
 		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
@@ -311,6 +317,7 @@ static int ad7476_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
+
 	st->chip_info =
 		&ad7476_chip_info_tbl[spi_get_device_id(spi)->driver_data];
 
@@ -435,6 +442,7 @@ static const struct spi_device_id ad7476_id[] = {
 	{ "ads7866", ID_ADS7866 },
 	{ "ads7867", ID_ADS7867 },
 	{ "ads7868", ID_ADS7868 },
+	{ "bu79100g", ID_BU79100G },
 	{ "ltc2314-14", ID_LTC2314_14 },
 	{ }
 };
-- 
2.49.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] dt-bindings: iio: adc: Add ROHM BD79100G

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]

On Tue, May 13, 2025 at 11:26:27AM +0300, Matti Vaittinen wrote:
> The ROHM BD79100G is a 12-bit ADC which can be read over SPI. Device has
> no MOSI pin. ADC results can be read from MISO by clocking in 16 bits.
> The 4 leading bits will be zero, last 12 containig the data.

I think it is probably worth mentioning why a rohm device is going into
this binding (clone?) and that the 12-bit thing is a differentiator that
is why you're not using a fallback.

> 
> Device has only VCC supply pin, which acts also as a VFS, determining the
> voltage for full 12-bits. Specifying it is mandatory.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
> index 44c671eeda73..10fb6e14c2d0 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
> @@ -46,6 +46,7 @@ properties:
>        - ti,ads7867
>        - ti,ads7868
>        - lltc,ltc2314-14
> +      - rohm,bu79100g
>  
>    reg:
>      maxItems: 1
> @@ -96,6 +97,7 @@ allOf:
>                - ti,adc121s
>                - ti,ads7866
>                - ti,ads7868
> +              - rohm,bu79100g
>      then:
>        required:
>          - vcc-supply
> -- 
> 2.49.0
> 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] dt-bindings: iio: adc: Add ROHM BD79100G

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

On Tue, May 13, 2025 at 03:39:11PM +0100, Conor Dooley wrote:
> On Tue, May 13, 2025 at 11:26:27AM +0300, Matti Vaittinen wrote:
> > The ROHM BD79100G is a 12-bit ADC which can be read over SPI. Device has
> > no MOSI pin. ADC results can be read from MISO by clocking in 16 bits.
> > The 4 leading bits will be zero, last 12 containig the data.
> 
> I think it is probably worth mentioning why a rohm device is going into
> this binding (clone?) and that the 12-bit thing is a differentiator that
> is why you're not using a fallback.

Sent too soon, meant to say: with that
Acked-by: Conor Dooley <conor.dooley@microchip.com>

> 
> > 
> > Device has only VCC supply pin, which acts also as a VFS, determining the
> > voltage for full 12-bits. Specifying it is mandatory.
> > 
> > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
> > index 44c671eeda73..10fb6e14c2d0 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
> > @@ -46,6 +46,7 @@ properties:
> >        - ti,ads7867
> >        - ti,ads7868
> >        - lltc,ltc2314-14
> > +      - rohm,bu79100g
> >  
> >    reg:
> >      maxItems: 1
> > @@ -96,6 +97,7 @@ allOf:
> >                - ti,adc121s
> >                - ti,ads7866
> >                - ti,ads7868
> > +              - rohm,bu79100g
> >      then:
> >        required:
> >          - vcc-supply
> > -- 
> > 2.49.0
> > 
> 
> 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] dt-bindings: iio: adc: Add ROHM BD79100G

[Matti Vaittinen]

On 13/05/2025 17:39, Conor Dooley wrote:
> On Tue, May 13, 2025 at 11:26:27AM +0300, Matti Vaittinen wrote:
>> The ROHM BD79100G is a 12-bit ADC which can be read over SPI. Device has
>> no MOSI pin. ADC results can be read from MISO by clocking in 16 bits.
>> The 4 leading bits will be zero, last 12 containig the data.
> 
> I think it is probably worth mentioning why a rohm device is going into
> this binding (clone?) and that the 12-bit thing is a differentiator that
> is why you're not using a fallback.

Thanks for mentioning the fallback option Conor! You're a hero :)

Now that you mentioned using a fallback, I believe I can ditch the 
driver changes and make BU79100G to use adc101s as a fallback!

I didn't even consider if some of the existing devices were (from SW 
perspective) identical. I was just happy when I found there was a driver 
supporting these simple SPI ADCs. Then I picked the right macro for 
doing register data conversion and correct shift, dumped in the bit 
width and extended the longish list of devices. I never checked if 
another device in the driver had similar set of "IC specific values".

Yours,
	-- Matti

Re: [PATCH 1/2] dt-bindings: iio: adc: Add ROHM BD79100G

[Matti Vaittinen]

On 14/05/2025 08:36, Matti Vaittinen wrote:
> On 13/05/2025 17:39, Conor Dooley wrote:
>> On Tue, May 13, 2025 at 11:26:27AM +0300, Matti Vaittinen wrote:
>>> The ROHM BD79100G is a 12-bit ADC which can be read over SPI. Device has
>>> no MOSI pin. ADC results can be read from MISO by clocking in 16 bits.
>>> The 4 leading bits will be zero, last 12 containig the data.
>>
>> I think it is probably worth mentioning why a rohm device is going into
>> this binding (clone?) and that the 12-bit thing is a differentiator that
>> is why you're not using a fallback.
> 
> Thanks for mentioning the fallback option Conor! You're a hero :)
> 
> Now that you mentioned using a fallback, I believe I can ditch the 
> driver changes and make BU79100G to use adc101s as a fallback!

I mean, ads7866 as a fallback. Sorry.

> 
> I didn't even consider if some of the existing devices were (from SW 
> perspective) identical. I was just happy when I found there was a driver 
> supporting these simple SPI ADCs. Then I picked the right macro for 
> doing register data conversion and correct shift, dumped in the bit 
> width and extended the longish list of devices. I never checked if 
> another device in the driver had similar set of "IC specific values".
> 
> Yours,
>      -- Matti

Re: [PATCH 2/2] iio: adc: ad7476: Support ROHM BU79100G

[Matti Vaittinen]

On 13/05/2025 11:26, Matti Vaittinen wrote:
> ROHM BU79100G is a 12-bit, single channel ADC. Support reading ADC
> measurements using the ad7476.c
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
>   drivers/iio/adc/ad7476.c | 8 ++++++++
>   1 file changed, 8 insertions(+)

For anyone who might hit this mail thread later:

Conor made me realize that, for now, the BU79100G looks identical to the 
ads7866. Thus, these code-changes aren't needed at the moment, and this 
patch can be dropped. For those who wish to use BU79100G, please 
introduce it as

compatible = "rohm,bu79100g", "ti,ads7866";

having the ads7866 as a fallback. This should allow us to introduce 
driver side differences later, should they be needed.

V2: 
https://lore.kernel.org/all/4907a096eee1f54afae834213cf721b551382d4e.1747203712.git.mazziesaccount@gmail.com/
drops the driver changes and adds only the fallback compatible to the 
bindings.

Re: [PATCH 2/2] iio: adc: ad7476: Support ROHM BU79100G

[Matti Vaittinen]

On 14/05/2025 10:38, Matti Vaittinen wrote:
> On 13/05/2025 11:26, Matti Vaittinen wrote:
>> ROHM BU79100G is a 12-bit, single channel ADC. Support reading ADC
>> measurements using the ad7476.c
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> ---
>>   drivers/iio/adc/ad7476.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
> 
> For anyone who might hit this mail thread later:
> 
> Conor made me realize that, for now, the BU79100G looks identical to the 
> ads7866. Thus, these code-changes aren't needed at the moment, and this 
> patch can be dropped. For those who wish to use BU79100G, please 
> introduce it as
> 
> compatible = "rohm,bu79100g", "ti,ads7866";
> 

I was too hasty.

It seems to me that the fallback won't work with the current driver 
because the driver is not populating the of_match_table, but is relying 
solely on the spi_device_id table.

Judging a quick code reading, the spi_driver_id table entries are 
matched to the modalias:
https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/spi/spi.c#L393

Which is (as far as I understand), generated from the first compatible:
https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/of/base.c#L1170

and not from the fallback one.

I suppose this means that we would need to add the of_match_table entry 
for the ti,ads7866 to make the fallback entry to match the driver.

But...

The __spi_register_driver() has following comment:
	/*
	 * For Really Good Reasons we use spi: modaliases not of:
	 * modaliases for DT so module autoloading won't work if we
	 * don't have a spi_device_id as well as a compatible string.
	 */
https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/spi/spi.c#L487

So, having the of_match_table for would not be sufficient for the 
autoloading, which would still require the bu79100g to be in the 
spi_device_id table.

Am I missing something? I don't see how the Linux SPI drivers benefit 
from the fallback entries in the dt? (Not saying fallbacks wouldn't be 
The Right Thing To Do. Ideally DTs aren't for Linux only, maybe some 
other systems can utilize them). To me it seems I still need to add the 
spi_device_id entry for the BU79100G, and of_match_table has no 
additional benefit? If this is right, then this patch is still relevant, 
even though the binding should be done as in v2.

Yours,
	-- Matti

Re: [PATCH 2/2] iio: adc: ad7476: Support ROHM BU79100G

[Jonathan Cameron]

On Wed, 14 May 2025 12:21:30 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 14/05/2025 10:38, Matti Vaittinen wrote:
> > On 13/05/2025 11:26, Matti Vaittinen wrote:  
> >> ROHM BU79100G is a 12-bit, single channel ADC. Support reading ADC
> >> measurements using the ad7476.c
> >>
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> >> ---
> >>   drivers/iio/adc/ad7476.c | 8 ++++++++
> >>   1 file changed, 8 insertions(+)  
> > 
> > For anyone who might hit this mail thread later:
> > 
> > Conor made me realize that, for now, the BU79100G looks identical to the 
> > ads7866. Thus, these code-changes aren't needed at the moment, and this 
> > patch can be dropped. For those who wish to use BU79100G, please 
> > introduce it as
> > 
> > compatible = "rohm,bu79100g", "ti,ads7866";
> >   
> 
> I was too hasty.
> 
> It seems to me that the fallback won't work with the current driver 
> because the driver is not populating the of_match_table, but is relying 
> solely on the spi_device_id table.
> 
> Judging a quick code reading, the spi_driver_id table entries are 
> matched to the modalias:
> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/spi/spi.c#L393
> 
> Which is (as far as I understand), generated from the first compatible:
> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/of/base.c#L1170
> 
> and not from the fallback one.
> 
> I suppose this means that we would need to add the of_match_table entry 
> for the ti,ads7866 to make the fallback entry to match the driver.
> 
> But...
> 
> The __spi_register_driver() has following comment:
> 	/*
> 	 * For Really Good Reasons we use spi: modaliases not of:
> 	 * modaliases for DT so module autoloading won't work if we
> 	 * don't have a spi_device_id as well as a compatible string.
> 	 */
> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/spi/spi.c#L487
> 
> So, having the of_match_table for would not be sufficient for the 
> autoloading, which would still require the bu79100g to be in the 
> spi_device_id table.
> 
> Am I missing something? I don't see how the Linux SPI drivers benefit 
> from the fallback entries in the dt? (Not saying fallbacks wouldn't be 
> The Right Thing To Do. Ideally DTs aren't for Linux only, maybe some 
> other systems can utilize them). To me it seems I still need to add the 
> spi_device_id entry for the BU79100G, and of_match_table has no 
> additional benefit? If this is right, then this patch is still relevant, 
> even though the binding should be done as in v2.
+CC Mark Brown and linux-spi.


> 
> Yours,
> 	-- Matti
> 
> 
> 

Re: [PATCH 2/2] iio: adc: ad7476: Support ROHM BU79100G

[Matti Vaittinen]

On 15/05/2025 20:06, Jonathan Cameron wrote:
> On Wed, 14 May 2025 12:21:30 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> On 14/05/2025 10:38, Matti Vaittinen wrote:
>>> On 13/05/2025 11:26, Matti Vaittinen wrote:
>>>> ROHM BU79100G is a 12-bit, single channel ADC. Support reading ADC
>>>> measurements using the ad7476.c
>>>>
>>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>>> ---
>>>>    drivers/iio/adc/ad7476.c | 8 ++++++++
>>>>    1 file changed, 8 insertions(+)
>>>
>>> For anyone who might hit this mail thread later:
>>>
>>> Conor made me realize that, for now, the BU79100G looks identical to the
>>> ads7866. Thus, these code-changes aren't needed at the moment, and this
>>> patch can be dropped. For those who wish to use BU79100G, please
>>> introduce it as
>>>
>>> compatible = "rohm,bu79100g", "ti,ads7866";
>>>    
>>
>> I was too hasty.
>>
>> It seems to me that the fallback won't work with the current driver
>> because the driver is not populating the of_match_table, but is relying
>> solely on the spi_device_id table.
>>
>> Judging a quick code reading, the spi_driver_id table entries are
>> matched to the modalias:
>> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/spi/spi.c#L393
>>
>> Which is (as far as I understand), generated from the first compatible:
>> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/of/base.c#L1170
>>
>> and not from the fallback one.
>>
>> I suppose this means that we would need to add the of_match_table entry
>> for the ti,ads7866 to make the fallback entry to match the driver.
>>
>> But...
>>
>> The __spi_register_driver() has following comment:
>> 	/*
>> 	 * For Really Good Reasons we use spi: modaliases not of:
>> 	 * modaliases for DT so module autoloading won't work if we
>> 	 * don't have a spi_device_id as well as a compatible string.
>> 	 */
>> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/spi/spi.c#L487
>>
>> So, having the of_match_table for would not be sufficient for the
>> autoloading, which would still require the bu79100g to be in the
>> spi_device_id table.
>>
>> Am I missing something? I don't see how the Linux SPI drivers benefit
>> from the fallback entries in the dt? (Not saying fallbacks wouldn't be
>> The Right Thing To Do. Ideally DTs aren't for Linux only, maybe some
>> other systems can utilize them). To me it seems I still need to add the
>> spi_device_id entry for the BU79100G, and of_match_table has no
>> additional benefit? If this is right, then this patch is still relevant,
>> even though the binding should be done as in v2.
> +CC Mark Brown and linux-spi.

A quick test with device-tree having SPI device adc0:

adc: adc@0 {
	compatible = "rohm,bu79100g", "ti,ads7866";
};

yields:
root@arm:/home/debian# cat 
/sys/devices/platform/ocp/48000000.interconnect/48000000.interconnect:segment@0/48030000.target-module/48030000.spi/spi_master/spi0/spi0.0/modalias
spi:bu79100g

AFAICS, this means the module must have an ID for the 'bu79100g' - 
relying on the 'ti,ads7866' do not work.

Just for a comparison, (platform device) serial node:

uart0: serial@44e09000 {
	compatible = "ti,am3352-uart", "ti,omap3-uart";
};

yields:
root@arm:/home/debian# cat 
/sys/devices/platform/ocp/44c00000.interconnect/44c00000.interconnect:segment@200000/44e09050.target-module/44e09000.serial/modalias
of:NserialT(null)Cti,am3352-uartCti,omap3-uart

- which I suppose means the userland can match also modules which only 
have the 'ti,omap3-uart' fallback without the 'ti,am3352-uart'

I spent some quality time reading:

https://lore.kernel.org/all/564B2DD5.2060502@osg.samsung.com/
https://lkml.org/lkml/2014/9/19/179
https://lkml.org/lkml/2015/8/20/109

What I picked from it is that we have ... intersting history.

If I understood it right (please, correct me if I didn't):

- For historical reasons, the SPI core always sends spi:<foo> modalias, 
instead of device-tree variant of:vendor,<foo>
- The of-variant (from other subsystems) seems to be containing modalias 
matching all the compatibles, including potential fallback compatibles.
- The spi-variant seems to only contain a single modalias, matching the 
first compatible and not the fallback(s).
- The user-space uses the SPI one, or the OF one, not both. Eg, sending 
both wouldn't help unless user-space was changed.
- The SPI variant, for historical reasons, does not include vendor prefix.

=> Changing from SPI one to OF one has not been the way to go.

What I don't know is if the spi:<foo> modalias list could be expanded to 
also include the fallbacks from the device-tree, or if the user-space 
loading implementation(s) would work with multiple spi-modaliases.

Anyhow, it seems to me we don't currently have a de-facto way to utilize 
the device-tree fallback information in the SPI device drivers(?), but 
we need to populate all the IDs in the driver's ID list(s).

So, in order to (reliably) support the BU79100G (with currently existing 
user-space module loading), we need to add the ID for it in the driver 
(as was done in the v1)?

Any better opinions? :)

Yours,
	-- Matti