linux-kernel - Re: [PATCH v5 2/2] iio: adc: Add the NXP SAR ADC support for the s32g2/3 platforms

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: <aPP0uVZu1T7tTQGo@ashevche-desk.local>
Date: Sat, 18 Oct 2025 23:12:41 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: jic23@...nel.org, dlechner@...libre.com, nuno.sa@...log.com,
	andy@...nel.org, robh@...nel.org, conor+dt@...nel.org,
	krzk+dt@...nel.org, linux-iio@...r.kernel.org, s32@....com,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	chester62515@...il.com, mbrugger@...e.com,
	ghennadi.procopciuc@....nxp.com
Subject: Re: [PATCH v5 2/2] iio: adc: Add the NXP SAR ADC support for the
 s32g2/3 platforms

On Fri, Oct 17, 2025 at 06:42:38PM +0200, Daniel Lezcano wrote:
> From: Stefan-Gabriel Mirea <stefan-gabriel.mirea@....com>
> 
> The NXP S32G2 and S32G3 platforms integrate a successive approximation
> register (SAR) ADC. Two instances are available, each providing 8
> multiplexed input channels with 12-bit resolution. The conversion rate
> is up to 1 Msps depending on the configuration and sampling window.
> 
> The SAR ADC supports raw, buffer, and trigger modes. It can operate
> in both single-shot and continuous conversion modes, with optional
> hardware triggering through the cross-trigger unit (CTU) or external
> events. An internal prescaler allows adjusting the sampling clock,
> while per-channel programmable sampling times provide fine-grained
> trade-offs between accuracy and latency. Automatic calibration is
> performed at probe time to minimize offset and gain errors.
> 
> The driver is derived from the BSP implementation and has been partly
> rewritten to comply with upstream requirements. For this reason, all
> contributors are listed as co-developers, while the author refers to
> the initial BSP driver file creator.
> 
> All modes have been validated on the S32G274-RDB2 platform using an
> externally generated square wave captured by the ADC. Tests covered
> buffered streaming via IIO, trigger synchronization, and accuracy
> verification against a precision laboratory signal source.

...

> +#include <linux/circ_buf.h>

Why not kfifo?

...

> +#define NXP_SAR_ADC_IIO_BUFF_SZ		(NXP_SAR_ADC_NR_CHANNELS + (sizeof(u64) / sizeof(u16)))

Hmm... Don't we have some macros so we can avoid this kind of hard coding?

...

> +	ndelay(div64_u64(NSEC_PER_SEC, clk_get_rate(info->clk)) * 80U);

Do you need those 'U':s? clk_get_rate() already returns unsigned value of the
same or higher rank than int. No?

...

> +static int nxp_sar_adc_start_conversion(struct nxp_sar_adc *info, bool raw)
> +{
> +	u32 mcr;
> +
> +	mcr = readl(NXP_SAR_ADC_MCR(info->regs));
> +
> +	FIELD_MODIFY(NXP_SAR_ADC_MCR_NSTART, &mcr, 0x1);
> +	FIELD_MODIFY(NXP_SAR_ADC_MCR_MODE, &mcr, !raw);

!raw, which is boolean, as a parameter to FIELD_MODIFY() seems a bit odd to me,
perhaps simple

	raw ? 0 : 1

would work better?

(Note, optimizer of the complier will avoid any branching)

> +	writel(mcr, NXP_SAR_ADC_MCR(info->regs));
> +
> +	return 0;
> +}

...

> +	dma_samples = (u32 *)dma_buf->buf;

Is it aligned properly for this type of casting?

...

> +	dmaengine_tx_status(info->dma_chan, info->cookie, &state);

No return value check?

...

> +static const struct nxp_sar_adc_data s32g2_sar_adc_data = {
> +	.vref_mV = 1800,
> +	.model = "s32g2-sar-adc"

Keep a trailing comma as here it's not a termination member.

> +};

-- 
With Best Regards,
Andy Shevchenko