| Message ID | 20181017115332.22312-1-rajat.srivastava@nxp.com |
|---|---|
| Headers | show |
| Series | Read SFDP parameters and access flash above 16MB | expand |
On 17.10.2018 13:27, Rajat Srivastava wrote: > Add functionality to read and parse SFDP parameters to auto-detect > flash size, page size and address width of flash. This enables > flash access above 16MB using 4-byte addressing mode. > > Add driver support to get SFDP information of flash and use it to > access flash above 16MB. > > Enable reading and parsing of SFDP parameters for Spansion's > s25fs512 flash to auto-detect its size, its page size and the > addressing mode it supports. Why do you need driver-specific code to read the sfdp parameters? Wouldn't it be much cleaner to solve this from spi_flash.c only? If so, just adding the 'spi_flash_parse_sfdp()' function on top of Stefan's patch should work? I tried testing this patch, but just like for Stefan, but it just didn't work for me and I failed to see some simple steps to adjust my spi driver to make it work. Simon > > Rajat Srivastava (3): > mtd/spi: Add JEDEC SFDP support in SPI framework > fsl_qspi: Access flash above 16MB using SFDP > spi_flash_ids: Enable SFDP parsing for s25fs512 flash > > drivers/mtd/spi/sf_internal.h | 4 + > drivers/mtd/spi/spi_flash.c | 297 +++++++++++++++++++++++++++++++++++++--- > drivers/mtd/spi/spi_flash_ids.c | 3 +- > drivers/spi/fsl_qspi.c | 103 +++++++++++++- > include/spi.h | 2 + > include/spi_flash.h | 120 ++++++++++++++++ > 6 files changed, 501 insertions(+), 28 deletions(-) >
On Tue, Oct 30, 2018 at 9:35 PM Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote: > > On 17.10.2018 13:27, Rajat Srivastava wrote: > > Add functionality to read and parse SFDP parameters to auto-detect > > flash size, page size and address width of flash. This enables > > flash access above 16MB using 4-byte addressing mode. > > > > Add driver support to get SFDP information of flash and use it to > > access flash above 16MB. > > > > Enable reading and parsing of SFDP parameters for Spansion's > > s25fs512 flash to auto-detect its size, its page size and the > > addressing mode it supports. > > Why do you need driver-specific code to read the sfdp parameters? > Wouldn't it be much cleaner to solve this from spi_flash.c only? > > If so, just adding the 'spi_flash_parse_sfdp()' function on top of > Stefan's patch should work? > > I tried testing this patch, but just like for Stefan, but it just didn't > work for me and I failed to see some simple steps to adjust my spi > driver to make it work. And taking this further, would it make sense to add a config option that dumps the whole flash chip table and only uses SFDP? Then we could save a lot of space in SPL for boards where reading SFDP is enough. Simon > > Simon > > > > > Rajat Srivastava (3): > > mtd/spi: Add JEDEC SFDP support in SPI framework > > fsl_qspi: Access flash above 16MB using SFDP > > spi_flash_ids: Enable SFDP parsing for s25fs512 flash > > > > drivers/mtd/spi/sf_internal.h | 4 + > > drivers/mtd/spi/spi_flash.c | 297 +++++++++++++++++++++++++++++++++++++--- > > drivers/mtd/spi/spi_flash_ids.c | 3 +- > > drivers/spi/fsl_qspi.c | 103 +++++++++++++- > > include/spi.h | 2 + > > include/spi_flash.h | 120 ++++++++++++++++ > > 6 files changed, 501 insertions(+), 28 deletions(-) > > >
> -----Original Message----- > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Sent: Wednesday, October 31, 2018 2:06 AM > To: Rajat Srivastava <rajat.srivastava@nxp.com>; u-boot@lists.denx.de > Cc: jagan@openedev.com > Subject: Re: [U-Boot] [PATCH 0/3] Read SFDP parameters and access flash > above 16MB > > On 17.10.2018 13:27, Rajat Srivastava wrote: > > Add functionality to read and parse SFDP parameters to auto-detect > > flash size, page size and address width of flash. This enables > > flash access above 16MB using 4-byte addressing mode. > > > > Add driver support to get SFDP information of flash and use it to > > access flash above 16MB. > > > > Enable reading and parsing of SFDP parameters for Spansion's > > s25fs512 flash to auto-detect its size, its page size and the > > addressing mode it supports. > > Why do you need driver-specific code to read the sfdp parameters? > Wouldn't it be much cleaner to solve this from spi_flash.c only? That would have been cleaner only if it was possible. To read SFDP parameters of any flash, a READ_SFDP (0x5A) command needs to be sent to flash. The mtd framework does not allow sending any command to any flash directly from mtd layer. Any transaction initiated from mtd framework will call spi_xfer() function of the respective SPI driver which will further send the actual command to flash. Linux has also implemented reading SFDP parameters functionality in a similar way. > If so, just adding the 'spi_flash_parse_sfdp()' function on top of > Stefan's patch should work? > > I tried testing this patch, but just like for Stefan, but it just didn't > work for me and I failed to see some simple steps to adjust my spi > driver to make it work. You can see these patches for SFDP implementation in SPI driver: https://patchwork.ozlabs.org/patch/985329/ https://patchwork.ozlabs.org/patch/985328/ You, basically, need to send: - 0x5A command (READ_SFDP command) - 3-byte address (address will be provided by framework) - 8 dummy cycles > > Simon > > > > > Rajat Srivastava (3): > > mtd/spi: Add JEDEC SFDP support in SPI framework > > fsl_qspi: Access flash above 16MB using SFDP > > spi_flash_ids: Enable SFDP parsing for s25fs512 flash > > > > drivers/mtd/spi/sf_internal.h | 4 + > > drivers/mtd/spi/spi_flash.c | 297 > +++++++++++++++++++++++++++++++++++++--- > > drivers/mtd/spi/spi_flash_ids.c | 3 +- > > drivers/spi/fsl_qspi.c | 103 +++++++++++++- > > include/spi.h | 2 + > > include/spi_flash.h | 120 ++++++++++++++++ > > 6 files changed, 501 insertions(+), 28 deletions(-) > >
On Wed, Oct 31, 2018 at 9:41 AM Rajat Srivastava <rajat.srivastava@nxp.com> wrote: > > > > > -----Original Message----- > > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Sent: Wednesday, October 31, 2018 2:06 AM > > To: Rajat Srivastava <rajat.srivastava@nxp.com>; u-boot@lists.denx.de > > Cc: jagan@openedev.com > > Subject: Re: [U-Boot] [PATCH 0/3] Read SFDP parameters and access flash > > above 16MB > > > > On 17.10.2018 13:27, Rajat Srivastava wrote: > > > Add functionality to read and parse SFDP parameters to auto-detect > > > flash size, page size and address width of flash. This enables > > > flash access above 16MB using 4-byte addressing mode. > > > > > > Add driver support to get SFDP information of flash and use it to > > > access flash above 16MB. > > > > > > Enable reading and parsing of SFDP parameters for Spansion's > > > s25fs512 flash to auto-detect its size, its page size and the > > > addressing mode it supports. > > > > Why do you need driver-specific code to read the sfdp parameters? > > Wouldn't it be much cleaner to solve this from spi_flash.c only? > > That would have been cleaner only if it was possible. To read SFDP parameters of > any flash, a READ_SFDP (0x5A) command needs to be sent to flash. > > The mtd framework does not allow sending any command to any flash directly > from mtd layer. Any transaction initiated from mtd framework will call > spi_xfer() function of the respective SPI driver which will further send the actual > command to flash. > > Linux has also implemented reading SFDP parameters functionality in a > similar way. I don't know the Linux way. I only looked at the driver my platform uses (cadence_qspi) and I can't match this to your driver. The cadence driver more or less sends 'dout' as command, so using 'spi_flash_read_common()' in spi_flash.c should work. Is this different in your driver? > > If so, just adding the 'spi_flash_parse_sfdp()' function on top of > > Stefan's patch should work? > > > > I tried testing this patch, but just like for Stefan, but it just didn't > > work for me and I failed to see some simple steps to adjust my spi > > driver to make it work. > > You can see these patches for SFDP implementation in SPI driver: > https://patchwork.ozlabs.org/patch/985329/ > https://patchwork.ozlabs.org/patch/985328/ Well, of course I saw those patches. As written above, I had a hard time adapting your fsl_qspi changes to the cadence_qspi driver and I think it would be better if this wasn't needed. Simon > > You, basically, need to send: > - 0x5A command (READ_SFDP command) > - 3-byte address (address will be provided by framework) > - 8 dummy cycles > > > > > Simon > > > > > > > > Rajat Srivastava (3): > > > mtd/spi: Add JEDEC SFDP support in SPI framework > > > fsl_qspi: Access flash above 16MB using SFDP > > > spi_flash_ids: Enable SFDP parsing for s25fs512 flash > > > > > > drivers/mtd/spi/sf_internal.h | 4 + > > > drivers/mtd/spi/spi_flash.c | 297 > > +++++++++++++++++++++++++++++++++++++--- > > > drivers/mtd/spi/spi_flash_ids.c | 3 +- > > > drivers/spi/fsl_qspi.c | 103 +++++++++++++- > > > include/spi.h | 2 + > > > include/spi_flash.h | 120 ++++++++++++++++ > > > 6 files changed, 501 insertions(+), 28 deletions(-) > > > >
> -----Original Message----- > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Sent: Wednesday, October 31, 2018 1:09 PM > To: Rajat Srivastava <rajat.srivastava@nxp.com>; U-Boot Mailing List <u- > boot@lists.denx.de> > Cc: Jagan Teki <jagan@openedev.com> > Subject: Re: [U-Boot] [PATCH 0/3] Read SFDP parameters and access flash > above 16MB > > On Tue, Oct 30, 2018 at 9:35 PM Simon Goldschmidt > <simon.k.r.goldschmidt@gmail.com> wrote: > > > > On 17.10.2018 13:27, Rajat Srivastava wrote: > > > Add functionality to read and parse SFDP parameters to auto-detect > > > flash size, page size and address width of flash. This enables > > > flash access above 16MB using 4-byte addressing mode. > > > > > > Add driver support to get SFDP information of flash and use it to > > > access flash above 16MB. > > > > > > Enable reading and parsing of SFDP parameters for Spansion's > > > s25fs512 flash to auto-detect its size, its page size and the > > > addressing mode it supports. > > > > Why do you need driver-specific code to read the sfdp parameters? > > Wouldn't it be much cleaner to solve this from spi_flash.c only? > > > > If so, just adding the 'spi_flash_parse_sfdp()' function on top of > > Stefan's patch should work? > > > > I tried testing this patch, but just like for Stefan, but it just didn't > > work for me and I failed to see some simple steps to adjust my spi > > driver to make it work. > > And taking this further, would it make sense to add a config option > that dumps the whole flash chip table and only uses SFDP? Then we > could save a lot of space in SPL for boards where reading SFDP is > enough. Does the whole flash chip table mean the table that consists CFI information as well as SFDP information? If that's so, could you please elaborate where would we dump and save this table? And how will this help in saving space in SPL? Rajat > > Simon > > > > > Simon > > > > > > > > Rajat Srivastava (3): > > > mtd/spi: Add JEDEC SFDP support in SPI framework > > > fsl_qspi: Access flash above 16MB using SFDP > > > spi_flash_ids: Enable SFDP parsing for s25fs512 flash > > > > > > drivers/mtd/spi/sf_internal.h | 4 + > > > drivers/mtd/spi/spi_flash.c | 297 > +++++++++++++++++++++++++++++++++++++--- > > > drivers/mtd/spi/spi_flash_ids.c | 3 +- > > > drivers/spi/fsl_qspi.c | 103 +++++++++++++- > > > include/spi.h | 2 + > > > include/spi_flash.h | 120 ++++++++++++++++ > > > 6 files changed, 501 insertions(+), 28 deletions(-) > > > > >
On Wed, Oct 31, 2018 at 10:38 AM Rajat Srivastava <rajat.srivastava@nxp.com> wrote: > > > > > -----Original Message----- > > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Sent: Wednesday, October 31, 2018 1:09 PM > > To: Rajat Srivastava <rajat.srivastava@nxp.com>; U-Boot Mailing List <u- > > boot@lists.denx.de> > > Cc: Jagan Teki <jagan@openedev.com> > > Subject: Re: [U-Boot] [PATCH 0/3] Read SFDP parameters and access flash > > above 16MB > > > > On Tue, Oct 30, 2018 at 9:35 PM Simon Goldschmidt > > <simon.k.r.goldschmidt@gmail.com> wrote: > > > > > > On 17.10.2018 13:27, Rajat Srivastava wrote: > > > > Add functionality to read and parse SFDP parameters to auto-detect > > > > flash size, page size and address width of flash. This enables > > > > flash access above 16MB using 4-byte addressing mode. > > > > > > > > Add driver support to get SFDP information of flash and use it to > > > > access flash above 16MB. > > > > > > > > Enable reading and parsing of SFDP parameters for Spansion's > > > > s25fs512 flash to auto-detect its size, its page size and the > > > > addressing mode it supports. > > > > > > Why do you need driver-specific code to read the sfdp parameters? > > > Wouldn't it be much cleaner to solve this from spi_flash.c only? > > > > > > If so, just adding the 'spi_flash_parse_sfdp()' function on top of > > > Stefan's patch should work? > > > > > > I tried testing this patch, but just like for Stefan, but it just didn't > > > work for me and I failed to see some simple steps to adjust my spi > > > driver to make it work. > > > > And taking this further, would it make sense to add a config option > > that dumps the whole flash chip table and only uses SFDP? Then we > > could save a lot of space in SPL for boards where reading SFDP is > > enough. > > Does the whole flash chip table mean the table that consists CFI information > as well as SFDP information? If that's so, could you please elaborate where > would we dump and save this table? And how will this help in saving space in SPL? I meant the 'spi_flash_ids' table in spi_flash_ids.c. This table is > 1 kByte for my configuration and it contains strings that add up to roughly another kByte. So if we could remove this, we could save 2 kByte of unused rodata in SPL (for my SPL, rodata and rwdata are in the same RAM). I haven't looked into this in detail, so I don't know if this is really possible. Just from looking at the table, it seems SFDP should provide all the information contained therein? Simon > > Rajat > > > > > Simon > > > > > > > > Simon > > > > > > > > > > > Rajat Srivastava (3): > > > > mtd/spi: Add JEDEC SFDP support in SPI framework > > > > fsl_qspi: Access flash above 16MB using SFDP > > > > spi_flash_ids: Enable SFDP parsing for s25fs512 flash > > > > > > > > drivers/mtd/spi/sf_internal.h | 4 + > > > > drivers/mtd/spi/spi_flash.c | 297 > > +++++++++++++++++++++++++++++++++++++--- > > > > drivers/mtd/spi/spi_flash_ids.c | 3 +- > > > > drivers/spi/fsl_qspi.c | 103 +++++++++++++- > > > > include/spi.h | 2 + > > > > include/spi_flash.h | 120 ++++++++++++++++ > > > > 6 files changed, 501 insertions(+), 28 deletions(-) > > > > > > >