[PATCH 0/3] Support ROHM BD7910[0,1,2,3]

[PATCH 0/3] Support ROHM BD7910[0,1,2,3]

[Matti Vaittinen]

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

Add support for ROHM BD7910[0,1,2,3] ADCs.

The ROHM BD79100, BD79101, BD79102 and BD79103 are ADCs derived from the
BD79104. According to the data-sheets, the BD79103 is compatible with the
BD79104. Rest of the ICs have different number of analog input channels.

This series adds support for these ICs using the ti-adc128s052.c.

NOTE: There has been work on couple of other patch series [1][2] touching
this same driver. I haven't considered those changes because, AFAICS,
there has been no new revisions of these series since mid June.

[1]: https://lore.kernel.org/all/20250614091504.575685-1-sbellary@baylibre.com/
[2]: https://lore.kernel.org/all/20250625170218.545654-2-l.rubusch@gmail.com/


Matti Vaittinen (3):
  dt-bindings: iio: adc: Add BD7910[0,1,2,3]
  iio: adc: adc128s052: Simplify matching chip_data
  iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]

 .../bindings/iio/adc/rohm,bd79104.yaml        |  11 +-
 drivers/iio/adc/ti-adc128s052.c               | 114 ++++++++++++------
 2 files changed, 87 insertions(+), 38 deletions(-)


base-commit: 856d7be7f3c459a6d646b1f8432c6f616ade0d10
-- 
2.50.1


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

[PATCH 1/3] dt-bindings: iio: adc: Add BD7910[0,1,2,3]

[Matti Vaittinen]

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

The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
and, based on the data sheets, they seem identical from the software
point-of-view.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 .../devicetree/bindings/iio/adc/rohm,bd79104.yaml     | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml b/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
index f0a1347ba4db..6a6e6ab4aca3 100644
--- a/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
@@ -14,7 +14,16 @@ description: |
 
 properties:
   compatible:
-    const: rohm,bd79104
+    oneOf:
+      - items:
+          - enum:
+              - rohm,bd79100
+              - rohm,bd79101
+              - rohm,bd79102
+              - rohm,bd79104
+      - items:
+          - const: rohm,bd79104
+          - const: rohm,bd79103
 
   reg:
     maxItems: 1
-- 
2.50.1


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

Re: [PATCH 1/3] dt-bindings: iio: adc: Add BD7910[0,1,2,3]

[Matti Vaittinen]

On 14/08/2025 11:35, Matti Vaittinen wrote:
> The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
> ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
> the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
> and, based on the data sheets, they seem identical from the software
> point-of-view.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
>   .../devicetree/bindings/iio/adc/rohm,bd79104.yaml     | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml b/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
> index f0a1347ba4db..6a6e6ab4aca3 100644
> --- a/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
> @@ -14,7 +14,16 @@ description: |
>   
>   properties:
>     compatible:
> -    const: rohm,bd79104
> +    oneOf:
> +      - items:
> +          - enum:
> +              - rohm,bd79100
> +              - rohm,bd79101
> +              - rohm,bd79102
> +              - rohm,bd79104
> +      - items:
> +          - const: rohm,bd79104
> +          - const: rohm,bd79103

Oops. I believe the order of the compatibles is wrong for the fallback.

>   
>     reg:
>       maxItems: 1

Yours,
	-- Matti

Re: [PATCH 1/3] dt-bindings: iio: adc: Add BD7910[0,1,2,3]

[David Lechner]

On 8/14/25 4:57 AM, Matti Vaittinen wrote:
> On 14/08/2025 11:35, Matti Vaittinen wrote:
>> The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
>> ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
>> the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,

Is it just a difference in max sample rate? or pinout?

>> and, based on the data sheets, they seem identical from the software
>> point-of-view.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> ---
>>   .../devicetree/bindings/iio/adc/rohm,bd79104.yaml     | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml b/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
>> index f0a1347ba4db..6a6e6ab4aca3 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
>> @@ -14,7 +14,16 @@ description: |
>>     properties:
>>     compatible:
>> -    const: rohm,bd79104
>> +    oneOf:
>> +      - items:

You can drop the items: here since there is only one item.

>> +          - enum:
>> +              - rohm,bd79100
>> +              - rohm,bd79101
>> +              - rohm,bd79102
>> +              - rohm,bd79104
>> +      - items:
>> +          - const: rohm,bd79104
>> +          - const: rohm,bd79103
> 
> Oops. I believe the order of the compatibles is wrong for the fallback.

Indeed.

> 
>>       reg:
>>       maxItems: 1
> 
> Yours,
>     -- Matti

Re: [PATCH 1/3] dt-bindings: iio: adc: Add BD7910[0,1,2,3]

[Matti Vaittinen]

On 14/08/2025 17:51, David Lechner wrote:
> On 8/14/25 4:57 AM, Matti Vaittinen wrote:
>> On 14/08/2025 11:35, Matti Vaittinen wrote:
>>> The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
>>> ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
>>> the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
> 
> Is it just a difference in max sample rate? or pinout?

The BD79104 comes in 2 different packages (BD79104MUF - VQFN16FV3030 and 
BD79104FV - SSOP-B16).

BD79103MUF pins seem identical to the BD79104MUF pins. Not sure if there 
is (or will be) BD79103FV. (Both the MUF and FV have identically named 
pins, with same functionality. Only the pin positioning and potentially 
amount of unused pins differ).

So, looking at the functionality, the pinout is same. Looking at the 
physical IC, they may use different packaging for these ICs.

Finally, the only difference between BD79104 and BD79103 I have been 
able to find from the data-sheet is the differential nonlinearity.

For BD79104 it is said to be:
DNL: +1.2 / -0.99 LSB @ VDD = 3 V (Typ)

For BD79103 it is said to be:
DNL: ±0.99 LSB @ VDD = 3 V (Typ)

> 
>>> and, based on the data sheets, they seem identical from the software
>>> point-of-view.
>>>
>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>> ---
>>>    .../devicetree/bindings/iio/adc/rohm,bd79104.yaml     | 11 ++++++++++-
>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml b/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
>>> index f0a1347ba4db..6a6e6ab4aca3 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/adc/rohm,bd79104.yaml
>>> @@ -14,7 +14,16 @@ description: |
>>>      properties:
>>>      compatible:
>>> -    const: rohm,bd79104
>>> +    oneOf:
>>> +      - items:
> 
> You can drop the items: here since there is only one item.

Thanks.

> 
>>> +          - enum:
>>> +              - rohm,bd79100
>>> +              - rohm,bd79101
>>> +              - rohm,bd79102
>>> +              - rohm,bd79104
>>> +      - items:
>>> +          - const: rohm,bd79104
>>> +          - const: rohm,bd79103
>>
>> Oops. I believe the order of the compatibles is wrong for the fallback.
> 
> Indeed.

Probably needless to say, but I'll fix this for the next version :)

Thanks for the review David!

Yours,
	-- Matti

[PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data

[Matti Vaittinen]

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

The adc128s052 driver supports a few different ICs. IC specific
configuration data is stored in an array. IC data, residing in a
specific point of the array, is pointed from the SPI device match data.

There is no need to have the chip config data structures in an array
and splitting them out of an array has at least following benefits:

- Chip-specific structures can be named after the chips they support.
  This makes referring them a tad cleaner, compared to using a generic
  array name with a numerical index.

- Avoid all potential 'out of bounds' errors which can result if the
  array is changed.

Split the chip configuration data array to individual structures.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
There are couple of other series[1][2] changing this driver ongoing. I
haven't heard of those latelty (after JUN?) so those changes haven't
been taken into account and will conflict if those serieses are re-spun.
Please, let me know if I should rebase my changes.

[1]: https://lore.kernel.org/all/20250614091504.575685-1-sbellary@baylibre.com/
[2]: https://lore.kernel.org/all/20250625170218.545654-2-l.rubusch@gmail.com/
---
 drivers/iio/adc/ti-adc128s052.c | 78 +++++++++++++++++----------------
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index 1b46a8155803..81153253529e 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -124,26 +124,30 @@ static const struct iio_chan_spec adc124s021_channels[] = {
 
 static const char * const bd79104_regulators[] = { "iovdd" };
 
-static const struct adc128_configuration adc128_config[] = {
-	{
-		.channels = adc128s052_channels,
-		.num_channels = ARRAY_SIZE(adc128s052_channels),
-		.refname = "vref",
-	}, {
-		.channels = adc122s021_channels,
-		.num_channels = ARRAY_SIZE(adc122s021_channels),
-		.refname = "vref",
-	}, {
-		.channels = adc124s021_channels,
-		.num_channels = ARRAY_SIZE(adc124s021_channels),
-		.refname = "vref",
-	}, {
-		.channels = adc128s052_channels,
-		.num_channels = ARRAY_SIZE(adc128s052_channels),
-		.refname = "vdd",
-		.other_regulators = &bd79104_regulators,
-		.num_other_regulators = 1,
-	},
+static const struct adc128_configuration adc122s_config = {
+	.channels = adc122s021_channels,
+	.num_channels = ARRAY_SIZE(adc122s021_channels),
+	.refname = "vref",
+};
+
+static const struct adc128_configuration adc124s_config = {
+	.channels = adc124s021_channels,
+	.num_channels = ARRAY_SIZE(adc124s021_channels),
+	.refname = "vref",
+};
+
+static const struct adc128_configuration adc128s_config = {
+	.channels = adc128s052_channels,
+	.num_channels = ARRAY_SIZE(adc128s052_channels),
+	.refname = "vref",
+};
+
+static const struct adc128_configuration bd79104_config = {
+	.channels = adc128s052_channels,
+	.num_channels = ARRAY_SIZE(adc128s052_channels),
+	.refname = "vdd",
+	.other_regulators = &bd79104_regulators,
+	.num_other_regulators = 1,
 };
 
 static const struct iio_info adc128_info = {
@@ -199,33 +203,33 @@ static int adc128_probe(struct spi_device *spi)
 }
 
 static const struct of_device_id adc128_of_match[] = {
-	{ .compatible = "ti,adc128s052", .data = &adc128_config[0] },
-	{ .compatible = "ti,adc122s021", .data = &adc128_config[1] },
-	{ .compatible = "ti,adc122s051", .data = &adc128_config[1] },
-	{ .compatible = "ti,adc122s101", .data = &adc128_config[1] },
-	{ .compatible = "ti,adc124s021", .data = &adc128_config[2] },
-	{ .compatible = "ti,adc124s051", .data = &adc128_config[2] },
-	{ .compatible = "ti,adc124s101", .data = &adc128_config[2] },
-	{ .compatible = "rohm,bd79104", .data = &adc128_config[3] },
+	{ .compatible = "ti,adc128s052", .data = &adc128s_config },
+	{ .compatible = "ti,adc122s021", .data = &adc122s_config },
+	{ .compatible = "ti,adc122s051", .data = &adc122s_config },
+	{ .compatible = "ti,adc122s101", .data = &adc122s_config },
+	{ .compatible = "ti,adc124s021", .data = &adc124s_config },
+	{ .compatible = "ti,adc124s051", .data = &adc124s_config },
+	{ .compatible = "ti,adc124s101", .data = &adc124s_config },
+	{ .compatible = "rohm,bd79104", .data = &bd79104_config },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, adc128_of_match);
 
 static const struct spi_device_id adc128_id[] = {
-	{ "adc128s052", (kernel_ulong_t)&adc128_config[0] },
-	{ "adc122s021",	(kernel_ulong_t)&adc128_config[1] },
-	{ "adc122s051",	(kernel_ulong_t)&adc128_config[1] },
-	{ "adc122s101",	(kernel_ulong_t)&adc128_config[1] },
-	{ "adc124s021", (kernel_ulong_t)&adc128_config[2] },
-	{ "adc124s051", (kernel_ulong_t)&adc128_config[2] },
-	{ "adc124s101", (kernel_ulong_t)&adc128_config[2] },
-	{ "bd79104", (kernel_ulong_t)&adc128_config[3] },
+	{ "adc128s052", (kernel_ulong_t)&adc128s_config },
+	{ "adc122s021",	(kernel_ulong_t)&adc122s_config },
+	{ "adc122s051",	(kernel_ulong_t)&adc122s_config },
+	{ "adc122s101",	(kernel_ulong_t)&adc122s_config },
+	{ "adc124s021", (kernel_ulong_t)&adc124s_config },
+	{ "adc124s051", (kernel_ulong_t)&adc124s_config },
+	{ "adc124s101", (kernel_ulong_t)&adc124s_config },
+	{ "bd79104", (kernel_ulong_t)&bd79104_config },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, adc128_id);
 
 static const struct acpi_device_id adc128_acpi_match[] = {
-	{ "AANT1280", (kernel_ulong_t)&adc128_config[2] },
+	{ "AANT1280", (kernel_ulong_t)&adc124s_config },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
-- 
2.50.1


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

Re: [PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data

[David Lechner]

On 8/14/25 3:35 AM, Matti Vaittinen wrote:
> The adc128s052 driver supports a few different ICs. IC specific
> configuration data is stored in an array. IC data, residing in a
> specific point of the array, is pointed from the SPI device match data.
> 
> There is no need to have the chip config data structures in an array
> and splitting them out of an array has at least following benefits:
> 
> - Chip-specific structures can be named after the chips they support.
>   This makes referring them a tad cleaner, compared to using a generic
>   array name with a numerical index.
> 
> - Avoid all potential 'out of bounds' errors which can result if the
>   array is changed.
> 
> Split the chip configuration data array to individual structures.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data

[Jonathan Cameron]

On Thu, 14 Aug 2025 09:53:21 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 8/14/25 3:35 AM, Matti Vaittinen wrote:
> > The adc128s052 driver supports a few different ICs. IC specific
> > configuration data is stored in an array. IC data, residing in a
> > specific point of the array, is pointed from the SPI device match data.
> > 
> > There is no need to have the chip config data structures in an array
> > and splitting them out of an array has at least following benefits:
> > 
> > - Chip-specific structures can be named after the chips they support.
> >   This makes referring them a tad cleaner, compared to using a generic
> >   array name with a numerical index.
> > 
> > - Avoid all potential 'out of bounds' errors which can result if the
> >   array is changed.
> > 
> > Split the chip configuration data array to individual structures.
> > 
> > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > 
> > ---  
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 
Any racing series get to rebase on top of this.

Applied this patch as it is good in it's own right.

Thanks,

Jonathan

Re: [PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data

[Jonathan Cameron]

On Sat, 16 Aug 2025 13:28:23 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 14 Aug 2025 09:53:21 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > On 8/14/25 3:35 AM, Matti Vaittinen wrote:  
> > > The adc128s052 driver supports a few different ICs. IC specific
> > > configuration data is stored in an array. IC data, residing in a
> > > specific point of the array, is pointed from the SPI device match data.
> > > 
> > > There is no need to have the chip config data structures in an array
> > > and splitting them out of an array has at least following benefits:
> > > 
> > > - Chip-specific structures can be named after the chips they support.
> > >   This makes referring them a tad cleaner, compared to using a generic
> > >   array name with a numerical index.
> > > 
> > > - Avoid all potential 'out of bounds' errors which can result if the
> > >   array is changed.
> > > 
> > > Split the chip configuration data array to individual structures.
> > > 
> > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > > 
> > > ---    
> > Reviewed-by: David Lechner <dlechner@baylibre.com>
> >   
> Any racing series get to rebase on top of this.
> 
> Applied this patch as it is good in it's own right.
Dropped again given rename discussion on patch 3.
> 
> Thanks,
> 
> Jonathan
> 

[PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]

[Matti Vaittinen]

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

The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
and, based on the data sheets, they seem identical from the software
point-of-view.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
Tested only using the BD79104. The ROHM hardware colleagues swore this
testing should be sufficient...
---
 drivers/iio/adc/ti-adc128s052.c | 36 +++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index 81153253529e..2f2ed438cf4e 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -122,6 +122,10 @@ static const struct iio_chan_spec adc124s021_channels[] = {
 	ADC128_VOLTAGE_CHANNEL(3),
 };
 
+static const struct iio_chan_spec bd79100_channels[] = {
+	ADC128_VOLTAGE_CHANNEL(0),
+};
+
 static const char * const bd79104_regulators[] = { "iovdd" };
 
 static const struct adc128_configuration adc122s_config = {
@@ -142,6 +146,30 @@ static const struct adc128_configuration adc128s_config = {
 	.refname = "vref",
 };
 
+static const struct adc128_configuration bd79100_config = {
+	.channels = bd79100_channels,
+	.num_channels = ARRAY_SIZE(bd79100_channels),
+	.refname = "vdd",
+	.other_regulators = &bd79104_regulators,
+	.num_other_regulators = 1,
+};
+
+static const struct adc128_configuration bd79101_config = {
+	.channels = adc122s021_channels,
+	.num_channels = ARRAY_SIZE(adc122s021_channels),
+	.refname = "vdd",
+	.other_regulators = &bd79104_regulators,
+	.num_other_regulators = 1,
+};
+
+static const struct adc128_configuration bd79102_config = {
+	.channels = adc124s021_channels,
+	.num_channels = ARRAY_SIZE(adc124s021_channels),
+	.refname = "vdd",
+	.other_regulators = &bd79104_regulators,
+	.num_other_regulators = 1,
+};
+
 static const struct adc128_configuration bd79104_config = {
 	.channels = adc128s052_channels,
 	.num_channels = ARRAY_SIZE(adc128s052_channels),
@@ -210,6 +238,10 @@ static const struct of_device_id adc128_of_match[] = {
 	{ .compatible = "ti,adc124s021", .data = &adc124s_config },
 	{ .compatible = "ti,adc124s051", .data = &adc124s_config },
 	{ .compatible = "ti,adc124s101", .data = &adc124s_config },
+	{ .compatible = "rohm,bd79100", .data = &bd79100_config },
+	{ .compatible = "rohm,bd79101", .data = &bd79101_config },
+	{ .compatible = "rohm,bd79102", .data = &bd79102_config },
+	{ .compatible = "rohm,bd79103", .data = &bd79104_config },
 	{ .compatible = "rohm,bd79104", .data = &bd79104_config },
 	{ }
 };
@@ -223,6 +255,10 @@ static const struct spi_device_id adc128_id[] = {
 	{ "adc124s021", (kernel_ulong_t)&adc124s_config },
 	{ "adc124s051", (kernel_ulong_t)&adc124s_config },
 	{ "adc124s101", (kernel_ulong_t)&adc124s_config },
+	{ "bd79100", (kernel_ulong_t)&bd79100_config },
+	{ "bd79101", (kernel_ulong_t)&bd79101_config },
+	{ "bd79102", (kernel_ulong_t)&bd79102_config },
+	{ "bd79103", (kernel_ulong_t)&bd79104_config },
 	{ "bd79104", (kernel_ulong_t)&bd79104_config },
 	{ }
 };
-- 
2.50.1


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

Re: [PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]

[David Lechner]

On 8/14/25 3:35 AM, Matti Vaittinen wrote:
> The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
> ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
> the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
> and, based on the data sheets, they seem identical from the software
> point-of-view.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---

One small suggestion. With that:

Reviewed-by: David Lechner <dlechner@baylibre.com>

> ---
>  drivers/iio/adc/ti-adc128s052.c | 36 +++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> index 81153253529e..2f2ed438cf4e 100644
> --- a/drivers/iio/adc/ti-adc128s052.c
> +++ b/drivers/iio/adc/ti-adc128s052.c
> @@ -122,6 +122,10 @@ static const struct iio_chan_spec adc124s021_channels[] = {
>  	ADC128_VOLTAGE_CHANNEL(3),
>  };
>  
> +static const struct iio_chan_spec bd79100_channels[] = {

Even though the driver doesn't support it yet, there is a
adc121s021 [1] so would be nice to use that instead of bd79100
to keep the naming consistent.

[1]: https://www.ti.com/product/ADC121C021

> +	ADC128_VOLTAGE_CHANNEL(0),
> +};
> +

Re: [PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]

[Matti Vaittinen]

On 14/08/2025 18:01, David Lechner wrote:
> On 8/14/25 3:35 AM, Matti Vaittinen wrote:
>> The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
>> ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
>> the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
>> and, based on the data sheets, they seem identical from the software
>> point-of-view.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
> 
> One small suggestion. With that:
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 
>> ---
>>   drivers/iio/adc/ti-adc128s052.c | 36 +++++++++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
>> index 81153253529e..2f2ed438cf4e 100644
>> --- a/drivers/iio/adc/ti-adc128s052.c
>> +++ b/drivers/iio/adc/ti-adc128s052.c
>> @@ -122,6 +122,10 @@ static const struct iio_chan_spec adc124s021_channels[] = {
>>   	ADC128_VOLTAGE_CHANNEL(3),
>>   };
>>   
>> +static const struct iio_chan_spec bd79100_channels[] = {
> 
> Even though the driver doesn't support it yet, there is a
> adc121s021 [1] so would be nice to use that instead of bd79100
> to keep the naming consistent.

I have to disagree on this one. For people who don't use the TI ADCs, 
the TI numbering does not bring any clarity. Furthermore, I don't like 
preparing for the support added somewhere in the future - because future 
is uncertain. It could be this TI's variant never gets added here. If 
this series gets merged now, then there is only one IC using this 
channel spec - the bd79100. Naming it after unsupported TI's IC would be 
plain confusing.

In my opinion, structs should get either named based on the IC model 
which is using them first - or based on the functionality. And actually, 
when the design of the IC is not too obscure, I would prefer naming 
based on the functionality, which should help others to re-use the 
driver. Hence, I wouldn't object someone re-naming all these channel 
structs based on functionality though - for example something like:

static const struct iio_chan_spec simple_adc_channels1 {}
static const struct iio_chan_spec simple_adc_channels2 {}
static const struct iio_chan_spec simple_adc_channels4 {}
static const struct iio_chan_spec simple_adc_channels8 {}

This which should be clear(ish) for developer no matter which of the 
supported IC(s) were used. But if we stick with the IC based naming, 
then we should use naming by supported IC.

> 
> [1]: https://www.ti.com/product/ADC121C021
> 
>> +	ADC128_VOLTAGE_CHANNEL(0),
>> +};
>> +

Yours,
	-- Matti

Re: [PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]

[David Lechner]

On 8/15/25 12:23 AM, Matti Vaittinen wrote:
> On 14/08/2025 18:01, David Lechner wrote:
>> On 8/14/25 3:35 AM, Matti Vaittinen wrote:
>>> The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
>>> ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
>>> the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
>>> and, based on the data sheets, they seem identical from the software
>>> point-of-view.
>>>
>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>>
>>> ---
>>
>> One small suggestion. With that:
>>
>> Reviewed-by: David Lechner <dlechner@baylibre.com>
>>
>>> ---
>>>   drivers/iio/adc/ti-adc128s052.c | 36 +++++++++++++++++++++++++++++++++
>>>   1 file changed, 36 insertions(+)
>>>
>>> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
>>> index 81153253529e..2f2ed438cf4e 100644
>>> --- a/drivers/iio/adc/ti-adc128s052.c
>>> +++ b/drivers/iio/adc/ti-adc128s052.c
>>> @@ -122,6 +122,10 @@ static const struct iio_chan_spec adc124s021_channels[] = {
>>>       ADC128_VOLTAGE_CHANNEL(3),
>>>   };
>>>   +static const struct iio_chan_spec bd79100_channels[] = {
>>
>> Even though the driver doesn't support it yet, there is a
>> adc121s021 [1] so would be nice to use that instead of bd79100
>> to keep the naming consistent.
> 
> I have to disagree on this one. For people who don't use the TI ADCs, the TI numbering does not bring any clarity. 


I think it does in this case because the part number includes the bits
and number of channels. And the pattern is pretty easy to spot without
looking at the datasheets. This is why I suggested it. Otherwise, I would
agree with your points in general.

Re: [PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]

[David Lechner]

On 8/15/25 12:23 AM, Matti Vaittinen wrote:
> On 14/08/2025 18:01, David Lechner wrote:
>> On 8/14/25 3:35 AM, Matti Vaittinen wrote:
>>> The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
>>> ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
>>> the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
>>> and, based on the data sheets, they seem identical from the software
>>> point-of-view.
>>>
>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>>

...

> static const struct iio_chan_spec simple_adc_channels1 {}
> static const struct iio_chan_spec simple_adc_channels2 {}
> static const struct iio_chan_spec simple_adc_channels4 {}
> static const struct iio_chan_spec simple_adc_channels8 {}
> 
> This which should be clear(ish) for developer no matter which of the supported IC(s) were used. But if we stick with the IC based naming, then we should use naming by supported IC.
> 
>>
Even better.

Re: [PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]

[Jonathan Cameron]

On Fri, 15 Aug 2025 09:17:29 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 8/15/25 12:23 AM, Matti Vaittinen wrote:
> > On 14/08/2025 18:01, David Lechner wrote:  
> >> On 8/14/25 3:35 AM, Matti Vaittinen wrote:  
> >>> The ROHM BD79100, BD79101, BD79102, BD79103 are very similar ADCs as the
> >>> ROHM BD79104. The BD79100 has only 1 channel. BD79101 has 2 channels and
> >>> the BD79102 has 4 channels. Both BD79103 and BD79104 have 4 channels,
> >>> and, based on the data sheets, they seem identical from the software
> >>> point-of-view.
> >>>
> >>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> >>>  
> 
> ...
> 
> > static const struct iio_chan_spec simple_adc_channels1 {}
> > static const struct iio_chan_spec simple_adc_channels2 {}
> > static const struct iio_chan_spec simple_adc_channels4 {}
> > static const struct iio_chan_spec simple_adc_channels8 {}
> > 
> > This which should be clear(ish) for developer no matter which of the supported IC(s) were used. But if we stick with the IC based naming, then we should use naming by supported IC.
> >   
> >>  
> Even better.
Agreed that's appropriate for this driver so do that rename as a precursor.

Given you get to choose the order I've dropped patch 2 for now. Don't mind
if that goes in first or this rename does. 

Jonathan

>