| Message ID | 1447136531-1325-2-git-send-email-vigneshr@ti.com |
|---|---|
| State | Accepted |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
On Tuesday 10 November 2015 11:52 AM, Vignesh R wrote: > Add dummy readl after invalidating cmd field of QSPI_CMD_REG to ensure > bus sync. Without this device's CS is not deactivated reliably leading > to failure to enumerate flash or failure to set quad enable bit on > Macronix flash present on am437x-sk and am437x-idk evms. > > Signed-off-by: Vignesh R <vigneshr@ti.com> > --- Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com> Regards Mugunthan V N
On 10 November 2015 at 11:52, Vignesh R <vigneshr@ti.com> wrote: > Add dummy readl after invalidating cmd field of QSPI_CMD_REG to ensure > bus sync. Without this device's CS is not deactivated reliably leading > to failure to enumerate flash or failure to set quad enable bit on > Macronix flash present on am437x-sk and am437x-idk evms. > > Signed-off-by: Vignesh R <vigneshr@ti.com> > --- > drivers/spi/ti_qspi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c > index bd63db8a2acc..ba2da017c21a 100644 > --- a/drivers/spi/ti_qspi.c > +++ b/drivers/spi/ti_qspi.c > @@ -170,6 +170,8 @@ void spi_cs_deactivate(struct spi_slave *slave) > debug("spi_cs_deactivate: 0x%08x\n", (u32)slave); > > writel(qslave->cmd | QSPI_INVAL, &qslave->base->cmd); > + /* dummy readl to ensure bus sync */ > + readl(&qslave->base->cmd); Isn't effect anything for non-quad case? and dummy bytes might changed based on quad and quadio does this single dummy readl is same for those cases as well? thanks!
On 11/17/2015 11:33 AM, Jagan Teki wrote: > On 10 November 2015 at 11:52, Vignesh R <vigneshr@ti.com> wrote: >> Add dummy readl after invalidating cmd field of QSPI_CMD_REG to ensure >> bus sync. Without this device's CS is not deactivated reliably leading >> to failure to enumerate flash or failure to set quad enable bit on >> Macronix flash present on am437x-sk and am437x-idk evms. >> >> Signed-off-by: Vignesh R <vigneshr@ti.com> >> --- >> drivers/spi/ti_qspi.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >> index bd63db8a2acc..ba2da017c21a 100644 >> --- a/drivers/spi/ti_qspi.c >> +++ b/drivers/spi/ti_qspi.c >> @@ -170,6 +170,8 @@ void spi_cs_deactivate(struct spi_slave *slave) >> debug("spi_cs_deactivate: 0x%08x\n", (u32)slave); >> >> writel(qslave->cmd | QSPI_INVAL, &qslave->base->cmd); >> + /* dummy readl to ensure bus sync */ >> + readl(&qslave->base->cmd); > > Isn't effect anything for non-quad case? and dummy bytes might changed > based on quad and quadio does this single dummy readl is same for > those cases as well? No, this dummy readl is not related as dummy bytes used in QUAD mode. In ti_qspi controller, in order to end a read/write request (ie to deassert ChipSelect) invalid cmd(0x4) needs to be written to CMD field of QSPI_CMD_REG (which is done by above function) in all modes. Sometimes, this write may have been flushed out of ARM but may not have reached the QSPI IP yet (ie bus/interconnect is not flushed yet) hence ChipSelect is not deactivated. Therefore, the successive write/read operation may overlaps resulting in wrong data being read from flash. Reading back QSPI_CMD_REG makes sure that the bus/interconnect is flushed and register write is affected making sure CS is deactivated.
On 17 November 2015 at 11:57, Vignesh R <vigneshr@ti.com> wrote: > > > On 11/17/2015 11:33 AM, Jagan Teki wrote: >> On 10 November 2015 at 11:52, Vignesh R <vigneshr@ti.com> wrote: >>> Add dummy readl after invalidating cmd field of QSPI_CMD_REG to ensure >>> bus sync. Without this device's CS is not deactivated reliably leading >>> to failure to enumerate flash or failure to set quad enable bit on >>> Macronix flash present on am437x-sk and am437x-idk evms. >>> >>> Signed-off-by: Vignesh R <vigneshr@ti.com> >>> --- >>> drivers/spi/ti_qspi.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >>> index bd63db8a2acc..ba2da017c21a 100644 >>> --- a/drivers/spi/ti_qspi.c >>> +++ b/drivers/spi/ti_qspi.c >>> @@ -170,6 +170,8 @@ void spi_cs_deactivate(struct spi_slave *slave) >>> debug("spi_cs_deactivate: 0x%08x\n", (u32)slave); >>> >>> writel(qslave->cmd | QSPI_INVAL, &qslave->base->cmd); >>> + /* dummy readl to ensure bus sync */ >>> + readl(&qslave->base->cmd); >> >> Isn't effect anything for non-quad case? and dummy bytes might changed >> based on quad and quadio does this single dummy readl is same for >> those cases as well? > > No, this dummy readl is not related as dummy bytes used in QUAD mode. > > In ti_qspi controller, in order to end a read/write request (ie to > deassert ChipSelect) invalid cmd(0x4) needs to be written to CMD field > of QSPI_CMD_REG (which is done by above function) in all modes. > Sometimes, this write may have been flushed out of ARM but may not have > reached the QSPI IP yet (ie bus/interconnect is not flushed yet) hence > ChipSelect is not deactivated. Therefore, the successive write/read > operation may overlaps resulting in wrong data being read from flash. > Reading back QSPI_CMD_REG makes sure that the bus/interconnect is > flushed and register write is affected making sure CS is deactivated. Thanks for the info. Reviewed-by: Jagan Teki <jteki@openedev.com>
On 17 November 2015 at 12:16, Jagan Teki <jteki@openedev.com> wrote: > On 17 November 2015 at 11:57, Vignesh R <vigneshr@ti.com> wrote: >> >> >> On 11/17/2015 11:33 AM, Jagan Teki wrote: >>> On 10 November 2015 at 11:52, Vignesh R <vigneshr@ti.com> wrote: >>>> Add dummy readl after invalidating cmd field of QSPI_CMD_REG to ensure >>>> bus sync. Without this device's CS is not deactivated reliably leading >>>> to failure to enumerate flash or failure to set quad enable bit on >>>> Macronix flash present on am437x-sk and am437x-idk evms. >>>> >>>> Signed-off-by: Vignesh R <vigneshr@ti.com> >>>> --- >>>> drivers/spi/ti_qspi.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >>>> index bd63db8a2acc..ba2da017c21a 100644 >>>> --- a/drivers/spi/ti_qspi.c >>>> +++ b/drivers/spi/ti_qspi.c >>>> @@ -170,6 +170,8 @@ void spi_cs_deactivate(struct spi_slave *slave) >>>> debug("spi_cs_deactivate: 0x%08x\n", (u32)slave); >>>> >>>> writel(qslave->cmd | QSPI_INVAL, &qslave->base->cmd); >>>> + /* dummy readl to ensure bus sync */ >>>> + readl(&qslave->base->cmd); >>> >>> Isn't effect anything for non-quad case? and dummy bytes might changed >>> based on quad and quadio does this single dummy readl is same for >>> those cases as well? >> >> No, this dummy readl is not related as dummy bytes used in QUAD mode. >> >> In ti_qspi controller, in order to end a read/write request (ie to >> deassert ChipSelect) invalid cmd(0x4) needs to be written to CMD field >> of QSPI_CMD_REG (which is done by above function) in all modes. >> Sometimes, this write may have been flushed out of ARM but may not have >> reached the QSPI IP yet (ie bus/interconnect is not flushed yet) hence >> ChipSelect is not deactivated. Therefore, the successive write/read >> operation may overlaps resulting in wrong data being read from flash. >> Reading back QSPI_CMD_REG makes sure that the bus/interconnect is >> flushed and register write is affected making sure CS is deactivated. > > Thanks for the info. > > Reviewed-by: Jagan Teki <jteki@openedev.com> Applied to u-boot-spi/master thanks!
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c index bd63db8a2acc..ba2da017c21a 100644 --- a/drivers/spi/ti_qspi.c +++ b/drivers/spi/ti_qspi.c @@ -170,6 +170,8 @@ void spi_cs_deactivate(struct spi_slave *slave) debug("spi_cs_deactivate: 0x%08x\n", (u32)slave); writel(qslave->cmd | QSPI_INVAL, &qslave->base->cmd); + /* dummy readl to ensure bus sync */ + readl(&qslave->base->cmd); } void spi_init(void)
Add dummy readl after invalidating cmd field of QSPI_CMD_REG to ensure bus sync. Without this device's CS is not deactivated reliably leading to failure to enumerate flash or failure to set quad enable bit on Macronix flash present on am437x-sk and am437x-idk evms. Signed-off-by: Vignesh R <vigneshr@ti.com> --- drivers/spi/ti_qspi.c | 2 ++ 1 file changed, 2 insertions(+)