linux-kernel - Re: [PATCH] iio: adc: ad7124: add debugfs to disable single cycle mode

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] [day] [month] [year] [list]

Message-ID: <d20225be2339e7b08f65edf18f9fa71ae5840f0c.camel@gmail.com>
Date: Thu, 18 Sep 2025 11:34:48 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Michael Hennerich
	 <Michael.Hennerich@...log.com>, Jonathan Cameron <jic23@...nel.org>, Nuno
 Sá
	 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: adc: ad7124: add debugfs to disable single cycle
 mode

On Wed, 2025-09-17 at 18:03 -0500, David Lechner wrote:
> Add a boolean debugfs attribute to allow disabling the SINGLE_CYCLE
> bit in the FILTER registers.
> 
> This causes data to be read on every conversion instead of doing the
> usual 3 or 4 conversions per sample (depending on the filter). This is
> only needed for very specific use cases, such as validating the
> performance of the ADC. So we just expose this feature through debugfs
> for the rare cases where it is needed by people who really know what
> they are doing.
> 
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
> In a recent discussion with an engineer who has used these chips a lot,
> we confirmed that we made the right choice in [1] about always enabling
> the SINGLE_CYCLE bit. That is what is needed in normal operation and is
> the expected behavior.
> 
> But there are some occasions where we might want to turn it off for
> hardware debugging (e.g. to peer into what the filter on the ADC is
> doing). Hence, this patch to add a debugfs entry to allow it.
> 
> FYI, there will be some trivial fuzz between this patch and the "iio:
> adc: ad7124: change setup reg allocation strategy" patch, but I expect
> changes to be requested on that one, so will likely work itself out
> by the time it actually gets picked up.
> 
> [1]:
> https://lore.kernel.org/linux-iio/20250910-iio-adc-ad7124-fix-samp-freq-for-multi-channel-v4-1-8ca624c6114c@baylibre.com/
> ---

Reviewed-by: Nuno Sá <nuno.sa@...log.com>

>  drivers/iio/adc/ad7124.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index
> 910b40393f77de84afc77d406c17c6e5051a02cd..ad6edbc792db35a644df37fce62bf95b7988
> 1455 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -10,6 +10,7 @@
>  #include <linux/cleanup.h>
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
> +#include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> @@ -223,6 +224,7 @@ struct ad7124_state {
>  	 */
>  	unsigned int gain_default;
>  	DECLARE_KFIFO(live_cfgs_fifo, struct ad7124_channel_config *,
> AD7124_MAX_CONFIGS);
> +	bool enable_single_cycle;
>  };
>  
>  static const struct ad7124_chip_info ad7124_4_chip_info = {
> @@ -560,13 +562,15 @@ static int ad7124_write_config(struct ad7124_state *st,
> struct ad7124_channel_co
>  	 * sampling frequency even when only one channel is enabled in a
>  	 * buffered read. If it was not set, the N in
> ad7124_set_channel_odr()
>  	 * would be 1 and we would get a faster sampling frequency than what
> -	 * was requested.
> +	 * was requested. It may only be disabled through debugfs for testing
> +	 * purposes.
>  	 */
>  	return ad_sd_write_reg(&st->sd, AD7124_FILTER(cfg->cfg_slot), 3,
>  			       FIELD_PREP(AD7124_FILTER_FILTER, filter) |
>  			       FIELD_PREP(AD7124_FILTER_REJ60, rej60) |
>  			       FIELD_PREP(AD7124_FILTER_POST_FILTER, post) |
> -			       AD7124_FILTER_SINGLE_CYCLE |
> +			       FIELD_PREP(AD7124_FILTER_SINGLE_CYCLE,
> +					  st->enable_single_cycle) |
>  			       FIELD_PREP(AD7124_FILTER_FS, cfg-
> >odr_sel_bits));
>  }
>  
> @@ -1609,6 +1613,18 @@ static void ad7124_reg_disable(void *r)
>  	regulator_disable(r);
>  }
>  
> +static void ad7124_debugfs_init(struct iio_dev *indio_dev)
> +{
> +	struct dentry *dentry = iio_get_debugfs_dentry(indio_dev);
> +	struct ad7124_state *st = iio_priv(indio_dev);
> +
> +	if (!IS_ENABLED(CONFIG_DEBUG_FS))
> +		return;
> +
> +	debugfs_create_bool("enable_single_cycle", 0644, dentry,
> +			    &st->enable_single_cycle);
> +}
> +
>  static int ad7124_probe(struct spi_device *spi)
>  {
>  	const struct ad7124_chip_info *info;
> @@ -1629,6 +1645,9 @@ static int ad7124_probe(struct spi_device *spi)
>  
>  	st->chip_info = info;
>  
> +	/* Only disabled for debug/testing purposes. */
> +	st->enable_single_cycle = true;
> +
>  	indio_dev->name = st->chip_info->name;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->info = &ad7124_info;
> @@ -1686,6 +1705,8 @@ static int ad7124_probe(struct spi_device *spi)
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "Failed to register iio
> device\n");
>  
> +	ad7124_debugfs_init(indio_dev);
> +
>  	return 0;
>  }
>  
> 
> ---
> base-commit: 561285d048053fec8a3d6d1e3ddc60df11c393a0
> change-id: 20250917-iio-adc-ad7124-add-debugfs-to-disable-single_cycle-
> c378ef330b89
> 
> Best regards,