Re: [PATCH v2] dt-bindings: mtd: Add sst25vf016b to the list of supported chip names

From: Geert Uytterhoeven
Date: Mon Nov 20 2017 - 03:49:41 EST

Hi Cyrille,

On Fri, Nov 17, 2017 at 6:36 PM, Cyrille Pitchen
<cyrille.pitchen@xxxxxxxxxx> wrote:
> sorry but I won't apply this patch.
> New values for the 'compatible' DT properties should only be added for
> memory parts not supporting the JEDEC READ ID (0x9F) command.

I tent to disagree. Documenting part numbers in the DT bindings serves two
1. Documenting which parts are supported,
2. Allowing checkpatch to validate compatible values in DTS files (see

Unfortunately the current
is useless for the latter, as the values listed don't contain the vendor
prefixes, still causing warnings like

WARNING: DT compatible string "sst,sst25vf016b" appears un-documented
-- check ./Documentation/devicetree/bindings/
#79: FILE: arch/arm/boot/dts/r8a7743-iwg20m.dtsi:79:
+ compatible = "sst,sst25vf016b", "jedec,spi-nor";

So I suggest prepending all part number with the appropriate vendor prefixes
in jedec,spi-nor.txt.

BTW, "sst" (for Silicon Storage Technology) should be added to
Documentation/devicetree/bindings/vendor-prefixes.txt, too, to avoid another

WARNING: DT compatible string vendor "sst" appears un-documented --
check ./Documentation/devicetree/bindings/vendor-prefixes.txt
#79: FILE: arch/arm/boot/dts/r8a7743-iwg20m.dtsi:79:
+ compatible = "sst,sst25vf016b", "jedec,spi-nor";

> SST25 memories do support this command hence should use the "jedec,spi-nor"
> value alone. For historical reasons, some DT nodes set their 'compatible'
> string to something like "<vendor>,<model>", "jedec,spi-nor".
> It should work as expected in most case however I discourage from doing so
> in new device trees because it may have some side effects especially when
> the m25p80.c driver is used between the spi-nor.c driver and the SPI
> controller driver.

It is considered good practice to always have in DT a compatible value for
the real part number, not just for a generic fallback.
This allows to discriminate using the real part number, in case an anomaly
shows up later.
How else are you gonna handle e.g. a production batch that accidentally
contains a wrong JEDEC ID? And adding the part-specific compatible value
after the discovery won't help, as it won't be present in existing DTSes.

> It's all about setting the 2nd parameter of spi_nor_scan(), the 'name'
> parameter, as NULL when possible. This parameter should be set to a non NULL
> value only for memories not supporting the JEDEC READ ID op code (0x9F).
> Best regards,
> Cyrille
> Le 24/10/2017 Ã 15:50, Fabrizio Castro a Ãcrit :
>> There are a few DT files that make use of sst25vf016b in their
>> compatible strings, and the driver supports this chip already.
>> This patch improves the documentation and therefore the result
>> of ./scripts/
>> Signed-off-by: Fabrizio Castro <fabrizio.castro@xxxxxxxxxxxxxx>
>> Signed-off-by: Chris Paterson <Chris.Paterson2@xxxxxxxxxxx>
>> Acked-by: Rob Herring <robh@xxxxxxxxxx>
>> Acked-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>> ---
>> Thank you Rob, thank you Geert, and sorry for the delay on this one.
>> Here is v2.
>> Changes in v2:
>> * fixed subject prefix
>> * added changelog
>> Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 +
>> 1 file changed, 1 insertion(+)
>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> index 4cab5d8..bf56d77 100644
>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> @@ -31,6 +31,7 @@ Required properties:
>> s25sl12801
>> s25fl008k
>> s25fl064k
>> + sst25vf016b
>> sst25vf040b
>> sst25wf040b
>> m25p40



Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds