[PATCH 1/2] spi: sunxi: drop max_hz handling

[Andre Przywara]

On Fri, 12 Jul 2024 19:14:56 +0200
Michael Walle <mwalle at kernel.org> wrote:

Hi Michael,

> The driver is trying to read the "spi-max-frequency" property of the
> *controller* driver node. There is no such property. The
> "spi-max-frequency" property belongs to the SPI devices on the bus.

Ah, indeed, good catch! Many thanks for sending this!
 
> Right now, the driver will always fall back to the default value of 1MHz
> and thus flash reads are very slow with just about 215kb/s.

That's even slower, right? I guess around 125 KB/s?
 
> In fact, the SPI uclass will already take care of everything and we just
> have to clamp the frequency to the values the driver/hardware supports.
> Thus, drop the whole max_hz handling.

Looks good to me, I verified this by timing the read, this patch indeed 
significantly increases the performance. Also changing the limit in the
DT gets reflected in the driver and in the read speed. Also verified
that the values read from the SPI flash are the same in all cases.

> Signed-off-by: Michael Walle <mwalle at kernel.org>

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

I will make this part of the first 2024.10 PR.

Cheers,
Andre


> ---
>  drivers/spi/spi-sunxi.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
> index 13725ee7a2d..bfb402902b8 100644
> --- a/drivers/spi/spi-sunxi.c
> +++ b/drivers/spi/spi-sunxi.c
> @@ -135,7 +135,6 @@ struct sun4i_spi_variant {
>  struct sun4i_spi_plat {
>  	struct sun4i_spi_variant *variant;
>  	u32 base;
> -	u32 max_hz;
>  };
>  
>  struct sun4i_spi_priv {
> @@ -237,6 +236,13 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
>  	unsigned int div;
>  	u32 reg;
>  
> +	/*
> +	 * The uclass should take care that this won't happen. But anyway,
> +	 * avoid a div-by-zero exception.
> +	 */
> +	if (!priv->freq)
> +		return;
> +
>  	/*
>  	 * Setup clock divider.
>  	 *
> @@ -401,11 +407,10 @@ static int sun4i_spi_xfer(struct udevice *dev, unsigned int bitlen,
>  
>  static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>  {
> -	struct sun4i_spi_plat *plat = dev_get_plat(dev);
>  	struct sun4i_spi_priv *priv = dev_get_priv(dev);
>  
> -	if (speed > plat->max_hz)
> -		speed = plat->max_hz;
> +	if (speed > SUN4I_SPI_MAX_RATE)
> +		speed = SUN4I_SPI_MAX_RATE;
>  
>  	if (speed < SUN4I_SPI_MIN_RATE)
>  		speed = SUN4I_SPI_MIN_RATE;
> @@ -458,7 +463,6 @@ static int sun4i_spi_probe(struct udevice *bus)
>  
>  	priv->variant = plat->variant;
>  	priv->base = plat->base;
> -	priv->freq = plat->max_hz;
>  
>  	return 0;
>  }
> @@ -466,16 +470,9 @@ static int sun4i_spi_probe(struct udevice *bus)
>  static int sun4i_spi_of_to_plat(struct udevice *bus)
>  {
>  	struct sun4i_spi_plat *plat = dev_get_plat(bus);
> -	int node = dev_of_offset(bus);
>  
>  	plat->base = dev_read_addr(bus);
>  	plat->variant = (struct sun4i_spi_variant *)dev_get_driver_data(bus);
> -	plat->max_hz = fdtdec_get_int(gd->fdt_blob, node,
> -				      "spi-max-frequency",
> -				      SUN4I_SPI_DEFAULT_RATE);
> -
> -	if (plat->max_hz > SUN4I_SPI_MAX_RATE)
> -		plat->max_hz = SUN4I_SPI_MAX_RATE;
>  
>  	return 0;
>  }