[U-Boot] [PATCH v2 1/3] spi: ti_qspi: Fix failure on multiple READ_ID cmd

Populating QSPI_RD_SNGL bit(0x1) in priv->cmd means that value QSPI_INVAL (0x4) is not written to CMD field of QSPI_SPI_CMD_REG in ti_qspi_cs_deactivate(). Therefore CS is never deactivated between successive READ ID which results in sf probe to fail. Fix this by not populating priv->cmd with QSPI_RD_SNGL and OR it wih priv->cmd as required (similar to the convention followed in the driver).
Signed-off-by: Vignesh R vigneshr@ti.com ---
v2: fix up debug print
drivers/spi/ti_qspi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c index 9a372ad31dae..a850aa26ec2b 100644 --- a/drivers/spi/ti_qspi.c +++ b/drivers/spi/ti_qspi.c @@ -247,13 +247,12 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen, debug("tx done, status %08x\n", status); } if (rxp) { - priv->cmd |= QSPI_RD_SNGL; debug("rx cmd %08x dc %08x\n", - priv->cmd, priv->dc); + ((u32)(priv->cmd | QSPI_RD_SNGL)), priv->dc); #ifdef CONFIG_DRA7XX udelay(500); #endif - writel(priv->cmd, &priv->base->cmd); + writel(priv->cmd | QSPI_RD_SNGL, &priv->base->cmd); status = readl(&priv->base->status); timeout = QSPI_TIMEOUT; while ((status & QSPI_WC_BUSY) != QSPI_XFER_DONE) {

clk_div is uninitialized at the beginning of ti_spi_set_speed(), move debug() print after clk_div calculation to avoid compiler warning and to have proper value of clk_div printed during debugging.
Signed-off-by: Vignesh R vigneshr@ti.com ---
v2: no change
drivers/spi/ti_qspi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c index a850aa26ec2b..56ae29a3ee7c 100644 --- a/drivers/spi/ti_qspi.c +++ b/drivers/spi/ti_qspi.c @@ -110,13 +110,13 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz) { uint clk_div;
- debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, clk_div); - if (!hz) clk_div = 0; else clk_div = (QSPI_FCLK / hz) - 1;
+ debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, clk_div); + /* disable SCLK */ writel(readl(&priv->base->clk_ctrl) & ~QSPI_CLK_EN, &priv->base->clk_ctrl);

On 22 July 2016 at 10:55, Vignesh R vigneshr@ti.com wrote:
clk_div is uninitialized at the beginning of ti_spi_set_speed(), move debug() print after clk_div calculation to avoid compiler warning and to have proper value of clk_div printed during debugging.
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Jagan Teki jteki@openedev.com

On Friday 22 July 2016 10:55 AM, Vignesh R wrote:
clk_div is uninitialized at the beginning of ti_spi_set_speed(), move debug() print after clk_div calculation to avoid compiler warning and to have proper value of clk_div printed during debugging.
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Mugunthan V N mugunthanvnm@ti.com
Regards Mugunthan V N

On 22 July 2016 at 15:39, Mugunthan V N mugunthanvnm@ti.com wrote:
On Friday 22 July 2016 10:55 AM, Vignesh R wrote:
clk_div is uninitialized at the beginning of ti_spi_set_speed(), move debug() print after clk_div calculation to avoid compiler warning and to have proper value of clk_div printed during debugging.
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Mugunthan V N mugunthanvnm@ti.com
Applied to u-boot-spi/master
thanks!

As per commit b545a98f5dc563 ("spi: ti_qspi: Add delay for successful bulk erase) says its added to meet bulk erase timing constraints. But bulk erase is a cmd to flash and delay in read path does not make sense. Morever, testing on DRA74/DRA72 evm has shown that this delay is no longer required.
Signed-off-by: Vignesh R vigneshr@ti.com ---
v2: no patch
drivers/spi/ti_qspi.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c index 56ae29a3ee7c..fa7ee229878a 100644 --- a/drivers/spi/ti_qspi.c +++ b/drivers/spi/ti_qspi.c @@ -249,9 +249,6 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen, if (rxp) { debug("rx cmd %08x dc %08x\n", ((u32)(priv->cmd | QSPI_RD_SNGL)), priv->dc); - #ifdef CONFIG_DRA7XX - udelay(500); - #endif writel(priv->cmd | QSPI_RD_SNGL, &priv->base->cmd); status = readl(&priv->base->status); timeout = QSPI_TIMEOUT;

On 22 July 2016 at 10:55, Vignesh R vigneshr@ti.com wrote:
As per commit b545a98f5dc563 ("spi: ti_qspi: Add delay for successful bulk erase) says its added to meet bulk erase timing constraints. But bulk erase is a cmd to flash and delay in read path does not make sense. Morever, testing on DRA74/DRA72 evm has shown that this delay is no longer required.
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Jagan Teki jteki@openedev.com
v2: no patch
drivers/spi/ti_qspi.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c index 56ae29a3ee7c..fa7ee229878a 100644 --- a/drivers/spi/ti_qspi.c +++ b/drivers/spi/ti_qspi.c @@ -249,9 +249,6 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen, if (rxp) { debug("rx cmd %08x dc %08x\n", ((u32)(priv->cmd | QSPI_RD_SNGL)), priv->dc);
#ifdef CONFIG_DRA7XX
udelay(500);
#endif
Thanks for this change.
-- Jagan.

On Friday 22 July 2016 10:55 AM, Vignesh R wrote:
As per commit b545a98f5dc563 ("spi: ti_qspi: Add delay for successful bulk erase) says its added to meet bulk erase timing constraints. But bulk erase is a cmd to flash and delay in read path does not make sense. Morever, testing on DRA74/DRA72 evm has shown that this delay is no longer required.
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Mugunthan V N mugunthanvnm@ti.com
Regards Mugunthan V N

On 22 July 2016 at 15:39, Mugunthan V N mugunthanvnm@ti.com wrote:
On Friday 22 July 2016 10:55 AM, Vignesh R wrote:
As per commit b545a98f5dc563 ("spi: ti_qspi: Add delay for successful bulk erase) says its added to meet bulk erase timing constraints. But bulk erase is a cmd to flash and delay in read path does not make sense. Morever, testing on DRA74/DRA72 evm has shown that this delay is no longer required.
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Mugunthan V N mugunthanvnm@ti.com
Applied to u-boot-spi/master
thanks!

On 22 July 2016 at 10:55, Vignesh R vigneshr@ti.com wrote:
Populating QSPI_RD_SNGL bit(0x1) in priv->cmd means that value QSPI_INVAL (0x4) is not written to CMD field of QSPI_SPI_CMD_REG in ti_qspi_cs_deactivate(). Therefore CS is never deactivated between successive READ ID which results in sf probe to fail. Fix this by not populating priv->cmd with QSPI_RD_SNGL and OR it wih priv->cmd as required (similar to the convention followed in the driver).
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Jagan Teki jteki@openedev.com
-- Jagan.

On Friday 22 July 2016 10:55 AM, Vignesh R wrote:
Populating QSPI_RD_SNGL bit(0x1) in priv->cmd means that value QSPI_INVAL (0x4) is not written to CMD field of QSPI_SPI_CMD_REG in ti_qspi_cs_deactivate(). Therefore CS is never deactivated between successive READ ID which results in sf probe to fail. Fix this by not populating priv->cmd with QSPI_RD_SNGL and OR it wih priv->cmd as required (similar to the convention followed in the driver).
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Mugunthan V N mugunthanvnm@ti.com
Regards Mugunthan V N

On 22 July 2016 at 15:38, Mugunthan V N mugunthanvnm@ti.com wrote:
On Friday 22 July 2016 10:55 AM, Vignesh R wrote:
Populating QSPI_RD_SNGL bit(0x1) in priv->cmd means that value QSPI_INVAL (0x4) is not written to CMD field of QSPI_SPI_CMD_REG in ti_qspi_cs_deactivate(). Therefore CS is never deactivated between successive READ ID which results in sf probe to fail. Fix this by not populating priv->cmd with QSPI_RD_SNGL and OR it wih priv->cmd as required (similar to the convention followed in the driver).
Signed-off-by: Vignesh R vigneshr@ti.com
Reviewed-by: Mugunthan V N mugunthanvnm@ti.com
Applied to u-boot-spi/master
thanks!
participants (3)
-
Jagan Teki
-
Mugunthan V N
-
Vignesh R