linux-kernel - Re: [PATCH 2/6] iio: adc: ad4030: add driver for ad4030-24

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D4554LAXBVEX.2Z0H6XMUIPOUB@baylibre.com>
Date: Fri, 13 Sep 2024 12:04:08 +0000
From: "Esteban Blanc" <eblanc@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>, "David Lechner"
 <dlechner@...libre.com>, "Lars-Peter Clausen" <lars@...afoo.de>, "Michael
 Hennerich" <Michael.Hennerich@...log.com>, "Jonathan Cameron"
 <jic23@...nel.org>, "Rob Herring" <robh@...nel.org>, "Krzysztof Kozlowski"
 <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>, "Nuno Sa"
 <nuno.sa@...log.com>, "Jonathan Corbet" <corbet@....net>
Cc: <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>
Subject: Re: [PATCH 2/6] iio: adc: ad4030: add driver for ad4030-24

On Fri Sep 13, 2024 at 10:22 AM UTC, Nuno Sá wrote:
> Hi Esteban,
>
> Just one remark...
>
> On Thu, 2024-08-22 at 14:39 -0500, David Lechner wrote:
> > On 8/22/24 7:45 AM, Esteban Blanc wrote:
> > > This adds a new driver for the Analog Devices INC. AD4030-24 ADC.
> > > 
> > > The driver implements basic support for the AD4030-24 1 channel
> > > differential ADC with hardware gain and offset control.
> > > 
> > > Signed-off-by: Esteban Blanc <eblanc@...libre.com>
> > > ---
> > >  MAINTAINERS              |   1 +
> > >  drivers/iio/adc/Kconfig  |  13 +
> > >  drivers/iio/adc/Makefile |   1 +
> > >  drivers/iio/adc/ad4030.c | 854 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 869 insertions(+)
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index f17c42bea19c..6a5a0e7b7a51 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -420,6 +420,7 @@ R:	Esteban Blanc <eblanc@...libre.com>
> > >  S:	Supported
> > >  W:	https://ez.analog.com/linux-software-drivers
> > >  F:	Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
> > > +F:	drivers/iio/adc/ad4030.c
> > >  
> > >  AD5110 ANALOG DEVICES DIGITAL POTENTIOMETERS DRIVER
> > >  M:	Mugilraj Dhavachelvan <dmugil2000@...il.com>
> > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > index 88e8ce2e78b3..f4bd05780f6f 100644
> > > --- a/drivers/iio/adc/Kconfig
> > > +++ b/drivers/iio/adc/Kconfig
> > > @@ -33,6 +33,19 @@ config AD4000
> > >  	  To compile this driver as a module, choose M here: the module will be
> > >  	  called ad4000.
> > >  
> > > +config AD4030
> > > +	tristate "Analog Device AD4630 ADC Driver"
> > > +	depends on SPI
> > > +	depends on GPIOLIB
> > > +	select REGMAP_SPI
> > 
> > It looks like we are just using REGMAP, not REGMAP_SPI.
> > 
> > > +	select IIO_BUFFER
> > 
> > And also select IIO_TRIGGERED_BUFFER?
> > 
> > > +	help
> > > +	  Say yes here to build support for Analog Devices AD4030 and AD4630
> > > high speed
> > > +	  SPI analog to digital converters (ADC).
> > > +
> > > +	  To compile this driver as a module, choose M here: the module will be
> > > +	  called ad4030.
> > > +
> > >  config AD4130
> > >  	tristate "Analog Device AD4130 ADC Driver"
> > >  	depends on SPI
> > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > > index 8b80664c6d6b..0e4f833abf0b 100644
> > > --- a/drivers/iio/adc/Makefile
> > > +++ b/drivers/iio/adc/Makefile
> > > @@ -7,6 +7,7 @@
> > >  obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
> > >  obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> > >  obj-$(CONFIG_AD4000) += ad4000.o
> > > +obj-$(CONFIG_AD4030) += ad4030.o
> > >  obj-$(CONFIG_AD4130) += ad4130.o
> > >  obj-$(CONFIG_AD4695) += ad4695.o
> > >  obj-$(CONFIG_AD7091R) += ad7091r-base.o
> > > diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> > > new file mode 100644
> > > index 000000000000..a981dce988e5
> > > --- /dev/null
> > > +++ b/drivers/iio/adc/ad4030.c
> > > @@ -0,0 +1,854 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Analog Devices AD4030 and AD4630 ADC family driver.
> > > + *
> > > + * Copyright 2024 Analog Devices, Inc.
> > > + * Copyright 2024 BayLibre, SAS
> > > + *
> > > + * based on code from:
> > > + *	Analog Devices, Inc.
> > > + *	  Sergiu Cuciurean <sergiu.cuciurean@...log.com>
> > > + *	  Nuno Sa <nuno.sa@...log.com>
> > > + *	  Marcelo Schmitt <marcelo.schmitt@...log.com>
> > > + *	  Liviu Adace <liviu.adace@...log.com>
> > > + */
> > > +	.type = IIO_VOLTAGE,						\
> > > +	.indexed = 1,							\
> > > +	.channel = _idx * 2 + 2,					\
> > > +	.scan_index = _idx * 2 + 1,					\
> > > +	.extend_name = "Channel" #_idx " common byte part",		\
> > 
> > Labels are usually one word and reflect the datasheet name.
> > 
> > Suggest `"common-mode" #_idx` or `"CM" #_idx` for this one.
> > 
>
> Also, .extend_name is not to be used anymore... In the end of the day IIO will create
> label files anyways but from what I remember about this, extend_name is not to be
> directly used this anymore (so other think it's still fine). Instead, use the label
> callback.

Yeah, Jonathan or David mentioned it. I'm using `.read_label` callback
now.

Thanks for your time,

-- 
Esteban Blanc
BayLibre