| Message ID | 20210727045222.905056-36-tudor.ambarus@microchip.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | mtd: spi-nor: Handle ID collisions and clean params init | expand |
Am 2021-07-27 06:52, schrieb Tudor Ambarus: > Add some guideliness on how to propose a new flash addition. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > Documentation/driver-api/mtd/spi-nor.rst | 65 ++++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > > diff --git a/Documentation/driver-api/mtd/spi-nor.rst > b/Documentation/driver-api/mtd/spi-nor.rst > index 4a3adca417fd..ffb8d97a2766 100644 > --- a/Documentation/driver-api/mtd/spi-nor.rst > +++ b/Documentation/driver-api/mtd/spi-nor.rst [..] > +Every new flash addition that define the SFDP tables, should hexdump > its SFDP > +tables in the patch's comment section below the --- line, so that we > can > +reference it in case of ID collisions. Nice, but could you add some guidelines how to do it? That is the exact commands, maybe with a notice one should use these whenever possible. I want to prevent having all sorts of variations of the output and I want to be able to reverse the operation and verify it. # xxd -p /path/to/sfdp # md5sum /path/to/sfdp # cat /path/to/jedec_id # cat /path/to/partname # cat /path/to/manufacturer -michael
On 7/27/21 10:22 AM, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2021-07-27 06:52, schrieb Tudor Ambarus: >> Add some guideliness on how to propose a new flash addition. >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> --- >> Documentation/driver-api/mtd/spi-nor.rst | 65 ++++++++++++++++++++++++ >> 1 file changed, 65 insertions(+) >> >> diff --git a/Documentation/driver-api/mtd/spi-nor.rst >> b/Documentation/driver-api/mtd/spi-nor.rst >> index 4a3adca417fd..ffb8d97a2766 100644 >> --- a/Documentation/driver-api/mtd/spi-nor.rst >> +++ b/Documentation/driver-api/mtd/spi-nor.rst > [..] >> +Every new flash addition that define the SFDP tables, should hexdump >> its SFDP >> +tables in the patch's comment section below the --- line, so that we >> can >> +reference it in case of ID collisions. > > Nice, but could you add some guidelines how to do it? That is the exact > commands, maybe with a notice one should use these whenever possible. I > want to prevent having all sorts of variations of the output and I want > to be able to reverse the operation and verify it. ok, will do > > # xxd -p /path/to/sfdp > # md5sum /path/to/sfdp maybe sha1sum here? > # cat /path/to/jedec_id > # cat /path/to/partname > # cat /path/to/manufacturer > Will add some short examples of mtd_debug and the erase, verify, write, read back and compare too. Do you have some locking test examples? I'll have to check those too.
Am 2021-07-27 10:09, schrieb Tudor.Ambarus@microchip.com: > On 7/27/21 10:22 AM, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> Am 2021-07-27 06:52, schrieb Tudor Ambarus: >>> Add some guideliness on how to propose a new flash addition. >>> >>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >>> --- >>> Documentation/driver-api/mtd/spi-nor.rst | 65 >>> ++++++++++++++++++++++++ >>> 1 file changed, 65 insertions(+) >>> >>> diff --git a/Documentation/driver-api/mtd/spi-nor.rst >>> b/Documentation/driver-api/mtd/spi-nor.rst >>> index 4a3adca417fd..ffb8d97a2766 100644 >>> --- a/Documentation/driver-api/mtd/spi-nor.rst >>> +++ b/Documentation/driver-api/mtd/spi-nor.rst >> [..] >>> +Every new flash addition that define the SFDP tables, should hexdump >>> its SFDP >>> +tables in the patch's comment section below the --- line, so that we >>> can >>> +reference it in case of ID collisions. >> >> Nice, but could you add some guidelines how to do it? That is the >> exact >> commands, maybe with a notice one should use these whenever possible. >> I >> want to prevent having all sorts of variations of the output and I >> want >> to be able to reverse the operation and verify it. > > ok, will do > >> >> # xxd -p /path/to/sfdp >> # md5sum /path/to/sfdp > > maybe sha1sum here? sure, that one doesn't really matter. any *sum will work. >> # cat /path/to/jedec_id >> # cat /path/to/partname >> # cat /path/to/manufacturer >> > > Will add some short examples of mtd_debug and the erase, verify, > write, read back > and compare too. > > Do you have some locking test examples? I'll have to check those too. Not really, I usually check the locking by looking at the BP bits, but there is no easy method to look at them in linux. -michael
On 27/07/21 09:22AM, Michael Walle wrote: > Am 2021-07-27 06:52, schrieb Tudor Ambarus: > > Add some guideliness on how to propose a new flash addition. > > > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > > --- > > Documentation/driver-api/mtd/spi-nor.rst | 65 ++++++++++++++++++++++++ > > 1 file changed, 65 insertions(+) > > > > diff --git a/Documentation/driver-api/mtd/spi-nor.rst > > b/Documentation/driver-api/mtd/spi-nor.rst > > index 4a3adca417fd..ffb8d97a2766 100644 > > --- a/Documentation/driver-api/mtd/spi-nor.rst > > +++ b/Documentation/driver-api/mtd/spi-nor.rst > [..] > > +Every new flash addition that define the SFDP tables, should hexdump > > its SFDP > > +tables in the patch's comment section below the --- line, so that we > > can > > +reference it in case of ID collisions. > > Nice, but could you add some guidelines how to do it? That is the exact > commands, maybe with a notice one should use these whenever possible. I > want to prevent having all sorts of variations of the output and I want > to be able to reverse the operation and verify it. > > # xxd -p /path/to/sfdp > # md5sum /path/to/sfdp > # cat /path/to/jedec_id > # cat /path/to/partname > # cat /path/to/manufacturer It would be nice if we could have checkpatch.pl check for these. But I don't know how difficult that would be to add.
diff --git a/Documentation/driver-api/mtd/spi-nor.rst b/Documentation/driver-api/mtd/spi-nor.rst index 4a3adca417fd..ffb8d97a2766 100644 --- a/Documentation/driver-api/mtd/spi-nor.rst +++ b/Documentation/driver-api/mtd/spi-nor.rst @@ -66,3 +66,68 @@ when you want to write a new driver for a SPI NOR controller. Another API is spi_nor_restore(), this is used to restore the status of SPI flash chip such as addressing mode. Call it whenever detach the driver from device or reboot the system. + +Part IV - How to propose a new flash addition? +---------------------------------------------- + +First we have to clarify where the new flash_info entry will reside. Typically +each manufacturer have their own driver and the new flash will be placed in that +specific manufacturer driver. There are cases however, where special care has to +be taken. In case of flash ID collisions between different manufacturers, the +place to add the new flash is in the manuf-id-collisions.c driver. ID collisions +between flashes of the same manufacturer should be handled in their own +manufacturer driver, macronix being an example. There will be a single +flash_info entry for all the ID collisions of the same ID. + +manuf-id-collisions.c is the place to add new flash additions where the +manufacturer is ignorant enough to not implement the ID continuation scheme +that is described in the JEP106 JEDEC Standard. One has to dump its flash ID and +compare it with the flash's manufacturer identification code that is defined in +the JEP106 JEDEC Standard. If the manufacturer ID is defined in bank two or +higher and the manufacturer does not implement the ID continuation scheme, then +it is likely that the flash ID will collide with a manufacturer from bank one or +with other manufacturer from other bank that does not implement the ID +continuation scheme as well. + +flash_info entries will be added in a first come, first served manner. If there +are ID collisions, differentiation between flashes will be done at runtime if +possible. Where runtime differentiation is not possible, new compatibles will be +introduced, but this will be done as a last resort. + +New flash additions that support the SFDP standard should be declared using +SPI_NOR_PARSE_SFDP. Support that can be discovered when parsing SFDP should not +be duplicated by explicit flags at flash declaration. All the SFDP flash +parameters and settings will be discovered when parsing SFDP. There are +flash_info flags that indicate support that is not SFDP discoverable. These +flags initialize non SFDP support in the spi_nor_nonsfdp_flags_init() method. +SPI_NOR_PARSE_SFDP is usually followed by other flash_info flags from the +aforementioned function. Sometimes manufacturers wrongly define some fields in +the SFDP tables. If that's the case, SFDP data can be amended with the fixups() +hooks. It is not common, but if the SFDP tables are entirely wrong, and it does +not worth the hassle to tweak the SFDP parameters by using the fixups hooks, or +if the flash does not define the SFDP tables at all, then one can statically +init the flash with the SPI_NOR_SKIP_SFDP flag and specify the rest of the flash +capabilities with the flash info flags. + +With time we want to convert all flashes to either use SPI_NOR_PARSE_SFDP or +SPI_NOR_SKIP_SFDP and stop triggering the SFDP parsing with the +SPI_NOR_{DUAL, QUAD, OCTAL*}_READ flags. There are flashes that support QUAD +mode but do not support the RDSFDP command, we should avoid issuing unsupported +commands to flashes where possible. It is unlikely that RDSFDP will cause any +problems, but still, it's better to avoid it. There are cases however of flash +ID collisions between flashes that define the SFDP tables and flashes that don't +(again, macronix). We usually differentiate between the two by issuing the +RDSFDP command. In such a case one has to declare the SPI_NOR_PARSE_SFDP +together with all the relevant flags from spi_nor_nonsfdp_flags_init() for the +SFDP compatible flash, but should also declare the relevant flags that are used +in the spi_nor_info_init_params() method in order to init support that can't be +discovered via SFDP for the non-SFDP compatible flash. + +Every new flash addition that define the SFDP tables, should hexdump its SFDP +tables in the patch's comment section below the --- line, so that we can +reference it in case of ID collisions. + +Every flash_info flag declared should be tested. Typically one uses the +mtd-utils and does an erase, verify erase, write, read back and compare test. +Locking and other flags that are declared in the flash_info entry and used in +the spi_nor_nonsfdp_flags_init() should be tested as well.
Add some guideliness on how to propose a new flash addition. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- Documentation/driver-api/mtd/spi-nor.rst | 65 ++++++++++++++++++++++++ 1 file changed, 65 insertions(+)