[U-Boot] [BUG] SPI NOR for SST25V hangs for TI AM335x.

Hi,
There is a bug in the SST25V SPI NOR for the TI AM335x. On a write the SPI NOR hangs. This is because of the spi_release_bus(..) in the spi_flash_read_common call done by spi_flash_sr_ready(..) called in the spi_flash_wait_till_ready(..) in sst_write_wp(..). This call clears all the registers and the setting associated with it and then calls the spi_flash_cmd_write_disable in sst_write_wp(..) with the wrong SPI settings.
Also there are spi_reset(..) calls happening in the sst_write_wp(..) which seems to be unnecessary when the data sheet does not mention any need for resets. Calling unnecessary resets simply increases the latency in the driver and causes more bugs.
Currently commenting out the spi_release_bus(..) makes a successful write.
Thanks, Gautam.

Hi,
Adding Vignesh. He should be able to help.
On Monday 21 May 2018 11:53 PM, Gautam Bhat wrote:
Hi,
There is a bug in the SST25V SPI NOR for the TI AM335x. On a write the SPI NOR hangs. This is because of the spi_release_bus(..) in the spi_flash_read_common call done by spi_flash_sr_ready(..) called in the spi_flash_wait_till_ready(..) in sst_write_wp(..). This call clears all the registers and the setting associated with it and then calls the spi_flash_cmd_write_disable in sst_write_wp(..) with the wrong SPI settings.
Also there are spi_reset(..) calls happening in the sst_write_wp(..) which seems to be unnecessary when the data sheet does not mention any need for resets. Calling unnecessary resets simply increases the latency in the driver and causes more bugs.
Currently commenting out the spi_release_bus(..) makes a successful write.
Thanks, Gautam. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Thanks, Faiz

On Tue, May 22, 2018 at 4:17 PM, Faiz Abbas faiz_abbas@ti.com wrote:
Hi,
Adding Vignesh. He should be able to help.
On Monday 21 May 2018 11:53 PM, Gautam Bhat wrote:
Hi,
There is a bug in the SST25V SPI NOR for the TI AM335x. On a write the SPI NOR hangs. This is because of the spi_release_bus(..) in the spi_flash_read_common call done by spi_flash_sr_ready(..) called in the spi_flash_wait_till_ready(..) in sst_write_wp(..). This call clears all the registers and the setting associated with it and then calls the spi_flash_cmd_write_disable in sst_write_wp(..) with the wrong SPI settings.
Also there are spi_reset(..) calls happening in the sst_write_wp(..) which seems to be unnecessary when the data sheet does not mention any need for resets. Calling unnecessary resets simply increases the latency in the driver and causes more bugs.
Currently commenting out the spi_release_bus(..) makes a successful write.
Thanks, Gautam. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Thanks, Faiz
To make things easier the following is my analysis:
In spi_flash.c:sst_write_wp(...) we have the spi_flash_wait_till_ready(...). In this function there is a check for spi_flash_ready(...) which calls the spi_flash_sr_ready(...). This function checks the "status" register which finally calls the read_sr(...). In the read_sr(...) the checking of the status register is done by the function spi_flash_read_common(...). The problem with this is that there is claim and especially a release call which clears all the registers and resets the SPI block. In the subsequent call for write disable WRDI in sst_write_wp(...) i.e. spi_flash_cmd_write_disable(...) there is no effort to do a claim and a release because it uses the simple spi_flash_cmd(...) call which expects the registers to be configured and thereby goes into an undefined state when a SPI transaction is made.
There are two problems here:
1) There is no need to claim and release just to check the status register. The claim and release has to be done at the start and the end of the transaction and not in between a Auto Address Increment word program. For this the check of the Status Register(SR) should be a simple spi_flash_cmd call. 2) The design of the read_sr wrapper should also have a simple spi_flash_cmd call for conditions such as this.
I have fixed this problem temporarily by writing a wrapper as in 2) and doing a simple check of whether there is a write.
There is another problem of the writes not actually happening which I am trying to fix now.
This problem was also raised previously in the mailing list here: https://lists.denx.de/pipermail/u-boot/2016-June/257447.html
Thanks, Gautam.

On Wed, May 23, 2018 at 11:29 AM, Gautam Bhat mindentropy@gmail.com wrote:
On Tue, May 22, 2018 at 4:17 PM, Faiz Abbas faiz_abbas@ti.com wrote:
Hi,
Adding Vignesh. He should be able to help.
On Monday 21 May 2018 11:53 PM, Gautam Bhat wrote:
Hi,
There is a bug in the SST25V SPI NOR for the TI AM335x. On a write the SPI NOR hangs. This is because of the spi_release_bus(..) in the spi_flash_read_common call done by spi_flash_sr_ready(..) called in the spi_flash_wait_till_ready(..) in sst_write_wp(..). This call clears all the registers and the setting associated with it and then calls the spi_flash_cmd_write_disable in sst_write_wp(..) with the wrong SPI settings.
Also there are spi_reset(..) calls happening in the sst_write_wp(..) which seems to be unnecessary when the data sheet does not mention any need for resets. Calling unnecessary resets simply increases the latency in the driver and causes more bugs.
Currently commenting out the spi_release_bus(..) makes a successful write.
Thanks, Gautam. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Thanks, Faiz
To make things easier the following is my analysis:
In spi_flash.c:sst_write_wp(...) we have the spi_flash_wait_till_ready(...). In this function there is a check for spi_flash_ready(...) which calls the spi_flash_sr_ready(...). This function checks the "status" register which finally calls the read_sr(...). In the read_sr(...) the checking of the status register is done by the function spi_flash_read_common(...). The problem with this is that there is claim and especially a release call which clears all the registers and resets the SPI block. In the subsequent call for write disable WRDI in sst_write_wp(...) i.e. spi_flash_cmd_write_disable(...) there is no effort to do a claim and a release because it uses the simple spi_flash_cmd(...) call which expects the registers to be configured and thereby goes into an undefined state when a SPI transaction is made.
There are two problems here:
- There is no need to claim and release just to check the status
register. The claim and release has to be done at the start and the end of the transaction and not in between a Auto Address Increment word program. For this the check of the Status Register(SR) should be a simple spi_flash_cmd call. 2) The design of the read_sr wrapper should also have a simple spi_flash_cmd call for conditions such as this.
I have fixed this problem temporarily by writing a wrapper as in 2) and doing a simple check of whether there is a write.
There is another problem of the writes not actually happening which I am trying to fix now.
This problem was also raised previously in the mailing list here: https://lists.denx.de/pipermail/u-boot/2016-June/257447.html
Thanks, Gautam.
An update. My writes are happening! Forgot to erase before write.
-Gautam.
participants (2)
-
Faiz Abbas
-
Gautam Bhat