Re: [PATCH v2 02/14] iio: adc: stm32-adc: core: manage clock prescaler diversity

[Jonathan Cameron]

On Wed, 23 Sep 2026 17:39:05 +0200
Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx> wrote:

> Make the clock prescaler selection configurable, to prepare stm32mp2
> support.
> Reuse existing stm32f4_adc_clk_sel(). Rename it so it's no longer
> specific to stm32f4. Add configuration data to handle upcoming
> differences on:
> - registers and bits
> - division ratio
> No functional change intended.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx>
Hi Fabrice,

One small suggestion to use field_prep() rather than having
to carry the shift around.
>  
> -	priv->common.rate = rate / stm32f4_pclk_div[i];
> -	val = readl_relaxed(priv->common.base + STM32F4_ADC_CCR);
> -	val &= ~STM32F4_ADC_ADCPRE_MASK;
> -	val |= i << STM32F4_ADC_ADCPRE_SHIFT;
> -	writel_relaxed(val, priv->common.base + STM32F4_ADC_CCR);
> +	priv->common.rate = rate / priv->cfg->presc[i];
> +	val = readl_relaxed(priv->common.base + priv->cfg->regs->ccr);
> +	val &= ~priv->cfg->regs->presc_msk;
> +	val |= i << priv->cfg->regs->presc_shift;

I think this isn't a particularly hot path, so we can use
field_prep()  (the lower case one doesn't mind non const masks)

The we don't need to store the shift as well as the mask.

Maybe it is worth thinking about adding a field_modify() similar
to FIELD_MODIFY(). Perhaps that's not a thing to do in this patch set
though.

> +	writel_relaxed(val, priv->common.base + priv->cfg->regs->ccr);
>  
>  	dev_dbg(&pdev->dev, "Using analog clock source at %ld kHz\n",
>  		priv->common.rate / 1000);

>