[1/3] spi: rockchip_sfc: Impletment set_speed logic

Set clock related processing into set_speed logic. And Optimize
printing format.

Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

 drivers/spi/rockchip_sfc.c | 83 ++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 43 deletions(-)

On Tue, Aug 17, 2021 at 09:40:59AM +0800, Jon Lin wrote:
> Set clock related processing into set_speed logic. And Optimize
> printing format.
> 
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> ---

I tested this one and it tests out okay. I figured out the issue that
was causing the performance regression. For the value of
CONFIG_SF_DEFAULT_SPEED the help text says "Not used for boot with
device tree", however I can confirm that after doing an "sf probe"
the speed would reset to whatever value I had set in this parameter
(which at the time was the default of 1000000). When I changed this
parameter to 108000000 the speed regression was fixed. Either the
help text needs to be updated or there is a bug where it's using
this value instead of the devicetree derived value.

> 
>  drivers/spi/rockchip_sfc.c | 83 ++++++++++++++++++--------------------
>  1 file changed, 40 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/spi/rockchip_sfc.c b/drivers/spi/rockchip_sfc.c
> index 4e2b861f22..2028b28e26 100644
> --- a/drivers/spi/rockchip_sfc.c
> +++ b/drivers/spi/rockchip_sfc.c
> @@ -12,6 +12,7 @@
>  #include <bouncebuf.h>
>  #include <clk.h>
>  #include <dm.h>
> +#include <dm/device_compat.h>
>  #include <linux/bitops.h>
>  #include <linux/delay.h>
>  #include <linux/iopoll.h>
> @@ -116,6 +117,7 @@
>  
>  /* Master trigger */
>  #define SFC_DMA_TRIGGER			0x80
> +#define SFC_DMA_TRIGGER_START		1
>  
>  /* Src or Dst addr for master */
>  #define SFC_DMA_ADDR			0x84
> @@ -163,14 +165,12 @@
>  #define SFC_DMA_TRANS_THRETHOLD		(0x40)
>  
>  /* Maximum clock values from datasheet suggest keeping clock value under
> - * 150MHz. No minimum or average value is suggested, but the U-boot BSP driver
> - * has a minimum of 10MHz and a default of 80MHz which seems reasonable.
> + * 150MHz. No minimum or average value is suggested.
>   */
> -#define SFC_MIN_SPEED_HZ		(10 * 1000 * 1000)
> -#define SFC_DEFAULT_SPEED_HZ		(80 * 1000 * 1000)
> -#define SFC_MAX_SPEED_HZ		(150 * 1000 * 1000)
> +#define SFC_MAX_SPEED		(150 * 1000 * 1000)
>  
>  struct rockchip_sfc {
> +	struct udevice *dev;
>  	void __iomem *regbase;
>  	struct clk hclk;
>  	struct clk clk;
> @@ -197,8 +197,6 @@ static int rockchip_sfc_reset(struct rockchip_sfc *sfc)
>  	/* Still need to clear the masked interrupt from RISR */
>  	writel(0xFFFFFFFF, sfc->regbase + SFC_ICLR);
>  
> -	debug("reset\n");
> -
>  	return err;
>  }
>  
> @@ -261,15 +259,11 @@ static int rockchip_sfc_probe(struct udevice *bus)
>  #if CONFIG_IS_ENABLED(CLK)
>  	ret = clk_enable(&sfc->hclk);
>  	if (ret)
> -		debug("Enable ahb clock fail %s: %d\n", bus->name, ret);
> +		dev_dbg(sfc->dev, "sfc Enable ahb clock fail %s: %d\n", bus->name, ret);
>  
>  	ret = clk_enable(&sfc->clk);
>  	if (ret)
> -		debug("Enable clock fail for %s: %d\n", bus->name, ret);
> -
> -	ret = clk_set_rate(&sfc->clk, SFC_DEFAULT_SPEED_HZ);
> -	if (ret)
> -		debug("Could not set sfc clock for %s: %d\n", bus->name, ret);
> +		dev_dbg(sfc->dev, "sfc Enable clock fail for %s: %d\n", bus->name, ret);
>  #endif
>  
>  	ret = rockchip_sfc_init(sfc);
> @@ -278,7 +272,8 @@ static int rockchip_sfc_probe(struct udevice *bus)
>  
>  	sfc->max_iosize = rockchip_sfc_get_max_iosize(sfc);
>  	sfc->version = rockchip_sfc_get_version(sfc);
> -	sfc->speed = SFC_DEFAULT_SPEED_HZ;
> +	sfc->max_freq = SFC_MAX_SPEED;
> +	sfc->dev = bus;
>  
>  	return 0;
>  
> @@ -411,11 +406,11 @@ static int rockchip_sfc_xfer_setup(struct rockchip_sfc *sfc,
>  	ctrl |= SFC_CTRL_PHASE_SEL_NEGETIVE;
>  	cmd |= plat->cs << SFC_CMD_CS_SHIFT;
>  
> -	debug("addr.nbytes=%x(x%d) dummy.nbytes=%x(x%d)\n",
> -	      op->addr.nbytes, op->addr.buswidth,
> -	      op->dummy.nbytes, op->dummy.buswidth);
> -	debug("ctrl=%x cmd=%x addr=%llx len=%x\n",
> -	      ctrl, cmd, op->addr.val, len);
> +	dev_dbg(sfc->dev, "sfc addr.nbytes=%x(x%d) dummy.nbytes=%x(x%d)\n",
> +		op->addr.nbytes, op->addr.buswidth,
> +		op->dummy.nbytes, op->dummy.buswidth);
> +	dev_dbg(sfc->dev, "sfc ctrl=%x cmd=%x addr=%llx len=%x\n",
> +		ctrl, cmd, op->addr.val, len);
>  
>  	writel(ctrl, sfc->regbase + SFC_CTRL);
>  	writel(cmd, sfc->regbase + SFC_CMD);
> @@ -492,7 +487,7 @@ static int rockchip_sfc_fifo_transfer_dma(struct rockchip_sfc *sfc, dma_addr_t d
>  {
>  	writel(0xFFFFFFFF, sfc->regbase + SFC_ICLR);
>  	writel((u32)dma_buf, sfc->regbase + SFC_DMA_ADDR);
> -	writel(0x1, sfc->regbase + SFC_DMA_TRIGGER);
> +	writel(SFC_DMA_TRIGGER_START, sfc->regbase + SFC_DMA_TRIGGER);
>  
>  	return len;
>  }
> @@ -500,7 +495,7 @@ static int rockchip_sfc_fifo_transfer_dma(struct rockchip_sfc *sfc, dma_addr_t d
>  static int rockchip_sfc_xfer_data_poll(struct rockchip_sfc *sfc,
>  				       const struct spi_mem_op *op, u32 len)
>  {
> -	debug("xfer_poll len=%x\n", len);
> +	dev_dbg(sfc->dev, "sfc xfer_poll len=%x\n", len);
>  
>  	if (op->data.dir == SPI_MEM_DATA_OUT)
>  		return rockchip_sfc_write_fifo(sfc, op->data.buf.out, len);
> @@ -516,7 +511,7 @@ static int rockchip_sfc_xfer_data_dma(struct rockchip_sfc *sfc,
>  	void *dma_buf;
>  	int ret;
>  
> -	debug("xfer_dma len=%x\n", len);
> +	dev_dbg(sfc->dev, "sfc xfer_dma len=%x\n", len);
>  
>  	if (op->data.dir == SPI_MEM_DATA_OUT) {
>  		dma_buf = (void *)op->data.buf.out;
> @@ -564,33 +559,16 @@ static int rockchip_sfc_exec_op(struct spi_slave *mem,
>  	u32 len = min_t(u32, op->data.nbytes, sfc->max_iosize);
>  	int ret;
>  
> -#if CONFIG_IS_ENABLED(CLK)
> -	if (unlikely(mem->max_hz != sfc->speed)) {
> -		ret = clk_set_rate(&sfc->clk, clamp(mem->max_hz, (uint)SFC_MIN_SPEED_HZ,
> -						    (uint)SFC_MAX_SPEED_HZ));
> -		if (ret < 0) {
> -			printf("set_freq=%dHz fail, check if it's the cru support level\n",
> -			       mem->max_hz);
> -			return ret;
> -		}
> -
> -		sfc->max_freq = mem->max_hz;
> -		sfc->speed = mem->max_hz;
> -		debug("set_freq=%dHz real_freq=%dHz\n", sfc->max_freq, sfc->speed);
> -	}
> -#endif
> -
>  	rockchip_sfc_adjust_op_work((struct spi_mem_op *)op);
> -
>  	rockchip_sfc_xfer_setup(sfc, mem, op, len);
>  	if (len) {
> -		if (likely(sfc->use_dma) && !(len & 0x3) && len >= SFC_DMA_TRANS_THRETHOLD)
> +		if (likely(sfc->use_dma) && len >= SFC_DMA_TRANS_THRETHOLD)
>  			ret = rockchip_sfc_xfer_data_dma(sfc, op, len);
>  		else
>  			ret = rockchip_sfc_xfer_data_poll(sfc, op, len);
>  
>  		if (ret != len) {
> -			printf("xfer data failed ret %d dir %d\n", ret, op->data.dir);
> +			dev_err(sfc->dev, "xfer data failed ret %d dir %d\n", ret, op->data.dir);
>  
>  			return -EIO;
>  		}
> @@ -604,13 +582,32 @@ static int rockchip_sfc_adjust_op_size(struct spi_slave *mem, struct spi_mem_op
>  	struct rockchip_sfc *sfc = dev_get_plat(mem->dev->parent);
>  
>  	op->data.nbytes = min(op->data.nbytes, sfc->max_iosize);
> +
>  	return 0;
>  }
>  
>  static int rockchip_sfc_set_speed(struct udevice *bus, uint speed)
>  {
> -	/* We set up speed later for each transmission.
> -	 */
> +	struct rockchip_sfc *sfc = dev_get_plat(bus);
> +	int ret;
> +
> +	if (speed > sfc->max_freq)
> +		speed = sfc->max_freq;
> +
> +	if (speed == sfc->speed)
> +		return 0;
> +
> +#if CONFIG_IS_ENABLED(CLK)
> +	ret = clk_set_rate(&sfc->clk, speed);
> +	if (ret < 0) {
> +		dev_err(sfc->dev, "set_freq=%dHz fail, check if it's the cru support level\n",
> +			speed);
> +		return ret;
> +	}
> +	sfc->speed = speed;
> +#else
> +	dev_dbg(sfc->dev, "sfc failed, CLK not support\n");
> +#endif
>  	return 0;
>  }
>  
> -- 
> 2.17.1
> 
> 
>