| 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: <d53b22d1-35d6-4fb8-ae56-3ba4953b64af@baylibre.com>
Date: Thu, 11 Sep 2025 15:10:48 -0500
From: David Lechner <dlechner@...libre.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>, jic23@...nel.org,
nuno.sa@...log.com, andy@...nel.org, robh@...nel.org, conor+dt@...nel.org,
krzk+dt@...nel.org
Cc: 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 v2 2/2] iio: adc: Add the NXP SAR ADC support for the
s32g2/3 platforms
On 9/10/25 10:57 AM, 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.
>
> Co-developed-by: Alexandru-Catalin Ionita <alexandru-catalin.ionita@....com>
> Signed-off-by: Alexandru-Catalin Ionita <alexandru-catalin.ionita@....com>
> Co-developed-by: Ciprian Costea <ciprianmarian.costea@....com>
> Signed-off-by: Ciprian Costea <ciprianmarian.costea@....com>
> Co-developed-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@....nxp.com>
> Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@....nxp.com>
> Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@....com>
> Co-developed-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
> drivers/iio/adc/Kconfig | 13 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/nxp-sar-adc.c | 1026 +++++++++++++++++++++++++++++++++
> 3 files changed, 1040 insertions(+)
> create mode 100644 drivers/iio/adc/nxp-sar-adc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 6de2abad0197..4c2473a1fa20 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1168,6 +1168,19 @@ config NPCM_ADC
> This driver can also be built as a module. If so, the module
> will be called npcm_adc.
>
> +config NXP_SAR_ADC
> + tristate "NXP S32G SAR-ADC driver"
> + depends on ARCH_S32 || COMPILE_TEST
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + select IIO_SYSFS_TRIGGER
> + help
> + Say yes here to build support for S32G platforms
> + analog-to-digital converter.
> +
> + This driver can also be built as a module. If so, the module will be
> + called nxp_sar_adc.
> +
> config PAC1921
> tristate "Microchip Technology PAC1921 driver"
> depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 1c6ca5fd4b6d..d1d939753bcc 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -119,6 +119,7 @@ obj-$(CONFIG_RN5T618_ADC) += rn5t618-adc.o
> obj-$(CONFIG_ROHM_BD79124) += rohm-bd79124.o
> obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o
> +obj-$(CONFIG_NXP_SAR_ADC) += nxp-sar-adc.o
We try to keep these in alphabetical order.
> 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/nxp-sar-adc.c b/drivers/iio/adc/nxp-sar-adc.c
> new file mode 100644
> index 000000000000..421ce42ad9ec
> --- /dev/null
> +++ b/drivers/iio/adc/nxp-sar-adc.c
> @@ -0,0 +1,1026 @@
> +// SPDX-License-Identifier: GPL-2.0
Prefer explicit GPL-2.0-only or GPL-2.0-or-later
...
> +static void nxp_sar_adc_dma_cb(void *data)
> +{
> + struct nxp_sar_adc *info = iio_priv((struct iio_dev *)data);
> + struct iio_dev *indio_dev = data;
> + struct dma_tx_state state;
> + struct circ_buf *dma_buf;
> + struct device *dev_dma;
> + u32 *dma_samples;
> + s64 timestamp;
> + int idx, ret;
> +
> + guard(spinlock_irqsave)(&info->lock);
> +
> + dma_buf = &info->dma_buf;
> + dma_samples = (u32 *)dma_buf->buf;
> + dev_dma = info->dma_chan->device->dev;
> +
> + dmaengine_tx_status(info->dma_chan, info->cookie, &state);
> +
> + dma_sync_single_for_cpu(dev_dma, info->rx_dma_buf,
> + NXP_SAR_ADC_DMA_BUFF_SZ, DMA_FROM_DEVICE);
> + /* Current head position */
> + dma_buf->head = (NXP_SAR_ADC_DMA_BUFF_SZ - state.residue) /
> + NXP_SAR_ADC_DMA_SAMPLE_SZ;
> +
> + /* If everything transferred, avoid an off by one error. */
> + if (!state.residue)
> + dma_buf->head--;
> +
> + /* Something went wrong and nothing transferred. */
> + if (state.residue == NXP_SAR_ADC_DMA_BUFF_SZ)
> + goto out;
> +
> + /* Make sure that head is multiple of info->channels_used */
> + dma_buf->head -= dma_buf->head % info->channels_used;
> +
> + /* dma_buf->tail != dma_buf->head condition will become false
> + * because dma_buf->tail will be incremented with 1.
> + */
> + while (dma_buf->tail != dma_buf->head) {
> + idx = dma_buf->tail % info->channels_used;
> + info->buffer[idx] = dma_samples[dma_buf->tail];
> + dma_buf->tail = (dma_buf->tail + 1) % NXP_SAR_ADC_DMA_SAMPLE_CNT;
> + if (idx != info->channels_used - 1)
> + continue;
> +
> + /* iio_push_to_buffers_with_timestamp should not be called
> + * with dma_samples as parameter. The samples will be smashed
> + * if timestamp is enabled.
> + */
> + timestamp = iio_get_time_ns(indio_dev);
> + ret = iio_push_to_buffers_with_timestamp(indio_dev,
> + info->buffer,
> + timestamp);
Is it OK to call this with spinlock held? It looks like it can call
devm_krealloc() which may sleep.
> + if (ret < 0 && ret != -EBUSY)
> + dev_err_ratelimited(&indio_dev->dev,
> + "failed to push iio buffer: %d",
> + ret);
> + }
> +
> + dma_buf->tail = dma_buf->head;
> +out:
> + dma_sync_single_for_device(dev_dma, info->rx_dma_buf,
> + NXP_SAR_ADC_DMA_BUFF_SZ, DMA_FROM_DEVICE);
> +}
> +
...
> +static int nxp_sar_adc_probe(struct platform_device *pdev)
> +{
> + const struct nxp_sar_adc_data *data;
> + struct nxp_sar_adc *info;
> + struct iio_dev *indio_dev;
> + struct resource *mem;
> + struct device *dev = &pdev->dev;
> + int irq;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(struct nxp_sar_adc));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + info = iio_priv(indio_dev);
> +
> + data = device_get_match_data(dev);
> +
> + info->vref_mV = data->vref_mV;
> +
> + info->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem);
> + if (IS_ERR(info->regs))
> + return dev_err_probe(dev, PTR_ERR(info->regs),
> + "failed to get and remap resource");
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + ret = devm_request_irq(dev, irq, nxp_sar_adc_isr, 0,
> + dev_name(dev), indio_dev);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed requesting irq, irq = %d\n", irq);
> +
> + info->regs_phys = mem->start;
> + spin_lock_init(&info->lock);
> +
> + info->clk = devm_clk_get_enabled(dev, "adc");
clock-names was dropped from bindings, so name should be NULL.
> + if (IS_ERR(info->clk))
> + return dev_err_probe(dev, PTR_ERR(info->clk),
> + "failed to get the clock\n");
> +
> + platform_set_drvdata(pdev, indio_dev);
> +
> + init_completion(&info->completion);
> +
> + indio_dev->name = dev_name(dev);
> + indio_dev->info = &nxp_sar_adc_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> + indio_dev->channels = nxp_sar_adc_iio_channels;
> + indio_dev->num_channels = ARRAY_SIZE(nxp_sar_adc_iio_channels);
> +
> + nxp_sar_adc_set_default_values(info);
> +
> + ret = nxp_sar_adc_calibration(info);
> + if (ret) {
> + dev_err(dev, "Calibration failed: %d\n", ret);
> + return ret;
> + }
> +
> + ret = nxp_sar_adc_dma_probe(dev, info);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to initialize the dma\n");
> +
> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> + &iio_pollfunc_store_time,
> + &nxp_sar_adc_trigger_handler,
> + &iio_triggered_buffer_setup_ops);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Couldn't initialise the buffer\n");
> +
> + ret = devm_iio_device_register(dev, indio_dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Couldn't register the device\n");
> +
> + return 0;
> +}
> +
...
> +static const struct nxp_sar_adc_data s32g2_sar_adc_data = { .vref_mV = 1800 };
Why have this if there is only one option?
> +
> +static const struct of_device_id nxp_sar_adc_match[] = {
> + { .compatible = "nxp,s32g2-sar-adc", .data = &s32g2_sar_adc_data },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, nxp_sar_adc_match);
> +
Powered by blists - more mailing lists