[PATCH v2 0/9] 5p80: Add SFDP support

Cédric Le Goater posted 9 patches 5 years, 2 months ago
Failed in applying to current master (apply log)
hw/block/m25p80_sfdp.h      |  27 ++++
include/hw/ssi/aspeed_smc.h |   1 +
hw/arm/aspeed.c             |   6 +-
hw/block/m25p80.c           |  47 +++++-
hw/block/m25p80_sfdp.c      | 296 ++++++++++++++++++++++++++++++++++++
hw/ssi/aspeed_smc.c         |  21 ++-
MAINTAINERS                 |   2 +-
hw/block/meson.build        |   1 +
hw/block/trace-events       |   1 +
9 files changed, 385 insertions(+), 17 deletions(-)
create mode 100644 hw/block/m25p80_sfdp.h
create mode 100644 hw/block/m25p80_sfdp.c
[PATCH v2 0/9] 5p80: Add SFDP support
Posted by Cédric Le Goater 5 years, 2 months ago
Hello,

JEDEC STANDARD JESD216 for Serial Flash Discovery Parameters (SFDP)
provides a mean to describe the features of a serial flash device
using a set of internal parameter tables. Support in Linux has been
added some time ago and the spi-nor driver is using it more often
to detect the flash settings and even flash models.

This is the initial framework for the RDSFDP command giving access to
a private SFDP area under the flash.

The patches available here :

  https://github.com/legoater/qemu/commits/aspeed-5.2

Thanks,

C.

Changes in v2:

 - fixed dummy value
 - replaced  'sfdp' table by a 'sfdp_read' handler
 - more SFDP tables
 - fixed Aspeed SMC support
 - introduced mx25l25635f chip model for test under Linux.

Cédric Le Goater (9):
  m25p80: Add basic support for the SFDP command
  m25p80: Add the n25q256a SFDP table
  m25p80: Add the mx25l25635e SFPD table
  m25p80: Add the mx25l25635f SFPD table
  m25p80: Add the mx66l1g45g SFDP table
  m25p80: Add the w25q256 SFPD table
  m25p80: Add the w25q512jv SFPD table
  arm/aspeed: Replace mx25l25635e chip model
  aspeed/smc: Add support for RDSFDP command

 hw/block/m25p80_sfdp.h      |  27 ++++
 include/hw/ssi/aspeed_smc.h |   1 +
 hw/arm/aspeed.c             |   6 +-
 hw/block/m25p80.c           |  47 +++++-
 hw/block/m25p80_sfdp.c      | 296 ++++++++++++++++++++++++++++++++++++
 hw/ssi/aspeed_smc.c         |  21 ++-
 MAINTAINERS                 |   2 +-
 hw/block/meson.build        |   1 +
 hw/block/trace-events       |   1 +
 9 files changed, 385 insertions(+), 17 deletions(-)
 create mode 100644 hw/block/m25p80_sfdp.h
 create mode 100644 hw/block/m25p80_sfdp.c

-- 
2.25.4


Re: [PATCH v2 0/9] 5p80: Add SFDP support
Posted by Joel Stanley 5 years, 1 month ago
On Wed, 2 Sep 2020 at 09:31, Cédric Le Goater <clg@kaod.org> wrote:
>
> Hello,
>
> JEDEC STANDARD JESD216 for Serial Flash Discovery Parameters (SFDP)
> provides a mean to describe the features of a serial flash device
> using a set of internal parameter tables. Support in Linux has been
> added some time ago and the spi-nor driver is using it more often
> to detect the flash settings and even flash models.
>
> This is the initial framework for the RDSFDP command giving access to
> a private SFDP area under the flash.
>
> The patches available here :
>
>   https://github.com/legoater/qemu/commits/aspeed-5.2

Reviewed-by: Joel Stanley <joel@jms.id.au>
Tested-by: Joel Stanley <joel@jms.id.au>

Note that these need to be rebased on master; there are some minor conflicts.

These patches have proved essential in debugging a recent kernel
regression. Thanks for adding this support Cédric.

Cheers,

Joel

>
> Thanks,
>
> C.
>
> Changes in v2:
>
>  - fixed dummy value
>  - replaced  'sfdp' table by a 'sfdp_read' handler
>  - more SFDP tables
>  - fixed Aspeed SMC support
>  - introduced mx25l25635f chip model for test under Linux.
>
> Cédric Le Goater (9):
>   m25p80: Add basic support for the SFDP command
>   m25p80: Add the n25q256a SFDP table
>   m25p80: Add the mx25l25635e SFPD table
>   m25p80: Add the mx25l25635f SFPD table
>   m25p80: Add the mx66l1g45g SFDP table
>   m25p80: Add the w25q256 SFPD table
>   m25p80: Add the w25q512jv SFPD table
>   arm/aspeed: Replace mx25l25635e chip model
>   aspeed/smc: Add support for RDSFDP command
>
>  hw/block/m25p80_sfdp.h      |  27 ++++
>  include/hw/ssi/aspeed_smc.h |   1 +
>  hw/arm/aspeed.c             |   6 +-
>  hw/block/m25p80.c           |  47 +++++-
>  hw/block/m25p80_sfdp.c      | 296 ++++++++++++++++++++++++++++++++++++
>  hw/ssi/aspeed_smc.c         |  21 ++-
>  MAINTAINERS                 |   2 +-
>  hw/block/meson.build        |   1 +
>  hw/block/trace-events       |   1 +
>  9 files changed, 385 insertions(+), 17 deletions(-)
>  create mode 100644 hw/block/m25p80_sfdp.h
>  create mode 100644 hw/block/m25p80_sfdp.c
>
> --
> 2.25.4
>

Re: [PATCH v2 0/9] 5p80: Add SFDP support
Posted by Joel Stanley 5 years, 1 month ago
On Wed, 7 Oct 2020 at 01:43, Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, 2 Sep 2020 at 09:31, Cédric Le Goater <clg@kaod.org> wrote:
> >
> > Hello,
> >
> > JEDEC STANDARD JESD216 for Serial Flash Discovery Parameters (SFDP)
> > provides a mean to describe the features of a serial flash device
> > using a set of internal parameter tables. Support in Linux has been
> > added some time ago and the spi-nor driver is using it more often
> > to detect the flash settings and even flash models.
> >
> > This is the initial framework for the RDSFDP command giving access to
> > a private SFDP area under the flash.
> >
> > The patches available here :
> >
> >   https://github.com/legoater/qemu/commits/aspeed-5.2
>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Tested-by: Joel Stanley <joel@jms.id.au>
>
> Note that these need to be rebased on master; there are some minor conflicts.
>
> These patches have proved essential in debugging a recent kernel
> regression. Thanks for adding this support Cédric.

For reference, an OpenBMC UBI based NOR image with an upstream (5.8,
5.9-rc) kernel can be used to reproduce the bug as follows:

qemu-system-arm -M witherspoon-bmc -nic user -nographic \
 -drive file=obmc-phosphor-image-witherspoon.ubi.mtd,format=raw,if=mtd \
 -kernel aspeed-g5-dev/arch/arm/boot/zImage \
 -dtb aspeed-g5-dev/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dtb \
 -append "console=ttyS4,115200n8 ubi.mtd=obmc-ubi,0,0,0
ubi.mtd=alt-obmc-ubi,0,0,4 ro rootfstype=squashfs ubi.block=0,1
root=/dev/ubiblock0_1"

The ubi.mtd can be found here[1].

This assumes you're running qemu with this series applied, and an
upstream kernel built with aspeed_g5_defconfig:

git checkout v5.8
CROSS_COMPILE="ccache arm-linux-gnueabi-" ARCH=arm make -j8
O=aspeed-g5-dev aspeed_g5_defconfig
CROSS_COMPILE="ccache arm-linux-gnueabi-" ARCH=arm make -j8 O=aspeed-g5-dev

The fix for the Linux kernel issue is on the linux-mtd list[2].
WIthout the fix, the kernel will fail to attach the UBI volume:

[    1.453224] ubi0: default fastmap pool size: 25
[    1.453370] ubi0: default fastmap WL pool size: 12
[    1.453505] ubi0: attaching mtd3
[    1.467316] ubi0 error: ubi_compare_lebs: unsupported on-flash UBI format
[    1.467902] ubi0 error: ubi_attach_mtd_dev: failed to attach mtd3, error -22

With the fix, the system will boot through to userspace.

Cheers,

Joel

[1] https://jenkins.openbmc.org/view/latest/job/latest-master/label=docker-builder,target=witherspoon/lastSuccessfulBuild/artifact/openbmc/build/tmp/deploy/images/witherspoon/obmc-phosphor-image-witherspoon.ubi.mtd
[2] https://lore.kernel.org/linux-mtd/CACPK8XceL_QHCQOhfus27rei0vwfRJAFjfL6JkVw9pwxJj2d6Q@mail.gmail.com/



>
> Cheers,
>
> Joel
>
> >
> > Thanks,
> >
> > C.
> >
> > Changes in v2:
> >
> >  - fixed dummy value
> >  - replaced  'sfdp' table by a 'sfdp_read' handler
> >  - more SFDP tables
> >  - fixed Aspeed SMC support
> >  - introduced mx25l25635f chip model for test under Linux.
> >
> > Cédric Le Goater (9):
> >   m25p80: Add basic support for the SFDP command
> >   m25p80: Add the n25q256a SFDP table
> >   m25p80: Add the mx25l25635e SFPD table
> >   m25p80: Add the mx25l25635f SFPD table
> >   m25p80: Add the mx66l1g45g SFDP table
> >   m25p80: Add the w25q256 SFPD table
> >   m25p80: Add the w25q512jv SFPD table
> >   arm/aspeed: Replace mx25l25635e chip model
> >   aspeed/smc: Add support for RDSFDP command
> >
> >  hw/block/m25p80_sfdp.h      |  27 ++++
> >  include/hw/ssi/aspeed_smc.h |   1 +
> >  hw/arm/aspeed.c             |   6 +-
> >  hw/block/m25p80.c           |  47 +++++-
> >  hw/block/m25p80_sfdp.c      | 296 ++++++++++++++++++++++++++++++++++++
> >  hw/ssi/aspeed_smc.c         |  21 ++-
> >  MAINTAINERS                 |   2 +-
> >  hw/block/meson.build        |   1 +
> >  hw/block/trace-events       |   1 +
> >  9 files changed, 385 insertions(+), 17 deletions(-)
> >  create mode 100644 hw/block/m25p80_sfdp.h
> >  create mode 100644 hw/block/m25p80_sfdp.c
> >
> > --
> > 2.25.4
> >

Re: [PATCH v2 0/9] 5p80: Add SFDP support
Posted by Cédric Le Goater 5 years, 1 month ago
On 10/7/20 3:43 AM, Joel Stanley wrote:
> On Wed, 2 Sep 2020 at 09:31, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> Hello,
>>
>> JEDEC STANDARD JESD216 for Serial Flash Discovery Parameters (SFDP)
>> provides a mean to describe the features of a serial flash device
>> using a set of internal parameter tables. Support in Linux has been
>> added some time ago and the spi-nor driver is using it more often
>> to detect the flash settings and even flash models.
>>
>> This is the initial framework for the RDSFDP command giving access to
>> a private SFDP area under the flash.
>>
>> The patches available here :
>>
>>   https://github.com/legoater/qemu/commits/aspeed-5.2
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Tested-by: Joel Stanley <joel@jms.id.au>
> 
> Note that these need to be rebased on master; there are some minor conflicts.
> 
> These patches have proved essential in debugging a recent kernel
> regression. Thanks for adding this support Cédric.

We have been discussing offline with Francisco of a more subtle approach 
to reduce the size of the definitions of the SFDP tables. I agree that 
the current approach is brutal (and efficient :) but I haven't had time 
to take a close look at his proposal. See below.

    typedef struct SFDPSection {
        const uint32_t addr;
        const uint32_t size;
        const uint32_t wrap_sz;
        const uint8_t *data;
    } SFDPSection;
    
    #define SFDP_RAW(start_addr, vals...) \
    {                                     \
      .addr = start_addr,                 \
      .size = sizeof((uint8_t[]){vals}),  \
      .data = (const uint8_t[]){vals}     \
    }
    
    #define SFDP_RAW_WRAP(start_addr, _wrap_sz, vals...) \
    {                                     \
      .addr = start_addr,                 \
      .size = sizeof((uint8_t[]){vals}),  \
      .wrap_sz = _wrap_sz,                \
      .data = (const uint8_t[]){vals}     \
    }
    
    #define SFDP_TABLE_END() { 0 }
    #define IS_SFDP_END(x) (x.size == 0)
    
    #define M35T4545_WRAP_SZ 0x100
    
    static const SFDPTable m35t4545 = {
        SFDP_RAW_WRAP(0, M35T4545_WRAP_SZ,
                      0x53, 0x46, 0x44, 0x50, 0x00, 0x01, 0x00, 0xff,
                      0x00, 0x00, 0x01, 0x09, 0x30, 0x00, 0x00, 0xff),
    
        SFDP_RAW(0x38,
                 0xe5, 0x20, 0xfb, 0xff, 0xff, 0xff, 0xff, 0x0f,
                 0x29, 0xeb, 0x27, 0x6b, 0x08, 0x3b, 0x27, 0xbb,
                 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x27, 0xbb,
                 0xff, 0xff, 0x29, 0xeb, 0x0c, 0x20, 0x10, 0xd8,
                 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff),
    
        SFDP_TABLE_END()
    };
    
    uint8_t m25p80_sfdp_read(SFDPTable t, uint32_t addr)
    {
        if (t[0].wrap_sz) {
            addr &= (t.wrap_sz-1);
        }
    
        for (int i = 0; !IS_SFDP_END(t[i]); i++) {
            if (addr >= t[i].addr && addr < (t[i].addr + t[i].size)) {
                return t[i].data[addr];
            }
        }
        return 0xFF;
    }


The SFDP header (SFDP_RAW_WRAP) contains the list of the SFDP tables, 
the first being the BFPT at offset 0x30. It would be nice to be able 
to build the list in the header from the different table definitions.

C.