Re: [PATCH] spi: zynqmp_gqspi: fix set_speed bug on multiple runs

st 20. 1. 2021 v 21:29 odesílatel Brandon Maier brandon.maier@rockwellcollins.com napsal:

If zynqmp_qspi_set_speed() is called multiple times with the same speed, then on the second call it will skip recalculating the baud_rate_val as it assumes the speed is already configured correctly. But it will still write the baud_rate_val to the configuration register and call zynqmp_gqspi_set_tapdelay(). Because it skipped recalculating the baud_rate_val, it will use the initial value of 0 . This causes the driver to run at maximum speed which for many spi flashes is too fast and causes data corruption.

Instead only write out a new baud_rate_val if we have calculated the correct baud_rate_val.

This opens up another issue with the "if (speed == 0)", we don't save off the new plat->speed_hz value when setting the baud rate on the speed=0 path. Instead mimic what the Linux zynqmp gqspi driver does, and have speed==0 just use the same calculation as a normal speed. That will cause the baud_rate_val to use the slowest speed possible, which is the safest option.

Signed-off-by: Brandon Maier brandon.maier@rockwellcollins.com CC: jagan@amarulasolutions.com CC: michal.simek@xilinx.com CC: Ashok Reddy Soma ashokred@xilinx.com

---

drivers/spi/zynqmp_gqspi.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c index c7db43a09a..6641c2e9d5 100644 --- a/drivers/spi/zynqmp_gqspi.c +++ b/drivers/spi/zynqmp_gqspi.c @@ -320,12 +320,9 @@ static int zynqmp_qspi_set_speed(struct udevice *bus, uint speed) if (speed > plat->frequency) speed = plat->frequency;

•   /* Set the clock frequency */
  

•   confr = readl(&regs->confr);
  

•   if (speed == 0) {
  

•           /* Set baudrate x8, if the freq is 0 */
  

•           baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
  

•   } else if (plat->speed_hz != speed) {
  

•   if (plat->speed_hz != speed) {
  

•           /* Set the clock frequency */
  

•           /* If speed == 0, default to lowest speed */
            while ((baud_rate_val < 8) &&
                   ((plat->frequency /
                   (2 << baud_rate_val)) > speed))
  

@@ -335,13 +332,15 @@ static int zynqmp_qspi_set_speed(struct udevice *bus, uint speed) baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;

            plat->speed_hz = plat->frequency / (2 << baud_rate_val);

•   }
  

•   confr &= ~GQSPI_BAUD_DIV_MASK;
  

•   confr |= (baud_rate_val << 3);
  

•   writel(confr, &regs->confr);
  
  

•   zynqmp_qspi_set_tapdelay(bus, baud_rate_val);
  

•   debug("regs=%p, speed=%d\n", priv->regs, plat->speed_hz);
  

•           confr = readl(&regs->confr);
  

•           confr &= ~GQSPI_BAUD_DIV_MASK;
  

•           confr |= (baud_rate_val << 3);
  

•           writel(confr, &regs->confr);
  

•           zynqmp_qspi_set_tapdelay(bus, baud_rate_val);
  

•           debug("regs=%p, speed=%d\n", priv->regs, plat->speed_hz);
  

•   }
  
    return 0;
  

}

2.29.1

Applied. Ashok: Please fix it on the top of this one if there is something wrong.

Thanks, Michal