Re: [PATCH v4 1/7] dt-bindings: mtd: jedec,spi-nor: allow the SFDP to be exposed via NVMEM

["Michael Walle" <michael@walle.cc>]

[-- Attachment #1: Type: text/plain, Size: 1630 bytes --]

Hi,

>> Add an optional "sfdp" child node (compatible "jedec,sfdp") that
>> describes the SFDP as a read-only NVMEM provider via nvmem.yaml, so its
>> contents (e.g. a vendor EUI-48/EUI-64) can be read through NVMEM cells.
>>
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>
> I would expect it to follow nvmem conventions like this, notice
> compatibles specific-to-general with sfdp first:


> sfdp {
>     /* NVMEM provided by SFDP */
>     compatible = "jedec,sfdp", "nvmem-cells";
>     label = "SFDP";

Isn't using label frowned upon? I wouldn't add that to the example.

>     read-only;
>     #address-cells = <1>;
>     #size-cells = <1>;
>
>     mac0: macaddr@0x00 {
>         reg = <0x00 0x06>;
>     };
>     mac1: macaddr@0x06 {
>         reg = <0x06 0x06>;
>     };
> };

If I'm correct, this is the old style, see commit bd912c991d2e
("dt-bindings: nvmem: layouts: add fixed-layout"). So it should
eventually look like:

sfdp {
     compatible = "jedec,sfdp";

     nvmem-layout {
	     compatible = "microchip,sst26vf-sfdp-eui";
     };
};

Which is what patch series will lead to.

Also I'm not sure if we really need to add the "nvmem-cells" here.
IIRC in MTD it was there to tell a driver to add an nvmem device to
an already existing compatible/node.

Apart from the MTD case, I've just found qcom,smem-part,yaml which
has compatible = "nvmem-cells".

-michael

> Your example should definitely be more elaborate like this,
> just an opaque sfdp node will not suffice. Maybe a separate
> example?
>
> Yours,
> Linus Walleij


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]