diff mbox series

[v2,2/2] iio: adc: ad7173: add support for ad4113

Message ID 20240809-ad4113-v2-2-2a70c101a1f4@analog.com (mailing list archive)
State Superseded
Headers show
Series Add support for AD4113 | expand

Commit Message

Dumitru Ceclan via B4 Relay Aug. 9, 2024, 10:33 a.m. UTC 
From: Dumitru Ceclan <dumitru.ceclan@analog.com>

This commit adds support for the AD4113 ADC.
The AD4113 is a low power, low noise, 16-bit, Σ-Δ analog-to-digital
converter (ADC) that integrates an analog front end (AFE) for four
fully differential or eight single-ended inputs.

Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
 drivers/iio/adc/ad7173.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

Comments

Nuno Sá Aug. 9, 2024, 12:26 p.m. UTC | #1 
On Fri, 2024-08-09 at 13:33 +0300, Dumitru Ceclan via B4 Relay wrote:
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> 
> This commit adds support for the AD4113 ADC.
> The AD4113 is a low power, low noise, 16-bit, Σ-Δ analog-to-digital
> converter (ADC) that integrates an analog front end (AFE) for four
> fully differential or eight single-ended inputs.
> 
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> ---

Any reason to drop my tag :)? There a b4 command that can help you with it.

- Nuno Sá
Ceclan, Dumitru Aug. 9, 2024, 12:32 p.m. UTC | #2 
On 09/08/2024 15:26, Nuno Sá wrote:
> On Fri, 2024-08-09 at 13:33 +0300, Dumitru Ceclan via B4 Relay wrote:
>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>
>> This commit adds support for the AD4113 ADC.
>> The AD4113 is a low power, low noise, 16-bit, Σ-Δ analog-to-digital
>> converter (ADC) that integrates an analog front end (AFE) for four
>> fully differential or eight single-ended inputs.
>>
>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
>> ---
> 
> Any reason to drop my tag :)? There a b4 command that can help you with it.
> 
> - Nuno Sá
> 
> 

Yes, I added a new field to the device info struct and changed dt parsing
because I missed in V1 that this model actually has a 16 bit data register.
I considered that the rb would not apply anymore and it would need a re-review.

Thanks for the b4 suggestion but this was intentional :))
Nuno Sá Aug. 9, 2024, 2:12 p.m. UTC | #3 
On Fri, 2024-08-09 at 15:32 +0300, Ceclan, Dumitru wrote:
> On 09/08/2024 15:26, Nuno Sá wrote:
> > On Fri, 2024-08-09 at 13:33 +0300, Dumitru Ceclan via B4 Relay wrote:
> > > From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> > > 
> > > This commit adds support for the AD4113 ADC.
> > > The AD4113 is a low power, low noise, 16-bit, Σ-Δ analog-to-digital
> > > converter (ADC) that integrates an analog front end (AFE) for four
> > > fully differential or eight single-ended inputs.
> > > 
> > > Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> > > ---
> > 
> > Any reason to drop my tag :)? There a b4 command that can help you with it.
> > 
> > - Nuno Sá
> > 
> > 
> 
> Yes, I added a new field to the device info struct and changed dt parsing
> because I missed in V1 that this model actually has a 16 bit data register.
> I considered that the rb would not apply anymore and it would need a re-review.
> 
> Thanks for the b4 suggestion but this was intentional :))

Got it. Next time mention that in the cover...

Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Christophe JAILLET Aug. 9, 2024, 2:30 p.m. UTC | #4 
Le 09/08/2024 à 12:33, Dumitru Ceclan via B4 Relay a écrit :
> From: Dumitru Ceclan <dumitru.ceclan-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> 
> This commit adds support for the AD4113 ADC.
> The AD4113 is a low power, low noise, 16-bit, Σ-Δ analog-to-digital
> converter (ADC) that integrates an analog front end (AFE) for four
> fully differential or eight single-ended inputs.
> 
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> ---
>   drivers/iio/adc/ad7173.c | 36 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index a854f2d30174..3ac09d326472 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -3,7 +3,7 @@
>    * AD717x and AD411x family SPI ADC driver
>    *
>    * Supported devices:
> - *  AD4111/AD4112/AD4114/AD4115/AD4116
> + *  AD4111/AD4112/AD4113/AD4114/AD4115/AD4116
>    *  AD7172-2/AD7172-4/AD7173-8/AD7175-2
>    *  AD7175-8/AD7176-2/AD7177-2
>    *
> @@ -84,6 +84,7 @@
>   #define AD4111_ID			AD7173_ID
>   #define AD4112_ID			AD7173_ID
>   #define AD4114_ID			AD7173_ID
> +#define AD4113_ID			0x31D0

Nitpick: others are in lowercase --> 0x31d0

>   #define AD4116_ID			0x34d0
>   #define AD4115_ID			0x38d0
>   #define AD7175_8_ID			0x3cd0

Other than that, is there any reason to have this "random" order for 
these defines?

CJ
Ceclan, Dumitru Aug. 9, 2024, 2:40 p.m. UTC | #5 
On 09/08/2024 17:30, Christophe JAILLET wrote:
> Le 09/08/2024 à 12:33, Dumitru Ceclan via B4 Relay a écrit :
>> From: Dumitru Ceclan <dumitru.ceclan-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>>
>> This commit adds support for the AD4113 ADC.
>> The AD4113 is a low power, low noise, 16-bit, Σ-Δ analog-to-digital
>> converter (ADC) that integrates an analog front end (AFE) for four
>> fully differential or eight single-ended inputs.
>>
>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>> ---
>>   drivers/iio/adc/ad7173.c | 36 +++++++++++++++++++++++++++++++++++-
>>   1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
>> index a854f2d30174..3ac09d326472 100644
>> --- a/drivers/iio/adc/ad7173.c
>> +++ b/drivers/iio/adc/ad7173.c
>> @@ -3,7 +3,7 @@
>>    * AD717x and AD411x family SPI ADC driver
>>    *
>>    * Supported devices:
>> - *  AD4111/AD4112/AD4114/AD4115/AD4116
>> + *  AD4111/AD4112/AD4113/AD4114/AD4115/AD4116
>>    *  AD7172-2/AD7172-4/AD7173-8/AD7175-2
>>    *  AD7175-8/AD7176-2/AD7177-2
>>    *
>> @@ -84,6 +84,7 @@
>>   #define AD4111_ID            AD7173_ID
>>   #define AD4112_ID            AD7173_ID
>>   #define AD4114_ID            AD7173_ID
>> +#define AD4113_ID            0x31D0
> 
> Nitpick: others are in lowercase --> 0x31d0
> 
>>   #define AD4116_ID            0x34d0
>>   #define AD4115_ID            0x38d0
>>   #define AD7175_8_ID            0x3cd0
> 
> Other than that, is there any reason to have this "random" order for these defines?
> 
> CJ
> 

It's not random, it was requested to order these defines by the ID value:
https://lore.kernel.org/all/CAHp75VcjcgnLkQWim1AVnyeRGFwwKpaWSCvrmqdv41Lx87hMKw@mail.gmail.com/
Christophe JAILLET Aug. 9, 2024, 3:34 p.m. UTC | #6 
Le 09/08/2024 à 16:40, Ceclan, Dumitru a écrit :
> On 09/08/2024 17:30, Christophe JAILLET wrote:
>> Le 09/08/2024 à 12:33, Dumitru Ceclan via B4 Relay a écrit :
>>> From: Dumitru Ceclan <dumitru.ceclan-OyLXuOCK7orQT0dZR+AlfA-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>
>>>
>>> This commit adds support for the AD4113 ADC.
>>> The AD4113 is a low power, low noise, 16-bit, Σ-Δ analog-to-digital
>>> converter (ADC) that integrates an analog front end (AFE) for four
>>> fully differential or eight single-ended inputs.
>>>
>>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan-OyLXuOCK7orQT0dZR+AlfA-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>
>>> ---
>>>    drivers/iio/adc/ad7173.c | 36 +++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 35 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
>>> index a854f2d30174..3ac09d326472 100644
>>> --- a/drivers/iio/adc/ad7173.c
>>> +++ b/drivers/iio/adc/ad7173.c
>>> @@ -3,7 +3,7 @@
>>>     * AD717x and AD411x family SPI ADC driver
>>>     *
>>>     * Supported devices:
>>> - *  AD4111/AD4112/AD4114/AD4115/AD4116
>>> + *  AD4111/AD4112/AD4113/AD4114/AD4115/AD4116
>>>     *  AD7172-2/AD7172-4/AD7173-8/AD7175-2
>>>     *  AD7175-8/AD7176-2/AD7177-2
>>>     *
>>> @@ -84,6 +84,7 @@
>>>    #define AD4111_ID            AD7173_ID
>>>    #define AD4112_ID            AD7173_ID
>>>    #define AD4114_ID            AD7173_ID
>>> +#define AD4113_ID            0x31D0
>>
>> Nitpick: others are in lowercase --> 0x31d0
>>
>>>    #define AD4116_ID            0x34d0
>>>    #define AD4115_ID            0x38d0
>>>    #define AD7175_8_ID            0x3cd0
>>
>> Other than that, is there any reason to have this "random" order for these defines?
>>
>> CJ
>>
> 
> It's not random, it was requested to order these defines by the ID value:
> https://lore.kernel.org/all/CAHp75VcjcgnLkQWim1AVnyeRGFwwKpaWSCvrmqdv41Lx87hMKw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org/
> 

Ok,

I thought about it, but it is not sorted either. (a few lines above the 
place you added the new #define):

#define AD7175_ID			0x0cd0
#define AD7176_ID			0x0c90

Maybe, it should be part of another patch to keep the logic by ID value?

CJ
diff mbox series

Patch

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index a854f2d30174..3ac09d326472 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -3,7 +3,7 @@ 
  * AD717x and AD411x family SPI ADC driver
  *
  * Supported devices:
- *  AD4111/AD4112/AD4114/AD4115/AD4116
+ *  AD4111/AD4112/AD4113/AD4114/AD4115/AD4116
  *  AD7172-2/AD7172-4/AD7173-8/AD7175-2
  *  AD7175-8/AD7176-2/AD7177-2
  *
@@ -84,6 +84,7 @@ 
 #define AD4111_ID			AD7173_ID
 #define AD4112_ID			AD7173_ID
 #define AD4114_ID			AD7173_ID
+#define AD4113_ID			0x31D0
 #define AD4116_ID			0x34d0
 #define AD4115_ID			0x38d0
 #define AD7175_8_ID			0x3cd0
@@ -170,6 +171,7 @@  struct ad7173_device_info {
 	bool has_temp;
 	/* ((AVDD1 − AVSS)/5) */
 	bool has_pow_supply_monitoring;
+	bool data_reg_only_16bit;
 	bool has_input_buf;
 	bool has_int_ref;
 	bool has_ref2;
@@ -294,6 +296,24 @@  static const struct ad7173_device_info ad4112_device_info = {
 	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
 };
 
+static const struct ad7173_device_info ad4113_device_info = {
+	.name = "ad4113",
+	.id = AD4113_ID,
+	.num_voltage_in_div = 8,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_voltage_in = 8,
+	.num_gpios = 2,
+	.data_reg_only_16bit = true,
+	.higher_gpio_bits = true,
+	.has_vincom_input = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.clock = 2 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7173_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
 static const struct ad7173_device_info ad4114_device_info = {
 	.name = "ad4114",
 	.id = AD4114_ID,
@@ -988,6 +1008,13 @@  static const struct iio_info ad7173_info = {
 	.update_scan_mode = ad7173_update_scan_mode,
 };
 
+static const struct iio_scan_type ad4113_scan_type = {
+	.sign = 'u',
+	.realbits = 16,
+	.storagebits = 16,
+	.endianness = IIO_BE,
+};
+
 static const struct iio_chan_spec ad7173_channel_template = {
 	.type = IIO_VOLTAGE,
 	.indexed = 1,
@@ -1229,6 +1256,8 @@  static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
 		chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
 		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
+		if (st->info->data_reg_only_16bit)
+			chan_arr[chan_index].scan_type = ad4113_scan_type;
 
 		chan_index++;
 	}
@@ -1309,6 +1338,9 @@  static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 			chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
 		}
 
+		if (st->info->data_reg_only_16bit)
+			chan_arr[chan_index].scan_type = ad4113_scan_type;
+
 		chan_index++;
 	}
 	return 0;
@@ -1437,6 +1469,7 @@  static int ad7173_probe(struct spi_device *spi)
 static const struct of_device_id ad7173_of_match[] = {
 	{ .compatible = "adi,ad4111",	.data = &ad4111_device_info },
 	{ .compatible = "adi,ad4112",	.data = &ad4112_device_info },
+	{ .compatible = "adi,ad4113",	.data = &ad4113_device_info },
 	{ .compatible = "adi,ad4114",	.data = &ad4114_device_info },
 	{ .compatible = "adi,ad4115",	.data = &ad4115_device_info },
 	{ .compatible = "adi,ad4116",	.data = &ad4116_device_info },
@@ -1454,6 +1487,7 @@  MODULE_DEVICE_TABLE(of, ad7173_of_match);
 static const struct spi_device_id ad7173_id_table[] = {
 	{ "ad4111",   (kernel_ulong_t)&ad4111_device_info },
 	{ "ad4112",   (kernel_ulong_t)&ad4112_device_info },
+	{ "ad4113",   (kernel_ulong_t)&ad4113_device_info },
 	{ "ad4114",   (kernel_ulong_t)&ad4114_device_info },
 	{ "ad4115",   (kernel_ulong_t)&ad4115_device_info },
 	{ "ad4116",   (kernel_ulong_t)&ad4116_device_info },