| 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
| ||
|
Message-ID:
<OSZPR01MB87987A7D3F418A6E7A24FC41851EA@OSZPR01MB8798.jpnprd01.prod.outlook.com>
Date: Fri, 26 Sep 2025 12:41:46 +0000
From: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
To: Nuno Sá <noname.nuno@...il.com>
CC: Jonathan Cameron <jic23@...nel.org>, David Lechner
<dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>, Andy
Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof
Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Geert
Uytterhoeven <geert+renesas@...der.be>, magnus.damm <magnus.damm@...il.com>,
"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 3/7] iio: adc: add RZ/T2H / RZ/N2H ADC driver
> -----Original Message-----
> From: Nuno Sá <noname.nuno@...il.com>
> Sent: Friday, September 26, 2025 3:11 PM
> To: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> Cc: Jonathan Cameron <jic23@...nel.org>; David Lechner <dlechner@...libre.com>; Nuno Sá
> <nuno.sa@...log.com>; Andy Shevchenko <andy@...nel.org>; Rob Herring <robh@...nel.org>; Krzysztof
> Kozlowski <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; Geert Uytterhoeven
> <geert+renesas@...der.be>; magnus.damm <magnus.damm@...il.com>; linux-iio@...r.kernel.org; linux-
> renesas-soc@...r.kernel.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v2 3/7] iio: adc: add RZ/T2H / RZ/N2H ADC driver
>
> On Fri, 2025-09-26 at 01:40 +0300, Cosmin Tanislav wrote:
> > Add support for the A/D 12-Bit successive approximation converters found
> > in the Renesas RZ/T2H (R9A09G077) and RZ/N2H (R9A09G087) SoCs.
> >
> > RZ/T2H has two ADCs with 4 channels and one with 6.
> > RZ/N2H has two ADCs with 4 channels and one with 15.
> >
> > Conversions can be performed in single or continuous mode. Result of the
> > conversion is stored in a 16-bit data register corresponding to each
> > channel.
> >
> > The conversions can be started by a software trigger, a synchronous
> > trigger (from MTU or from ELC) or an asynchronous external trigger (from
> > ADTRGn# pin).
> >
> > Only single mode with software trigger is supported for now.
> >
> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> > ---
>
> Just one small nit from me. With it:
>
> Reviewed-by: Nuno Sá <nuno.sa@...log.com>
>
> > MAINTAINERS | 1 +
> > drivers/iio/adc/Kconfig | 10 ++
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/rzt2h_adc.c | 306 ++++++++++++++++++++++++++++++++++++
> > 4 files changed, 318 insertions(+)
> > create mode 100644 drivers/iio/adc/rzt2h_adc.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index eed08d25cb7a..220d17039084 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -21837,6 +21837,7 @@ L: linux-iio@...r.kernel.org
> > L: linux-renesas-soc@...r.kernel.org
> > S: Supported
> > F: Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-adc.yaml
> > +F: drivers/iio/adc/rzt2h_adc.c
> >
> > RENESAS RTCA-3 RTC DRIVER
> > M: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 58a14e6833f6..cab5eeba48fe 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -1403,6 +1403,16 @@ config RZG2L_ADC
> > To compile this driver as a module, choose M here: the
> > module will be called rzg2l_adc.
> >
> > +config RZT2H_ADC
> > + tristate "Renesas RZ/T2H / RZ/N2H ADC driver"
> > + select IIO_ADC_HELPER
> > + help
> > + Say yes here to build support for the ADC found in Renesas
> > + RZ/T2H / RZ/N2H SoCs.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called rzt2h_adc.
> > +
> > config SC27XX_ADC
> > tristate "Spreadtrum SC27xx series PMICs ADC"
> > depends on MFD_SC27XX_PMIC || COMPILE_TEST
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index d008f78dc010..ed647a734c51 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -123,6 +123,7 @@ obj-$(CONFIG_ROHM_BD79112) += rohm-bd79112.o
> > obj-$(CONFIG_ROHM_BD79124) += rohm-bd79124.o
> > obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> > obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o
> > +obj-$(CONFIG_RZT2H_ADC) += rzt2h_adc.o
> > obj-$(CONFIG_SC27XX_ADC) += sc27xx_adc.o
> > obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o
> > obj-$(CONFIG_SOPHGO_CV1800B_ADC) += sophgo-cv1800b-adc.o
> > diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> > new file mode 100644
> > index 000000000000..6a49788a5c67
> > --- /dev/null
> > +++ b/drivers/iio/adc/rzt2h_adc.c
> > @@ -0,0 +1,306 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/cleanup.h>
> > +#include <linux/completion.h>
> > +#include <linux/delay.h>
> > +#include <linux/iio/adc-helpers.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/property.h>
> > +
>
> ...
>
> >
> > +
> > +static int rzt2h_adc_pm_runtime_resume(struct device *dev)
> > +{
> > + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > + struct rzt2h_adc *adc = iio_priv(indio_dev);
>
> Not seeing the point of the pointer arithmetic. You can pass your device pointer
> (adc) directly in platform_set_drvdata()
>
Thanks Nuno, I'll do that. I also have another change to make to the driver so
I will have to send a new version and you'll have to give your Reviewed-by
again.
Here's the change I'm planning to make, maybe I could keep the Reviewed-by
if you agree.
Without this change, pm_runtime_resume_and_get() is inside the mutex,
while pm_runtime_put_autosuspend() is outside of it. This is mostly for
symmetry, although it's not excluded for some subtle bugs to be able to
occur without it.
diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
index 708029dc8949..79053bbc71c9 100644
--- a/drivers/iio/adc/rzt2h_adc.c
+++ b/drivers/iio/adc/rzt2h_adc.c
@@ -81,9 +81,9 @@ static int rzt2h_adc_read_single(struct rzt2h_adc *adc, unsigned int ch, int *va
ret = pm_runtime_resume_and_get(adc->dev);
if (ret)
return ret;
- guard(mutex)(&adc->lock);
+ mutex_lock(&adc->lock);
reinit_completion(&adc->completion);
/* Enable a single channel */
@@ -106,8 +106,10 @@ static int rzt2h_adc_read_single(struct rzt2h_adc *adc, unsigned int ch, int *va
disable:
rzt2h_adc_start_stop(adc, false, 0);
+ mutex_unlock(&adc->lock);
+
pm_runtime_put_autosuspend(adc->dev);
return ret;
}
> - Nuno Sá
________________________________
Renesas Electronics Europe GmbH
Registered Office: Arcadiastrasse 10
DE-40472 Duesseldorf
Commercial Registry: Duesseldorf, HRB 3708
Managing Director: Carsten Jauch
VAT-No.: DE 14978647
Tax-ID-No: 105/5839/1793
Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.
Powered by blists - more mailing lists