Re: [PATCH v2 2/6] iio: adc: ti-ads1298: fix power-up and clock handling

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sat, Oct 24, 2026 at 08:13:31PM -0300, Stephan Costa Barros wrote:
> Fix three issues with the power-up sequence and the device clock:
> 
> - The datasheet powers up the supplies first, then the clock, then
>   waits t_POR = 2^18 clock cycles (128 ms at 2.048 MHz) before the
>   reset. The driver enabled an external clock and the vref supply
>   before AVDD, reset right away, and did not wait for the 150 ms
>   start-up of the internal reference. Follow the datasheet. Probe takes
>   about 280 ms longer.
> 
> - The binding describes one optional clock and no clock-names, but the
>   driver asked for the clock named "clk". With a device tree that
>   follows the binding, the lookup fails, the optional getter returns
>   NULL, and a clock given in the device tree is never enabled, while the
>   sampling frequency is computed from the nominal 2.048 MHz. Ask for the
>   first clock of the node; a device tree with clock-names = "clk" still
>   gets the same clock.
> 
> - A command takes 4 cycles of the device clock to decode, the minimum
>   reset pulse is 2 cycles, and the reset takes 18 more. These were
>   converted with the nominal 2.048 MHz, so a command ends after a fixed
>   2 us, short of 4 cycles with an external clock below 2 MHz, which the
>   ADS129x allows down to 1.94 MHz. Convert them with the rate of the
>   clock in use. At 2.048 MHz the delays do not change.
> 
> The clock lookup was seen to fail on a Raspberry Pi 4, with a
> fixed-clock in the device tree. The power-up order was found by review
> and reproduced on an emulated device only. The decode time was found
> with an emulated device; it is not seen on hardware running at
> 2.048 MHz.

...

> +	clk_rate = priv->clk ? clk_get_rate(priv->clk) : 0;

Why this conditional?

> +	if (!clk_rate)
> +		clk_rate = ADS1298_CLK_RATE_HZ;
> +	priv->clk_rate = clk_rate;

...

> +	/* Power-on reset time, counted from when the clock is valid */
> +	msleep(DIV_ROUND_UP(ADS1298_TPOR_CLOCKS * MILLI, clk_rate));

Do you mean MSEC_PER_SEC?

...

> +	priv->rdata_xfer.delay.value = ads1298_clocks_to_usecs(priv,
> +							       ADS1298_DECODE_CLOCKS);

Indent it as

	priv->rdata_xfer.delay.value =
		ads1298_clocks_to_usecs(priv, ADS1298_DECODE_CLOCKS);

-- 
With Best Regards,
Andy Shevchenko