Re: [PATCH v2 2/2] spi: sunxi: fix clock divider calculation for max frequency setting

[Andre Przywara <andre.przywara@arm.com>]

On Thu, 18 Jul 2024 22:42:53 +0200
Michael Walle <mwalle@kernel.org> wrote:

> If the maximum frequency is requested, we still fall into the CDR2
> handling. But there the minimal divider is 2. For the sun6i and sun8i we
> can do better with the CDR1 setting where the minimal divider is 1:
>   SPI_CLK = MOD_CLK / 2 ^ cdr with cdr = 0
> 
> Thus, handle the div = 1 case specially.
> 
> While at it, correct the comment above the calculation.
> 
> Signed-off-by: Michael Walle <mwalle@kernel.org>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Applied to sunxi/master.

Cheers,
Andre

> ---
>  drivers/spi/spi-sunxi.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
> index f110a8b7658..81f1298adea 100644
> --- a/drivers/spi/spi-sunxi.c
> +++ b/drivers/spi/spi-sunxi.c
> @@ -249,6 +249,8 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
>  	 * We have two choices there. Either we can use the clock
>  	 * divide rate 1, which is calculated thanks to this formula:
>  	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
> +	 * Or for sun6i/sun8i variants:
> +	 * SPI_CLK = MOD_CLK / (2 ^ cdr)
>  	 * Or we can use CDR2, which is calculated with the formula:
>  	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
>  	 * Whether we use the former or the latter is set through the
> @@ -256,13 +258,16 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
>  	 *
>  	 * First try CDR2, and if we can't reach the expected
>  	 * frequency, fall back to CDR1.
> +	 * There is one exception if the requested clock is the input
> +	 * clock. In that case we always use CDR1 because we'll get a
> +	 * 1:1 ration for sun6i/sun8i variants.
>  	 */
>  
>  	div = DIV_ROUND_UP(SUNXI_INPUT_CLOCK, priv->freq);
>  	div_cdr2 = DIV_ROUND_UP(div, 2);
>  	reg = readl(SPI_REG(priv, SPI_CCR));
>  
> -	if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> +	if (div != 1 && (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1))) {
>  		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
>  		reg |= SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
>  	} else {