public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: "Michael Walle" <mwalle@kernel.org>
To: <Manikandan.M@microchip.com>, <tudor.ambarus@linaro.org>,
	<robh@kernel.org>, <krzk+dt@kernel.org>, <conor+dt@kernel.org>,
	<Nicolas.Ferre@microchip.com>, <alexandre.belloni@bootlin.com>,
	<claudiu.beznea@tuxon.dev>, <pratyush@kernel.org>,
	<miquel.raynal@bootlin.com>, <richard@nod.at>, <vigneshr@ti.com>,
	<devicetree@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <linux-mtd@lists.infradead.org>
Subject: Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address
Date: Wed, 11 Jun 2025 09:31:59 +0200	[thread overview]
Message-ID: <DAJJ1UHCLV0M.2GGHO5PDRWMYH@kernel.org> (raw)
In-Reply-To: <759e4a1e-6af4-4bf9-9a95-01a7f6faaf46@microchip.com>


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

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

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


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

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  parent reply	other threads:[~2025-06-11  7:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-21  7:03 [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Manikandan Muralidharan
2025-05-21  7:03 ` [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework Manikandan Muralidharan
2025-06-07 11:01   ` Claudiu Beznea
2025-06-09  8:04   ` Tudor Ambarus
2025-06-11  6:49     ` Michael Walle
2025-05-21  7:03 ` [PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding Manikandan Muralidharan
2025-06-07 11:02   ` Claudiu Beznea
2025-05-21  7:03 ` [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address Manikandan Muralidharan
2025-06-09  8:17   ` Tudor Ambarus
     [not found]     ` <759e4a1e-6af4-4bf9-9a95-01a7f6faaf46@microchip.com>
2025-06-11  7:31       ` Michael Walle [this message]
     [not found]         ` <7c373149-53b9-4488-a8d0-e5560cdee7e0@microchip.com>
2025-06-12  6:17           ` Michael Walle
2025-05-21 12:56 ` [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Rob Herring (Arm)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DAJJ1UHCLV0M.2GGHO5PDRWMYH@kernel.org \
    --to=mwalle@kernel.org \
    --cc=Manikandan.M@microchip.com \
    --cc=Nicolas.Ferre@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=pratyush@kernel.org \
    --cc=richard@nod.at \
    --cc=robh@kernel.org \
    --cc=tudor.ambarus@linaro.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox