[U-Boot] x86: SPI flash broken with SPI NOR

[U-Boot] x86: SPI flash broken with SPI NOR

[Simon Glass]

Hi Bin,

I find that 'sf test 0 1000' does not work anymore since this commit:

c4e8862308 mtd: spi: Switch to new SPI NOR framework

Have you seen any problems? This seems to prevent proper SPI flash writing
(not sure about reading).

Also, it seemed to increase code size on samus by >2KB.

Regards,
Simon

[U-Boot] x86: SPI flash broken with SPI NOR

[Vignesh Raghavendra]

On 26/04/19 4:56 AM, Simon Glass wrote:
> Hi Bin,
> 
> I find that 'sf test 0 1000' does not work anymore since this commit:
> 
> c4e8862308 mtd: spi: Switch to new SPI NOR framework
> 
> Have you seen any problems? This seems to prevent proper SPI flash
> writing (not sure about reading).
> 
Sorry for that.

Could you see if this patch[1] fixes the write issue?

> Also, it seemed to increase code size on samus by >2KB.
> 

Above commit ports Linux SPI NOR framework in order to support 4 byte
addressing SPI NOR opcodes and spi_mem_ops (to support both SPI NOR and
SPI NAND flashes using single SPI driver) therefore there is code size
increase.

But, I did provide SPL_SPI_FLASH_TINY which can be enabled to reduce
binary size on resource constraint board. Note that SPL_SPI_FLASH_TINY
only supports reading from flash and does not support write (Since SPL
just needs to support reading next stage image)

[1] http://patchwork.ozlabs.org/patch/1090121/


-- 
Regards
Vignesh

[U-Boot] x86: SPI flash broken with SPI NOR

[Bin Meng]

Hi Simon,

On Fri, Apr 26, 2019 at 7:26 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> I find that 'sf test 0 1000' does not work anymore since this commit:
>
> c4e8862308 mtd: spi: Switch to new SPI NOR framework
>
> Have you seen any problems? This seems to prevent proper SPI flash writing (not sure about reading).

Yes, this seems to be broken. I just tested it on MinnowMax.

'sf probe' does not succeed anymore.

>
> Also, it seemed to increase code size on samus by >2KB.

Regards,
Bin

[U-Boot] x86: SPI flash broken with SPI NOR

[Vignesh Raghavendra]

Bin,

On 26/04/19 7:33 PM, Bin Meng wrote:
> Hi Simon,
> 
> On Fri, Apr 26, 2019 at 7:26 AM Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Bin,
>>
>> I find that 'sf test 0 1000' does not work anymore since this commit:
>>
>> c4e8862308 mtd: spi: Switch to new SPI NOR framework
>>
>> Have you seen any problems? This seems to prevent proper SPI flash writing (not sure about reading).
> 
> Yes, this seems to be broken. I just tested it on MinnowMax.
>

What is the flash part on this board?

 
> 'sf probe' does not succeed anymore.
> 

Could you enable all debug prints in spi_mem_exec_op() in spi-mem.c:
https://elixir.bootlin.com/u-boot/latest/source/drivers/spi/spi-mem.c#L376

And the post the full log here. If the log is too big please paste it to pastebin.ubuntu.com

Also please apply http://patchwork.ozlabs.org/patch/1090121/ on top of latest tree before doing any write to flash

Sorry for the trouble.

>>
>> Also, it seemed to increase code size on samus by >2KB.
> 
> Regards,
> Bin
> 

[U-Boot] x86: SPI flash broken with SPI NOR

[Bin Meng]

Hi Vignesh,

On Sat, Apr 27, 2019 at 12:26 AM Vignesh Raghavendra <vigneshr@ti.com> wrote:
>
> Bin,
>
> On 26/04/19 7:33 PM, Bin Meng wrote:
> > Hi Simon,
> >
> > On Fri, Apr 26, 2019 at 7:26 AM Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Hi Bin,
> >>
> >> I find that 'sf test 0 1000' does not work anymore since this commit:
> >>
> >> c4e8862308 mtd: spi: Switch to new SPI NOR framework
> >>
> >> Have you seen any problems? This seems to prevent proper SPI flash writing (not sure about reading).
> >
> > Yes, this seems to be broken. I just tested it on MinnowMax.
> >
>
> What is the flash part on this board?
>
>
> > 'sf probe' does not succeed anymore.
> >
>
> Could you enable all debug prints in spi_mem_exec_op() in spi-mem.c:
> https://elixir.bootlin.com/u-boot/latest/source/drivers/spi/spi-mem.c#L376
>
> And the post the full log here. If the log is too big please paste it to pastebin.ubuntu.com
>
> Also please apply http://patchwork.ozlabs.org/patch/1090121/ on top of latest tree before doing any write to flash
>
> Sorry for the trouble.
>

Sorry for the long delay. I finally got time to look into this.

However with the latest u-boot/master, the SPI flash seems to work
again on Intel MinnowMax. Did you recall anything changes in the
spi-nor recently?

Regards,
Bin

[U-Boot] x86: SPI flash broken with SPI NOR

[Bin Meng]

Hi Vignesh,

On Wed, Jun 5, 2019 at 9:40 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Vignesh,
>
> On Sat, Apr 27, 2019 at 12:26 AM Vignesh Raghavendra <vigneshr@ti.com> wrote:
> >
> > Bin,
> >
> > On 26/04/19 7:33 PM, Bin Meng wrote:
> > > Hi Simon,
> > >
> > > On Fri, Apr 26, 2019 at 7:26 AM Simon Glass <sjg@chromium.org> wrote:
> > >>
> > >> Hi Bin,
> > >>
> > >> I find that 'sf test 0 1000' does not work anymore since this commit:
> > >>
> > >> c4e8862308 mtd: spi: Switch to new SPI NOR framework
> > >>
> > >> Have you seen any problems? This seems to prevent proper SPI flash writing (not sure about reading).
> > >
> > > Yes, this seems to be broken. I just tested it on MinnowMax.
> > >
> >
> > What is the flash part on this board?
> >
> >
> > > 'sf probe' does not succeed anymore.
> > >
> >
> > Could you enable all debug prints in spi_mem_exec_op() in spi-mem.c:
> > https://elixir.bootlin.com/u-boot/latest/source/drivers/spi/spi-mem.c#L376
> >
> > And the post the full log here. If the log is too big please paste it to pastebin.ubuntu.com
> >
> > Also please apply http://patchwork.ozlabs.org/patch/1090121/ on top of latest tree before doing any write to flash
> >
> > Sorry for the trouble.
> >
>
> Sorry for the long delay. I finally got time to look into this.
>
> However with the latest u-boot/master, the SPI flash seems to work
> again on Intel MinnowMax. Did you recall anything changes in the
> spi-nor recently?

The thing I know is:

v2019.07-rc1: Intel ich-spi driver was broken
v2019.07-rc2: Intel ich-spi driver worked again

So I did a 'git bisect', although I used it in a reverse direction :)

This is the commit that makes the Intel ich-spi driver work again.

commit 60e2bf46784ebbd30ff29b3d3c7c97e56b11e86a
Author: Weijie Gao <weijie.gao@mediatek.com>
Date:   Fri Apr 26 17:22:19 2019 +0800

    mtd: spi-nor: fix page program issue when using spi-mem driver

    Some SPI controllers can't write nor->page_size bytes in a single step
    because their TX FIFO is too small, but when that happens we should
    make sure a WRITE_EN command before each write access and READ_SR command
    after each write access is issued.

    We should allow nor->write() to return a size that is smaller than the
    requested write size to gracefully handle this case.

    Also, the spi_nor_write_data() should return the actual number of bytes
    that were written during the spi_mem_exec_op() operation.

    This patch is a combination of two commits backported from kernel:

      commit 630d6bd8a3b4 ("mtd: spi-nor: Support controllers with limit ...")
      commit 3baa8ec88c2f ("mtd: devices: m25p80: Make sure WRITE_EN is ...")

    Cc: Vignesh R <vigneshr@ti.com>
    Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
    Acked-by: Vignesh R <vigneshr@ti.com>
    Tested-by: Shyam Saini <shyam.saini@amarulasolutions.com> # microzed
    Acked-by: Jagan Teki <jagan@amarulasolutions.com>

Hi Simon,

Could you please confirm that on the Chromebook?

Regards,
Bin

[U-Boot] x86: SPI flash broken with SPI NOR

[Simon Glass]

Hi Bin,

On Wed, 5 Jun 2019 at 08:47, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Vignesh,
>
> On Wed, Jun 5, 2019 at 9:40 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Vignesh,
> >
> > On Sat, Apr 27, 2019 at 12:26 AM Vignesh Raghavendra <vigneshr@ti.com> wrote:
> > >
> > > Bin,
> > >
> > > On 26/04/19 7:33 PM, Bin Meng wrote:
> > > > Hi Simon,
> > > >
> > > > On Fri, Apr 26, 2019 at 7:26 AM Simon Glass <sjg@chromium.org> wrote:
> > > >>
> > > >> Hi Bin,
> > > >>
> > > >> I find that 'sf test 0 1000' does not work anymore since this commit:
> > > >>
> > > >> c4e8862308 mtd: spi: Switch to new SPI NOR framework
> > > >>
> > > >> Have you seen any problems? This seems to prevent proper SPI flash writing (not sure about reading).
> > > >
> > > > Yes, this seems to be broken. I just tested it on MinnowMax.
> > > >
> > >
> > > What is the flash part on this board?
> > >
> > >
> > > > 'sf probe' does not succeed anymore.
> > > >
> > >
> > > Could you enable all debug prints in spi_mem_exec_op() in spi-mem.c:
> > > https://elixir.bootlin.com/u-boot/latest/source/drivers/spi/spi-mem.c#L376
> > >
> > > And the post the full log here. If the log is too big please paste it to pastebin.ubuntu.com
> > >
> > > Also please apply http://patchwork.ozlabs.org/patch/1090121/ on top of latest tree before doing any write to flash
> > >
> > > Sorry for the trouble.
> > >
> >
> > Sorry for the long delay. I finally got time to look into this.
> >
> > However with the latest u-boot/master, the SPI flash seems to work
> > again on Intel MinnowMax. Did you recall anything changes in the
> > spi-nor recently?
>
> The thing I know is:
>
> v2019.07-rc1: Intel ich-spi driver was broken
> v2019.07-rc2: Intel ich-spi driver worked again
>
> So I did a 'git bisect', although I used it in a reverse direction :)
>
> This is the commit that makes the Intel ich-spi driver work again.
>
> commit 60e2bf46784ebbd30ff29b3d3c7c97e56b11e86a
> Author: Weijie Gao <weijie.gao@mediatek.com>
> Date:   Fri Apr 26 17:22:19 2019 +0800
>
>     mtd: spi-nor: fix page program issue when using spi-mem driver
>
>     Some SPI controllers can't write nor->page_size bytes in a single step
>     because their TX FIFO is too small, but when that happens we should
>     make sure a WRITE_EN command before each write access and READ_SR command
>     after each write access is issued.
>
>     We should allow nor->write() to return a size that is smaller than the
>     requested write size to gracefully handle this case.
>
>     Also, the spi_nor_write_data() should return the actual number of bytes
>     that were written during the spi_mem_exec_op() operation.
>
>     This patch is a combination of two commits backported from kernel:
>
>       commit 630d6bd8a3b4 ("mtd: spi-nor: Support controllers with limit ...")
>       commit 3baa8ec88c2f ("mtd: devices: m25p80: Make sure WRITE_EN is ...")
>
>     Cc: Vignesh R <vigneshr@ti.com>
>     Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
>     Acked-by: Vignesh R <vigneshr@ti.com>
>     Tested-by: Shyam Saini <shyam.saini@amarulasolutions.com> # microzed
>     Acked-by: Jagan Teki <jagan@amarulasolutions.com>
>
> Hi Simon,
>
> Could you please confirm that on the Chromebook?

I was broken when I last tested a few weeks ago. I'll try again.

Regards,
Simon

[U-Boot] x86: SPI flash broken with SPI NOR

[Vignesh Raghavendra]

On 05/06/19 8:17 PM, Bin Meng wrote:
> Hi Vignesh,
> 
> On Wed, Jun 5, 2019 at 9:40 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> Hi Vignesh,
>>
>> On Sat, Apr 27, 2019 at 12:26 AM Vignesh Raghavendra <vigneshr@ti.com> wrote:
>>>
>>> Bin,
>>>
>>> On 26/04/19 7:33 PM, Bin Meng wrote:
>>>> Hi Simon,
>>>>
>>>> On Fri, Apr 26, 2019 at 7:26 AM Simon Glass <sjg@chromium.org> wrote:
>>>>>
>>>>> Hi Bin,
>>>>>
>>>>> I find that 'sf test 0 1000' does not work anymore since this commit:
>>>>>
>>>>> c4e8862308 mtd: spi: Switch to new SPI NOR framework
>>>>>
>>>>> Have you seen any problems? This seems to prevent proper SPI flash writing (not sure about reading).
>>>>
>>>> Yes, this seems to be broken. I just tested it on MinnowMax.
>>>>
>>>
>>> What is the flash part on this board?
>>>
>>>
>>>> 'sf probe' does not succeed anymore.
>>>>
>>>
>>> Could you enable all debug prints in spi_mem_exec_op() in spi-mem.c:
>>> https://elixir.bootlin.com/u-boot/latest/source/drivers/spi/spi-mem.c#L376
>>>
>>> And the post the full log here. If the log is too big please paste it to pastebin.ubuntu.com
>>>
>>> Also please apply http://patchwork.ozlabs.org/patch/1090121/ on top of latest tree before doing any write to flash
>>>
>>> Sorry for the trouble.
>>>
>>
>> Sorry for the long delay. I finally got time to look into this.
>>
>> However with the latest u-boot/master, the SPI flash seems to work
>> again on Intel MinnowMax. Did you recall anything changes in the
>> spi-nor recently?
> 
> The thing I know is:
> 
> v2019.07-rc1: Intel ich-spi driver was broken
> v2019.07-rc2: Intel ich-spi driver worked again
> 
> So I did a 'git bisect', although I used it in a reverse direction :)
> 
> This is the commit that makes the Intel ich-spi driver work again.


That's great to know! There is a patch on ML to convert ich-spi to
spi-mem which should make driver much simpler[1]. It would be great to
have that patch tested and merged.

[]1https://patchwork.ozlabs.org/patch/1072475/

Regards
Vignesh
> 
> commit 60e2bf46784ebbd30ff29b3d3c7c97e56b11e86a
> Author: Weijie Gao <weijie.gao@mediatek.com>
> Date:   Fri Apr 26 17:22:19 2019 +0800
> 
>     mtd: spi-nor: fix page program issue when using spi-mem driver
> 
>     Some SPI controllers can't write nor->page_size bytes in a single step
>     because their TX FIFO is too small, but when that happens we should
>     make sure a WRITE_EN command before each write access and READ_SR command
>     after each write access is issued.
> 
>     We should allow nor->write() to return a size that is smaller than the
>     requested write size to gracefully handle this case.
> 
>     Also, the spi_nor_write_data() should return the actual number of bytes
>     that were written during the spi_mem_exec_op() operation.
> 
>     This patch is a combination of two commits backported from kernel:
> 
>       commit 630d6bd8a3b4 ("mtd: spi-nor: Support controllers with limit ...")
>       commit 3baa8ec88c2f ("mtd: devices: m25p80: Make sure WRITE_EN is ...")
> 
>     Cc: Vignesh R <vigneshr@ti.com>
>     Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
>     Acked-by: Vignesh R <vigneshr@ti.com>
>     Tested-by: Shyam Saini <shyam.saini@amarulasolutions.com> # microzed
>     Acked-by: Jagan Teki <jagan@amarulasolutions.com>
> 
> Hi Simon,
> 
> Could you please confirm that on the Chromebook?
> 
> Regards,
> Bin
> 

-- 
Regards
Vignesh