mbox series

[v3,0/3] Read MAC address through NVMEM for sama7g5ek

Message ID 20240628080146.49545-1-andrei.simion@microchip.com (mailing list archive)
Headers show
Series Read MAC address through NVMEM for sama7g5ek | expand

Message

Andrei Simion June 28, 2024, 8:01 a.m. UTC
This series proposes to add EEPROM support and reading MAC addresses
through NVMEM (via Devicetree) for sama7g5ek:
- Add in DT bindings document the EEPROM compatibles :
"microchip,24aa025e48" and "microchip,24aa025e64"
- Update to the driver to support "microchip,24aa025e48" and
"microchip,24aa025e64" and adjusting offset for those 24AA025E{48, 64}.
- Added the nodes in devicetree for eeproms where are stored EUI-48 MAC,
and update gmac nodes to read the MAC via devicetree through NVMEM.

------------------------------------------------------------------
v2 -> v3:
* dt-bindings: eeprom: at24: Add Microchip 24AA025E48/24AA025E64
  - commit subject changed to reference Microchip 24AA025E48/24AA025E64
  - drop the pattern: mac02e4$ and mac02e6$ and a-z from regex
  - add these two devices down at the bottom
  - added Reviewed-by

* eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}
  - add specific compatible names according with
https://ww1.microchip.com/downloads/en/DeviceDoc/24AA02E48-24AA025E48-24AA02E64-24AA025E64-Data-Sheet-20002124H.pdf
  - add extended macros to initialize the structure with explicit value for adjoff
  - drop co-developed-by to maintain the commit history
  (chronological order of modifications)

* ARM: dts: at91: at91-sama7g5ek: add EEPROMs
  - change from atmel,24mac02e4 to microchip,24aa025e48 to align with the datasheet
  - drop co-developed-by to maintain the chronological order of the changes

v1 -> v2:
* dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6
  - change pattern into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler

* eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}
  - no change

* ARM: dts: at91: at91-sama7g5ek: add EEPROMs
  - remove unnecessary #address-cells #size-cells

------------------------------------------------------------------
Andrei Simion (1):
  dt-bindings: eeprom: at24: Add Microchip 24AA025E48/24AA025E64

Claudiu Beznea (2):
  eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}
  ARM: dts: at91: at91-sama7g5ek: add EEPROMs

 .../devicetree/bindings/eeprom/at24.yaml      |  4 ++
 .../arm/boot/dts/microchip/at91-sama7g5ek.dts | 40 +++++++++++++++++++
 drivers/misc/eeprom/at24.c                    | 28 ++++++++++---
 3 files changed, 67 insertions(+), 5 deletions(-)


base-commit: 642a16ca7994a50d7de85715996a8ce171a5bdfb

Comments

Arnd Bergmann June 28, 2024, 8:29 a.m. UTC | #1
On Fri, Jun 28, 2024, at 10:01, Andrei Simion wrote:
> This series proposes to add EEPROM support and reading MAC addresses
> through NVMEM (via Devicetree) for sama7g5ek:
> - Add in DT bindings document the EEPROM compatibles :
> "microchip,24aa025e48" and "microchip,24aa025e64"
> - Update to the driver to support "microchip,24aa025e48" and
> "microchip,24aa025e64" and adjusting offset for those 24AA025E{48, 64}.
> - Added the nodes in devicetree for eeproms where are stored EUI-48 MAC,
> and update gmac nodes to read the MAC via devicetree through NVMEM.

Can you add an explanation about what this is good for?

Do you need to work around broken boot loaders that cannot be
updated and that happen to store the MAC address in the EEPROM,
or are you proposing this as a generic solution that board
developers should actually use?

As far as I can tell, even with this logic in place, users
are better off just having the boot loader read the EEPROM
and storing the MAC address in the in-memory dtb as we do
on other platforms.

      Arnd
Andrei Simion June 28, 2024, 2:06 p.m. UTC | #2
On 28.06.2024 11:29, Arnd Bergmann wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Jun 28, 2024, at 10:01, Andrei Simion wrote:
>> This series proposes to add EEPROM support and reading MAC addresses
>> through NVMEM (via Devicetree) for sama7g5ek:
>> - Add in DT bindings document the EEPROM compatibles :
>> "microchip,24aa025e48" and "microchip,24aa025e64"
>> - Update to the driver to support "microchip,24aa025e48" and
>> "microchip,24aa025e64" and adjusting offset for those 24AA025E{48, 64}.
>> - Added the nodes in devicetree for eeproms where are stored EUI-48 MAC,
>> and update gmac nodes to read the MAC via devicetree through NVMEM.
> 
> Can you add an explanation about what this is good for?
> 
> Do you need to work around broken boot loaders that cannot be
> updated and that happen to store the MAC address in the EEPROM,
> or are you proposing this as a generic solution that board
> developers should actually use?
> 
> As far as I can tell, even with this logic in place, users
> are better off just having the boot loader read the EEPROM
> and storing the MAC address in the in-memory dtb as we do
> on other platforms.
> 
>       Arnd


Our boot chain is ROM BOOT -> AT91Bootstrap -> U-Boot -> Linux Kernel. U-Boot is the stage where we set up the MAC address.
We can skip U-Boot and use the following boot chain ROM BOOT -> AT91Boostrap -> Linux Kernel. 
This patch set is useful for this scenario and also for redundancy (if something related with NET/EEPROM fails in U-Boot).

Best Regards,
Andrei Simion
Arnd Bergmann June 28, 2024, 2:25 p.m. UTC | #3
On Fri, Jun 28, 2024, at 16:06, Andrei.Simion@microchip.com wrote:
> On 28.06.2024 11:29, Arnd Bergmann wrote:
>> 
>> As far as I can tell, even with this logic in place, users
>> are better off just having the boot loader read the EEPROM
>> and storing the MAC address in the in-memory dtb as we do
>> on other platforms.
>
> Our boot chain is ROM BOOT -> AT91Bootstrap -> U-Boot -> Linux Kernel. 
> U-Boot is the stage where we set up the MAC address.
> We can skip U-Boot and use the following boot chain ROM BOOT -> 
> AT91Boostrap -> Linux Kernel. 

Right, I can see how that is useful. Can you add that description
in the patch?

> This patch set is useful for this scenario and also for redundancy (if 
> something related with NET/EEPROM fails in U-Boot).

Not sure if redundancy is what we want the boot loader level ;-)

    Arnd