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

[Gautam Bhat]

On Wed, May 23, 2018 at 11:29 AM, Gautam Bhat <mindentropy at gmail.com> wrote:
> On Tue, May 22, 2018 at 4:17 PM, Faiz Abbas <faiz_abbas at 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 at 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.

An update. My writes are happening! Forgot to erase before write.

-Gautam.