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

Michael Walle
2025-06-12T06:20:11Z

[-- Attachment #1.1: Type: text/plain, Size: 5043 bytes --]

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@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 ?

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.

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

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]