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

[<Manikandan.M@microchip.com>]

Hi Michael,

On 11/06/25 1:01 pm, Michael Walle wrote:
> Hi,
> 
> On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote:
>>>> 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@microchip.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 ?

> };
> 
> 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

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.