linux-kernel - Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a9eea961-9dd5-4026-a15a-ff5635cda268@microchip.com>
Date: Mon, 16 Jun 2025 06:43:59 +0000
From: <Manikandan.M@...rochip.com>
To: <mwalle@...nel.org>, <tudor.ambarus@...aro.org>, <robh@...nel.org>,
	<krzk+dt@...nel.org>, <conor+dt@...nel.org>, <Nicolas.Ferre@...rochip.com>,
	<alexandre.belloni@...tlin.com>, <claudiu.beznea@...on.dev>,
	<pratyush@...nel.org>, <miquel.raynal@...tlin.com>, <richard@....at>,
	<vigneshr@...com>, <devicetree@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<linux-mtd@...ts.infradead.org>
Subject: Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add
 nvmem-layout in QSPI for EUI48 MAC Address

Hi Michael,

On 12/06/25 11:47 am, Michael Walle wrote:
> Hi,
> 
>>>>>> Add nvmem-layout in QSPI to read the EUI48 Mac address by the
>>>>>> net drivers using the nvmem property.The offset is set to 0x0
>>>>>> since the factory programmed address is available in the
>>>>>> resource managed space and the size determine if the requested
>>>>>> address is of EUI48 (0x6) or EUI-64 (0x8) type.
>>>>>> This is useful for cases where U-Boot is skipped and the Ethernet
>>>>>> MAC address is needed to be configured by the kernel
>>>>>>
>>>>>> Signed-off-by: Manikandan Muralidharan <manikandan.m@...rochip.com>
>>>>>> ---
>>>>>>     .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi    | 13 +++++++++++++
>>>>>>     1 file changed, 13 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>>>> index b34c5072425a..be06df1b7d66 100644
>>>>>> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>>>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>>>> @@ -210,6 +210,9 @@ &macb0 {
>>>>>>          #size-cells = <0>;
>>>>>>          phy-mode = "rmii";
>>>>>>
>>>>>> +     nvmem-cells = <&mac_address_eui48>;
>>>>>> +     nvmem-cell-names = "mac-address";
>>>>>> +
>>>>>>          ethernet-phy@0 {
>>>>>>                  reg = <0x0>;
>>>>>>                  interrupt-parent = <&pioA>;
>>>>>> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 {
>>>>>>                  m25p,fast-read;
>>>>>>                  status = "disabled";
>>>>>>
>>>>>> +             nvmem-layout {
>>>
>>> IMHO this should be "sfdp {".
>>>
>>>>>> +                     compatible = "fixed-layout";
>>>
>>> Please read my feedback on the first version again..
>>>
>>> For the DT maintainers. The SFDP is a small table based content that
>>> provide basic information about the flash. There are standard tables
>>> which are specified by the JEDEC standard and there are vendor
>>> tables, most of the time without proper documentation (or none at
>>> all).
>>>
>>> Somehow we need to specify at what table we want to look at. I'd
>>> like to see a binding which can potentially expose anything inside
>>> the SFDP.
>>>
>>> So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN"
>>> where NNNN is the table parameter id. Additionally, the standard ids
>>> could have names like "jedec,sfdp-bfpt" (basic flash parameter table).
>>>
>>> So in your case that would be:
>>>
>>> flash {
>>> 	sfdp {
>>> 		mac_address: table-1 {
>>> 			compatible = "jedec,sfdp-idNNNN";
>>> 		};
>>> 	};
>>
>> Should the nvmem-layout be included as a child node under sfdp {}, or
>> should it be implemented as a separate vendor-specific driver to handle
>> the changes introduced in patch 1/3 ?
> 
> There is no nvmem-layout involved here.
> 
> But another possibility is to make it one. Then you have to
>   (1) expose the *entire* sfpd as a nvmem device
>   (2a) write an nvmem-layouts driver (in drivers/nvmem/layouts/)
>   (2b) come up with a DT binding that is generic enough to expose
>        various parameters of the SFDP, not just a one-off for the
>        MAC address use case.
> 
> Maybe that is even a better fit.
> 
>>> };
>>>
>>> I don't know what NNNN is. Could you please provide a dump of the
>>> sfdp of your flash.
>>
>> Please find the entire SFDP data of SST26VF064BEUI flash in Table 11.1
>> of 11.0 APPENDIX
> 
> Please dump it according to [1], so we have it in a machine readable
> format.

Here is the dump of sfdp as per [1],

[root@...a5 ~]$ xxd -p /sys/bus/spi/devices/spi2.0/spi-nor/sfdp
53464450060102ff00060110300000ff81000106000100ffbf00021c0002
0001fffffffffffffffffffffffffffffffffd20f1ffffffff0344eb086b
083b80bbfeffffffffff00ffffff440b0c200dd80fd810d820914824806f
1d81ed0f773830b030b0f7ffffff29c25cfff030c080ffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffff0004fff37f0000f57f0000f9ff
7d00f57f0000f37f0000ffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffbf2643ffb95ffdff30f260f332ff0a122346ff0f19320f1919ffffff
ffffffff00669938ff05013506040232b03072428de89888a585c09faf5a
ffff06ec060c0003080bffffffffff07ffff0202ff060300fdfd040700fc
0300fefe0202070e3004916245a09240001ec0000005a092


> 
> -michael
> 
> [1] https://docs.kernel.org/driver-api/mtd/spi-nor.html
> 
> 
>>> On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote:
> 
>>
>> https://ww1.microchip.com/downloads/aemDocuments/documents/MPD/ProductDocuments/DataSheets/SST26VF064BEUI-Data-Sheet-DS20006138.pdf
>>
>>
>> The vendor parameter ID is 'BF' if I am not wrong.
>>>
>>> -michael
>>>
>>>>>> +                     #address-cells = <1>;
>>>>>> +                     #size-cells = <1>;
>>>>>> +
>>>>>> +                     mac_address_eui48: mac-address@0 {
>>>>>> +                             reg = <0x0 0x6>;
>>>>>> +                     };
>>>>>
>>>>> How would this work if in the future the mchp vendor table adds some
>>>>> other info that needs to be referenced as nvmem? How do you distinguish
>>>>> the info from the table?
>>>>> Would it be possible to have some kind of address and size to reference
>>>>> the SFDP?
>>>>
>>>> I was previously advised not to hardcode the offset in the Device Tree
>>>> [1]. In the current implementation (patch 1/3), the read callback for
>>>> the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC
>>>> addresses based on the nvmem size, which corresponds to the size of the
>>>> respective MAC address.
>>>>
>>>> [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/
>>>>
>>>>>
>>>>>> +             };
>>>>>> +
>>>>>>                  partitions {
>>>>>>                          compatible = "fixed-partitions";
>>>>>>                          #address-cells = <1>;
>>>>>
>>>
> 

-- 
Thanks and Regards,
Manikandan M.