mx28: fix i.MX28 spi driver
Subsystems (2)
PATCH 1/1 mx28: fix i.MX28 spi driver
The generic spi flash driver (drivers/mtd/spi/spi_flash.c) uses the spi low level driver's spi_xfer() function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.
This patch implements an exception for len=0 with the SPI_XFER_END flag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.
--- drivers/spi/mxs_spi.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
drivers/spi/mxs_spi.c +9 -3
@@ -129,9 +129,15 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,int len = bitlen /8 ;const char *tx = dout;char *rx = din; - -if (bitlen ==0 ) -return 0 ; +char dummy; + +if (bitlen ==0 ) { +if (flags &SPI_XFER_END ) { + rx = &dummy; + len =1 ; + }else +return 0 ; + }if (!rx && !tx)return 0 ;
PATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 9 lines
The generic spi flash driver (
drivers/mtd/spi/spi_flash.c) uses the spi low level driver'sspi_xfer()function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.This patch implements an exception for len=0 with the
SPI_XFER_ENDflag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.
This seems good, but it doesn't look too correct either (is there really no other way to deassert CS than doing dummy read?). Do you see an issue with current implementation on some board please?
Quoted 29 lines
Signed-off-by Matthias Fuchs <[email protected]>--- drivers/spi/mxs_spi.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)drivers/spi/mxs_spi.c +9
-3@@ -129,9 +129,15 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, int len = bitlen / 8;const char *tx = dout;char *rx = din; - -if (bitlen ==0 ) -return 0 ; +char dummy; + +if (bitlen ==0 ) { +if (flags &SPI_XFER_END ) { + rx = &dummy; + len =1 ; + }else +return 0 ; + }if (!rx && !tx)return 0 ;
PATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 5 lines · Marek Vasut
Quoted 5 lines
The generic spi flash driver (
drivers/mtd/spi/spi_flash.c) uses the spi low level driver'sspi_xfer()function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.
Quoted 7 lines
Quoted 4 lines
This patch implements an exception for len=0 with the
SPI_XFER_ENDflag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.This seems good, but it doesn't look too correct either (is there really no other way to deassert CS than doing dummy read?).
Not that I am aware of. Did you wrote that mxs_spi driver? How did you interpret the chips reference manual. I understood it this way: you need to tell the controller to deassert chip select before the final transfer.
I could be possible to change the driver implementation and use GPIOs for chip select. But I think that's not the philisophy of this controller.
Quoted 2 lines
Do you see an issue with
current implementation on some board please?
Yes, I installed an SST25VF032B SPI on the MX28EVK in preparation of our own board. The chip was detected correctly after power on, but erasing (that's one place where the status register polling is used) more than one page does not work and then after reset the device is not correctly detected.
I would expect problems on the M28EVK (Denx) also. You might want to turn SPI + spi_flash debugging to see it. Also I found a posting from Alexander Keller on the u-boot list from 12/13/2011. Perhaps the can check if this patch fixes his problems also.
Matthias
Quoted 34 lines
Quoted 20 lines
Signed-off-by Matthias Fuchs <[email protected]>--- drivers/spi/mxs_spi.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)drivers/spi/mxs_spi.c +2
-3@@ -129,9 +129,15 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, int len = bitlen / 8;const char *tx = dout;char *rx = din; - -if (bitlen ==0 ) -return 0 ; +char dummy; +drivers/spi/mxs_spi.c int spi_xfer(struct spi_slave *slave, unsigned int · line 143
Quoted 10 lines
drivers/spi/mxs_spi.c +7
-0+if (bitlen ==0 ) { +if (flags &SPI_XFER_END ) { + rx = &dummy; + len =1 ; + }else +return 0 ; + }if (!rx && !tx)return 0 ;_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Signature · 13 lines
------------------------------------------------------------------------- Dipl.-Ing. Matthias Fuchs Head of System Design esd electronic system design gmbh Vahrenwalder Str. 207 - 30165 Hannover - GERMANY Phone: +49-511-37298-0 - Fax: +49-511-37298-68 Please visit our homepage http://www.esd.eu Quality Products - Made in Germany ------------------------------------------------------------------------- Gesch?ftsf?hrer: Klaus Detering Amtsgericht Hannover HRB 51373 - VAT-ID DE 115672832 -------------------------------------------------------------------------
PATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 31 lines · Marek Vasut
Quoted 11 lines
Quoted 9 lines
The generic spi flash driver (
drivers/mtd/spi/spi_flash.c) uses the spi low level driver'sspi_xfer()function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.This patch implements an exception for len=0 with the
SPI_XFER_ENDflag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.This seems good, but it doesn't look too correct either (is there really
no
Quoted 1 line
other way to deassert CS than doing dummy read?).
Not that I am aware of. Did you wrote that
mxs_spidriver? How did you interpret the
chips reference manual. I understood it this way: you need to tell the controller
to deassert chip select before the final transfer.I could be possible to change the driver implementation and use GPIOs for chip select. But I think that's not the philisophy of this controller.
Quoted 2 lines
Do you see an issue with
current implementation on some board please?Yes, I installed an SST25VF032B SPI on the MX28EVK in preparation of our own board. The chip was detected correctly after power on, but erasing (that's one place where the status register polling is used) more than one page
does not work and then after reset the device is not correctly detected.
Quoted 3 lines
I would expect problems on the M28EVK (Denx) also. You might want to turn SPI +
spi_flashdebugging to see it. Also I found a posting from Alexander Keller on the u-boot list from 12/13/2011. Perhaps the can check
I just want to confirm and thanks to Matthias for this patch. I already stated my problems by writing the SPI flash ...
So, I use the IMX28EVK with a Winbond W25Q64CV SPI flash chip and it's working great now.
Thanks for the great work.
Alexander
Quoted 54 lines
if
this patch fixes his problems also.Matthias
Quoted 34 lines
Quoted 20 lines
Signed-off-by Matthias Fuchs <[email protected]>--- drivers/spi/mxs_spi.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)drivers/spi/mxs_spi.c +2
-3@@ -129,9 +129,15 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, int len = bitlen / 8;const char *tx = dout;char *rx = din; - -if (bitlen ==0 ) -return 0 ; +char dummy; +drivers/spi/mxs_spi.c int spi_xfer(struct spi_slave *slave, unsigned int · line 143
Quoted 10 lines
drivers/spi/mxs_spi.c +7
-0+if (bitlen ==0 ) { +if (flags &SPI_XFER_END ) { + rx = &dummy; + len =1 ; + }else +return 0 ; + }if (!rx && !tx)return 0 ;_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-bootSignature · 13 lines
------------------------------------------------------------------------- Dipl.-Ing. Matthias Fuchs Head of System Design esd electronic system design gmbh Vahrenwalder Str. 207 - 30165 Hannover - GERMANY Phone: +49-511-37298-0 - Fax: +49-511-37298-68 Please visit our homepage http://www.esd.eu Quality Products - Made in Germany ------------------------------------------------------------------------- Gesch?ftsf?hrer: Klaus Detering Amtsgericht Hannover HRB 51373 - VAT-ID DE 115672832 -------------------------------------------------------------------------
Fabio Estevam
reviewerPATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 7 lines · Alexander Keller
I just want to confirm and thanks to Matthias for this patch. I already stated my problems by writing the SPI flash ...
So, I use the IMX28EVK with a Winbond W25Q64CV SPI flash chip and it's working great now.
Thanks for the great work.
Ok, great. Can you or Matthias post a patch adding SPI flash support to mx28evk?
I have just soldered one SPI flash on my board and would be glad to try it.
Regards,
Fabio Estevam
Fabio Estevam
reviewerPATCH 1/1 mx28: fix i.MX28 spi driver
In reply to Matthias FuchsHi Matthias,
Quoted 11 lines · Matthias Fuchs
The generic spi flash driver (
drivers/mtd/spi/spi_flash.c) uses the spi low level driver'sspi_xfer()function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.This patch implements an exception for len=0 with the
SPI_XFER_ENDflag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.Signed-off-by Matthias Fuchs <[email protected]>
What about the approach bellow (untested)?
drivers/spi/mxs_spi.c +2 -5
@@ -126,17 +126,14 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, {struct mxs_spi_slave *mxs_slave =to_mxs_slave (slave);struct mx28_ssp_regs *ssp_regs = mxs_slave->regs; -int len = bitlen /8 ; +unsigned int len = bitlen /8 ;const char *tx = dout;char *rx = din; -if (bitlen ==0 ) -return 0 ; -if (!rx && !tx)return 0 ; -if (flags &SPI_XFER_BEGIN ) +if ((flags &SPI_XFER_BEGIN ) && !len)mxs_spi_start_xfer (ssp_regs);while (len--) {
Fabio Estevam
reviewerPATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 8 lines · Fabio Estevam
Hi Matthias,
Quoted 5 lines · Matthias Fuchs
The generic spi flash driver (
drivers/mtd/spi/spi_flash.c) uses the spi low level driver'sspi_xfer()function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.
Quoted 8 lines
Quoted 6 lines
This patch implements an exception for len=0 with the
SPI_XFER_ENDflag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.Signed-off-by Matthias Fuchs <[email protected]>What about the approach bellow (untested)?
Actually I meant this:
drivers/spi/mxs_spi.c +1 -1
@@ -136,7 +136,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,if (!rx && !tx)return 0 ; -if (flags &SPI_XFER_BEGIN ) +if ((flags &SPI_XFER_BEGIN ) && len)mxs_spi_start_xfer (ssp_regs);while (len--) {
Fabio Estevam
reviewerPATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 1 line · Fabio Estevam
Actually I meant this:
One more time ;-)
drivers/spi/mxs_spi.c +1 -4
@@ -130,13 +130,10 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,const char *tx = dout;char *rx = din; -if (bitlen ==0 ) -return 0 ; -if (!rx && !tx)return 0 ; -if (flags &SPI_XFER_BEGIN ) +if ((flags &SPI_XFER_BEGIN ) && len)mxs_spi_start_xfer (ssp_regs);while (len--) {
PATCH 1/1 mx28: fix i.MX28 spi driver
This cannot work. I do not understand what you are trying to achieve with this.
Matthias
Quoted 27 lines · Fabio Estevam
Quoted 1 line · Fabio Estevam
Actually I meant this:
One more time ;-)
drivers/spi/mxs_spi.c +1
-4@@ -130,13 +130,10 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,const char *tx = dout;char *rx = din; -if (bitlen ==0 ) -return 0 ; -if (!rx && !tx)return 0 ; -if (flags &SPI_XFER_BEGIN ) +if ((flags &SPI_XFER_BEGIN ) && len)mxs_spi_start_xfer (ssp_regs);while (len--) {_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Fabio Estevam
reviewerPATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 1 line · Matthias Fuchs
This cannot work. I do not understand what you are trying to achieve with this.
I would like to avoid the extra dummy read that your patch proposes.
PATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 4 lines · Fabio Estevam
Quoted 1 line · Matthias Fuchs
This cannot work. I do not understand what you are trying to achieve with this.
I would like to avoid the extra dummy read that your patch proposes.
That's what I also tried. But from the ref manual I got no idea. When we do not find a hy to deassert the chip select manually, we cannot avoid this read.
It seems that the mainline linux only implements a GPIO based SPI driver. So we cannot take it for reference.
I'm wonderng how this works on the Denx' M28EVK.
Matthias
Quoted 4 lines
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Fabio Estevam
reviewerPATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 3 lines · Matthias Fuchs
That's what I also tried. But from the ref manual I got no idea. When we do not find a hy to deassert the chip select manually, we cannot avoid this read.
I was assuming that the chip select deassertion was correctly being handled by themxs_spi_end_xfer function.
This function handles the LOCK_CS and IGNORE_CRC bits that are responsible for chip select assertion/deassertion.
PATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 10 lines · Fabio Estevam
Quoted 3 lines · Matthias Fuchs
That's what I also tried. But from the ref manual I got no idea. When we do not find a hy to deassert the chip select manually, we cannot avoid this read.
I was assuming that the chip select deassertion was correctly being handled by the
mxs_spi_end_xferfunction.
Quoted 7 lines
This function handles the
LOCK_CSandIGNORE_CRCbits that are responsible for chip select assertion/deassertion.From the MX28 Reference Manual:
"
IGNORE_CRCbit: In SPI/SSI modes: When set to 1, deassert the chip select (SSn) pin after the command is executed."
Exactly. That's the point. It says "... after the command ...". So you need a transfer after calling mxs_spi_end_xfer(). That's why the current code calls it before transferring the final byte.
Matthias
Quoted 12 lines
Then I tought that
mxs_spi_end_xfershould be also called in the case where bitlen==0. In current code this does not happen.Quoted 2 lines
It seems that the mainline linux only implements a GPIO based SPI driver. So we cannot take it for reference.
There is no mxs spi driver in the mainline kernel yet.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Fabio Estevam
reviewerPATCH 1/1 mx28: fix i.MX28 spi driver
In reply to Matthias FuchsQuoted 11 lines · Matthias Fuchs
The generic spi flash driver (
drivers/mtd/spi/spi_flash.c) uses the spi low level driver'sspi_xfer()function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.This patch implements an exception for len=0 with the
SPI_XFER_ENDflag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.Signed-off-by Matthias Fuchs <[email protected]>
Soldered a SST25VF016B on a mx28evk (and also the SPI pullups) and verified that the flash can be erased succesfully now.
I suggest that this patch gets applied as it fixes a real issue.
Thanks,
Fabio Estevam
PATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 15 lines · Matthias Fuchs
Quoted 11 lines
The generic spi flash driver (
drivers/mtd/spi/spi_flash.c) uses the spi low level driver'sspi_xfer()function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.This patch implements an exception for len=0 with the
SPI_XFER_ENDflag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.Signed-off-by Matthias Fuchs <[email protected]>Tested-by Fabio Estevam <[email protected]>
Quoted 8 lines
Soldered a SST25VF016B on a mx28evk (and also the SPI pullups) and verified that the flash can be erased succesfully now.
I suggest that this patch gets applied as it fixes a real issue.
Thanks,
Fabio Estevam
I'm all for it.
M
PATCH 1/1 mx28: fix i.MX28 spi driver
In reply to Matthias FuchsQuoted 5 lines · Matthias Fuchs
The generic spi flash driver (
drivers/mtd/spi/spi_flash.c) uses the spi low level driver'sspi_xfer()function with len=0 to deassert the SPI flash' chip select. But the i.MX28 spi driver rejects this call due to len=0.
Quoted 7 lines
This patch implements an exception for len=0 with the
SPI_XFER_ENDflag set. This results in an extra read with the chip select being deasserted afterwards. There seems to be no way to deassert the signal by hand.Signed-off-by Matthias Fuchs <[email protected]>---
Applied to u-boot-imx, thanks.
Best regards,
Stefano Babic
Signature · 5 lines
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de =====================================================================
Fabio Estevam
reviewerPATCH 1/1 mx28: fix i.MX28 spi driver
Hi Stefano,
Quoted 1 line · Stefano Babic
Applied to u-boot-imx, thanks.
Which branch, please?
I am not able to find it.
Regards,
Fabio Estevam
PATCH 1/1 mx28: fix i.MX28 spi driver
Quoted 6 lines · Fabio Estevam
Hi Stefano,
Quoted 1 line · Stefano Babic
Applied to u-boot-imx, thanks.
Hi Fabio,
Quoted 3 lines
Which branch, please?
I am not able to find it.
Sorry, I have not yet pushed to the server - done now.
Best regards,
Stefano Babic
Signature · 5 lines
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de =====================================================================