Re: [PATCH v5 7/8] iio: imu: adis16607: Add external clock sync support

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 02 Nov 2026 11:25:43 +0200
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:

> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add support for an optional external clock (clocks property),
> selecting between SYNC_DIRECT (1 kHz to 8 kHz) and SYNC_SCALED (1 Hz
> to 400 Hz) depending on the supplied frequency, with the internal
> 8 kHz clock used as the default when no clock is provided.
> 
> The clock rate is requested as exclusive since the sample period and
> the sync scale are derived from it. In scaled mode the sync scale is
> chosen when the sampling frequency is set, so that the requested rate
> is matched as closely as possible while keeping the clock multiplied
> at or below the internal rate. Loss of sync or of the PLL lock is
> reported from the trigger handler, as the device keeps sampling
> without it.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
Hi,

Mostly a few stray changes that belong in earlier patches.

Make sure to check series for this stuff. It is easy to clean up
and patch tidiness issues like this can just slow down getting
your code merged.

Jonathan

>  
>  struct adis16607 {
> @@ -115,6 +138,9 @@ struct adis16607 {
>  	const struct adis16607_chip_info *info;
>  	unsigned long clk_freq_mHz;
>  	u64 sample_period_ns;
> +	unsigned long ext_clk_Hz;
> +	u32 sync_mode;
> +	u16 sync_scale;
>  	u16 fifo_watermark;
>  	/* IIO buffer for reordered data - sized for max channels */
>  	IIO_DECLARE_BUFFER_WITH_TS(u8, data, ADIS16607_SCAN_DATA_SIZE);
> @@ -407,11 +433,18 @@ static void adis16607_fifo_drain(struct iio_dev *indio_dev, s64 irq_timestamp)
>  
>  	adis_dev_auto_lock(&st->adis);
>  
> -	/* Check if FIFO threshold is met */

Move the comment in the earlier patch, not here. It is perfect
valid in its new location back in that patch, so why add to
the code churn here?

>  	ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_DIAG_STAT, &diag_stat);
>  	if (ret)
>  		return;
>  
> +	/* The device keeps sampling without sync, so only warn about it */
> +	if (diag_stat & ADIS16607_DIAG_SYNC_LOST)
> +		dev_warn_ratelimited(&st->adis.spi->dev, "external sync lost\n");
> +
> +	if (diag_stat & ADIS16607_DIAG_DPLL_UNLOCKED)
> +		dev_warn_ratelimited(&st->adis.spi->dev, "sync PLL unlocked\n");
> +
> +	/* Check if FIFO threshold is met */
>  	if (!(diag_stat & ADIS16607_FIFO_THR_MET_MASK))
>  		return;

>  static int adis16607_config_clk(struct adis16607 *st)
>  {

> +	ret = adis_read_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, &dec);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Sample period is (dec + 1) / clk_freq, with clk_freq_mHz in
> +	 * millihertz. Same u64-first scheme as adis16607_set_freq().
> +	 */
> +	period_ns = NSEC_PER_SEC;

Move this in earlier patch to avoid churn here.

>  	period_ns *= MILLIHZ_PER_HZ;
> +	period_ns *= dec + 1;
>  	st->sample_period_ns = DIV_ROUND_CLOSEST_ULL(period_ns,
>  						     st->clk_freq_mHz);
> +

Unrelated change. Check for these and move to the correct patch.

>  	return 0;
>  }
>  
>