1. Patchew
  2. linux
  3. [PATCH 0/3] Support ROHM BD7910[0,1,2,3]
  4. View patch

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

Matti Vaittinen posted 3 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]
Posted by Matti Vaittinen 1 month, 3 weeks ago 
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
Re: [PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]
Posted by David Lechner 1 month, 3 weeks ago 
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]
Posted by Matti Vaittinen 1 month, 2 weeks ago 
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]
Posted by David Lechner 1 month, 2 weeks ago 
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]
Posted by Jonathan Cameron 1 month, 2 weeks ago 
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

>
Re: [PATCH 3/3] iio: adc: adc128s052: Support ROHM BD7910[0,1,2,3]
Posted by David Lechner 1 month, 2 weeks ago 
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.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.