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] [thread-next>] [day] [month] [year] [list]

Message-Id: <DAJJ1UHCLV0M.2GGHO5PDRWMYH@kernel.org>
Date: Wed, 11 Jun 2025 09:31:59 +0200
From: "Michael Walle" <mwalle@...nel.org>
To: <Manikandan.M@...rochip.com>, <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,

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@...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";
		};
	};
};

I don't know what NNNN is. Could you please provide a dump of the
sfdp of your flash.

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


Download attachment "signature.asc" of type "application/pgp-signature" (298 bytes)