* [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM @ 2021-06-22 8:08 Ahmad Fatoum 2021-06-22 8:35 ` Alexander Dahl 2021-09-22 10:54 ` Ahmad Fatoum 0 siblings, 2 replies; 6+ messages in thread From: Ahmad Fatoum @ 2021-06-22 8:08 UTC (permalink / raw) To: barebox; +Cc: Alexander Dahl, Ahmad Fatoum With the latest NVMEM enhancements merged, barebox networking core now always consults NVMEM cells referenced in the network controller device tree node before it falls back to randomizing a new address. The SAM5D27-SOM1 has a 256 byte EEPROM, which holds a MAC address in its last 6 bytes. Describe this in the device tree, so boards using the SoM will get an unique MAC address assigned and fixed up into the kernel device tree. This change can be dropped again when/if the change is submitted and applied upstream. Reported-by: Alexander Dahl <ada@thorsis.com> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- arch/arm/dts/at91-sama5d27_som1.dtsi | 18 ++++++++++++++++++ arch/arm/dts/at91-sama5d27_som1_ek.dts | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 arch/arm/dts/at91-sama5d27_som1.dtsi diff --git a/arch/arm/dts/at91-sama5d27_som1.dtsi b/arch/arm/dts/at91-sama5d27_som1.dtsi new file mode 100644 index 000000000000..0d84c45f9263 --- /dev/null +++ b/arch/arm/dts/at91-sama5d27_som1.dtsi @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) + +#include "sama5d2.dtsi" + +&macb0 { + nvmem-cells = <&macaddr>; + nvmem-cell-names = "mac-address"; +}; + +&{/ahb/apb/i2c@f8028000/at24@50} { + #address-cells = <1>; + #size-cells = <1>; + + macaddr: mac-address@fa { + reg = <0xfa 6>; + label = "mac-address"; + }; +}; diff --git a/arch/arm/dts/at91-sama5d27_som1_ek.dts b/arch/arm/dts/at91-sama5d27_som1_ek.dts index 97a326dd2b26..1a704b42680f 100644 --- a/arch/arm/dts/at91-sama5d27_som1_ek.dts +++ b/arch/arm/dts/at91-sama5d27_som1_ek.dts @@ -4,7 +4,7 @@ */ #include <arm/at91-sama5d27_som1_ek.dts> -#include "sama5d2.dtsi" +#include "at91-sama5d27_som1.dtsi" / { chosen { -- 2.29.2 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM 2021-06-22 8:08 [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM Ahmad Fatoum @ 2021-06-22 8:35 ` Alexander Dahl 2021-06-22 9:07 ` Ahmad Fatoum 2021-09-22 10:54 ` Ahmad Fatoum 1 sibling, 1 reply; 6+ messages in thread From: Alexander Dahl @ 2021-06-22 8:35 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox, Alexander Dahl Hei hei, Am Tue, Jun 22, 2021 at 10:08:11AM +0200 schrieb Ahmad Fatoum: > With the latest NVMEM enhancements merged, barebox networking core now > always consults NVMEM cells referenced in the network controller > device tree node before it falls back to randomizing a new address. > > The SAM5D27-SOM1 has a 256 byte EEPROM, which holds a MAC address in its > last 6 bytes. Describe this in the device tree, so boards using the SoM > will get an unique MAC address assigned and fixed up into the kernel > device tree. I have access to such a device, but I can not promise I have time to test this currently. :-/ > This change can be dropped again when/if the change is > submitted and applied upstream. What do you mean with "upstream" here? Linux kernel? I just had a short look into u-boot for that board, there's the i2c eeprom set in dts only, and dts is still the old u-boot way, not dts from kernel plus fixups in a separate file. The mac is set in board/atmel/sama5d27_som1_ek/sama5d27_som1_ek.c like this: 87 #define MAC24AA_MAC_OFFSET 0xfa 88 89 #ifdef CONFIG_MISC_INIT_R 90 int misc_init_r(void) 91 { 92 #ifdef CONFIG_I2C_EEPROM 93 at91_set_ethaddr(MAC24AA_MAC_OFFSET); 94 #endif 95 return 0; 96 } 97 #endif What would be the right way for kernel, u-boot, and barebox? Have i2c eeprom defined in dts and an nvmem cell on top like you proposed for barebox now? Not sure if u-boot can do that (already)? But it would still work if only Linux and barebox did it that way, right? Greets Alex > > Reported-by: Alexander Dahl <ada@thorsis.com> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > arch/arm/dts/at91-sama5d27_som1.dtsi | 18 ++++++++++++++++++ > arch/arm/dts/at91-sama5d27_som1_ek.dts | 2 +- > 2 files changed, 19 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/dts/at91-sama5d27_som1.dtsi > > diff --git a/arch/arm/dts/at91-sama5d27_som1.dtsi b/arch/arm/dts/at91-sama5d27_som1.dtsi > new file mode 100644 > index 000000000000..0d84c45f9263 > --- /dev/null > +++ b/arch/arm/dts/at91-sama5d27_som1.dtsi > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > + > +#include "sama5d2.dtsi" > + > +&macb0 { > + nvmem-cells = <&macaddr>; > + nvmem-cell-names = "mac-address"; > +}; > + > +&{/ahb/apb/i2c@f8028000/at24@50} { > + #address-cells = <1>; > + #size-cells = <1>; > + > + macaddr: mac-address@fa { > + reg = <0xfa 6>; > + label = "mac-address"; > + }; > +}; > diff --git a/arch/arm/dts/at91-sama5d27_som1_ek.dts b/arch/arm/dts/at91-sama5d27_som1_ek.dts > index 97a326dd2b26..1a704b42680f 100644 > --- a/arch/arm/dts/at91-sama5d27_som1_ek.dts > +++ b/arch/arm/dts/at91-sama5d27_som1_ek.dts > @@ -4,7 +4,7 @@ > */ > > #include <arm/at91-sama5d27_som1_ek.dts> > -#include "sama5d2.dtsi" > +#include "at91-sama5d27_som1.dtsi" > > / { > chosen { > -- > 2.29.2 > > > _______________________________________________ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM 2021-06-22 8:35 ` Alexander Dahl @ 2021-06-22 9:07 ` Ahmad Fatoum 2021-06-23 6:27 ` Alexander Dahl 0 siblings, 1 reply; 6+ messages in thread From: Ahmad Fatoum @ 2021-06-22 9:07 UTC (permalink / raw) To: barebox Hi, On 22.06.21 10:35, Alexander Dahl wrote: > Hei hei, > > Am Tue, Jun 22, 2021 at 10:08:11AM +0200 schrieb Ahmad Fatoum: >> With the latest NVMEM enhancements merged, barebox networking core now >> always consults NVMEM cells referenced in the network controller >> device tree node before it falls back to randomizing a new address. >> >> The SAM5D27-SOM1 has a 256 byte EEPROM, which holds a MAC address in its >> last 6 bytes. Describe this in the device tree, so boards using the SoM >> will get an unique MAC address assigned and fixed up into the kernel >> device tree. > > I have access to such a device, but I can not promise I have time to > test this currently. :-/ No worries, I've an EK1 myself and tested it before sending it out. >> This change can be dropped again when/if the change is >> submitted and applied upstream. > > What do you mean with "upstream" here? Linux kernel? Yes. The dts/ directory in barebox is regularly synchronized with the upstream Device Tree repository, which is still hosted in the Linux repository. > I just had a short look into u-boot for that board, there's the i2c > eeprom set in dts only, and dts is still the old u-boot way, not dts > from kernel plus fixups in a separate file. The mac is set in > board/atmel/sama5d27_som1_ek/sama5d27_som1_ek.c like this: > > 87 #define MAC24AA_MAC_OFFSET 0xfa > 88 > 89 #ifdef CONFIG_MISC_INIT_R > 90 int misc_init_r(void) > 91 { > 92 #ifdef CONFIG_I2C_EEPROM > 93 at91_set_ethaddr(MAC24AA_MAC_OFFSET); > 94 #endif > 95 return 0; > 96 } > 97 #endif > > What would be the right way for kernel, u-boot, and barebox? Have i2c > eeprom defined in dts and an nvmem cell on top like you proposed for > barebox now? Many Linux network drivers already call of_get_mac_address and thus would read out a NVMEM cell if available. There aren't too many device trees making use of it, but that seems the preferred way going forward. (MAC addressed fixed up by bootloader is higher priority though). In general, I think Linux should not rely more than necessary on firmware. > Not sure if u-boot can do that (already)? No idea. grepping "mac-address" shows no nvmem related C code though. > But it would > still work if only Linux and barebox did it that way, right? So far, either board code set it, similar to your snippet above: eth_register_ethaddr(if_index, mac_addr); Or NVMEM drivers had a barebox,provide-mac-address = <&phandle_to_netdev ...> property and they called eth_register_ethaddr. The NVMEM binding is the upstream way, so after a device tree sync, even existing boards may find themselves with the correct address instead of randomization without having to do anything (nvmem cells doesn't override other methods, so no brekage). Cheers, Ahmad > > Greets > Alex > >> >> Reported-by: Alexander Dahl <ada@thorsis.com> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> >> --- >> arch/arm/dts/at91-sama5d27_som1.dtsi | 18 ++++++++++++++++++ >> arch/arm/dts/at91-sama5d27_som1_ek.dts | 2 +- >> 2 files changed, 19 insertions(+), 1 deletion(-) >> create mode 100644 arch/arm/dts/at91-sama5d27_som1.dtsi >> >> diff --git a/arch/arm/dts/at91-sama5d27_som1.dtsi b/arch/arm/dts/at91-sama5d27_som1.dtsi >> new file mode 100644 >> index 000000000000..0d84c45f9263 >> --- /dev/null >> +++ b/arch/arm/dts/at91-sama5d27_som1.dtsi >> @@ -0,0 +1,18 @@ >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> + >> +#include "sama5d2.dtsi" >> + >> +&macb0 { >> + nvmem-cells = <&macaddr>; >> + nvmem-cell-names = "mac-address"; >> +}; >> + >> +&{/ahb/apb/i2c@f8028000/at24@50} { >> + #address-cells = <1>; >> + #size-cells = <1>; >> + >> + macaddr: mac-address@fa { >> + reg = <0xfa 6>; >> + label = "mac-address"; >> + }; >> +}; >> diff --git a/arch/arm/dts/at91-sama5d27_som1_ek.dts b/arch/arm/dts/at91-sama5d27_som1_ek.dts >> index 97a326dd2b26..1a704b42680f 100644 >> --- a/arch/arm/dts/at91-sama5d27_som1_ek.dts >> +++ b/arch/arm/dts/at91-sama5d27_som1_ek.dts >> @@ -4,7 +4,7 @@ >> */ >> >> #include <arm/at91-sama5d27_som1_ek.dts> >> -#include "sama5d2.dtsi" >> +#include "at91-sama5d27_som1.dtsi" >> >> / { >> chosen { >> -- >> 2.29.2 >> >> >> _______________________________________________ >> barebox mailing list >> barebox@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/barebox > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM 2021-06-22 9:07 ` Ahmad Fatoum @ 2021-06-23 6:27 ` Alexander Dahl 0 siblings, 0 replies; 6+ messages in thread From: Alexander Dahl @ 2021-06-23 6:27 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox Hi, Am Tue, Jun 22, 2021 at 11:07:17AM +0200 schrieb Ahmad Fatoum: > Hi, > > On 22.06.21 10:35, Alexander Dahl wrote: > > I just had a short look into u-boot for that board, there's the i2c > > eeprom set in dts only, and dts is still the old u-boot way, not dts > > from kernel plus fixups in a separate file. The mac is set in > > board/atmel/sama5d27_som1_ek/sama5d27_som1_ek.c like this: > > > > 87 #define MAC24AA_MAC_OFFSET 0xfa > > 88 > > 89 #ifdef CONFIG_MISC_INIT_R > > 90 int misc_init_r(void) > > 91 { > > 92 #ifdef CONFIG_I2C_EEPROM > > 93 at91_set_ethaddr(MAC24AA_MAC_OFFSET); > > 94 #endif > > 95 return 0; > > 96 } > > 97 #endif > > > > What would be the right way for kernel, u-boot, and barebox? Have i2c > > eeprom defined in dts and an nvmem cell on top like you proposed for > > barebox now? > > Many Linux network drivers already call of_get_mac_address and thus would > read out a NVMEM cell if available. There aren't too many device trees > making use of it, but that seems the preferred way going forward. > (MAC addressed fixed up by bootloader is higher priority though). > > In general, I think Linux should not rely more than necessary on firmware. > > > Not sure if u-boot can do that (already)? > > No idea. grepping "mac-address" shows no nvmem related C code though. FTR: grepping for "nvmem" in u-boot master matches in dts files and binding docs only. There seems to be currently no code in u-boot interpreting those dts nodes at all, no nvmem drivers. > > But it would > > still work if only Linux and barebox did it that way, right? > > So far, either board code set it, similar to your snippet above: > eth_register_ethaddr(if_index, mac_addr); > > Or NVMEM drivers had a barebox,provide-mac-address = <&phandle_to_netdev ...> > property and they called eth_register_ethaddr. > > The NVMEM binding is the upstream way, so after a device tree sync, even existing > boards may find themselves with the correct address instead of randomization without > having to do anything (nvmem cells doesn't override other methods, so no brekage). Makes sense. Greets Alex _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM 2021-06-22 8:08 [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM Ahmad Fatoum 2021-06-22 8:35 ` Alexander Dahl @ 2021-09-22 10:54 ` Ahmad Fatoum 2021-10-04 13:10 ` Sascha Hauer 1 sibling, 1 reply; 6+ messages in thread From: Ahmad Fatoum @ 2021-09-22 10:54 UTC (permalink / raw) To: barebox, Sascha Hauer; +Cc: Alexander Dahl Hello Sascha, On 22.06.21 10:08, Ahmad Fatoum wrote: > With the latest NVMEM enhancements merged, barebox networking core now > always consults NVMEM cells referenced in the network controller > device tree node before it falls back to randomizing a new address. > > The SAM5D27-SOM1 has a 256 byte EEPROM, which holds a MAC address in its > last 6 bytes. Describe this in the device tree, so boards using the SoM > will get an unique MAC address assigned and fixed up into the kernel > device tree. This change can be dropped again when/if the change is > submitted and applied upstream. This patch seems to have slipped through the cracks. Cheers, Ahmad > > Reported-by: Alexander Dahl <ada@thorsis.com> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > arch/arm/dts/at91-sama5d27_som1.dtsi | 18 ++++++++++++++++++ > arch/arm/dts/at91-sama5d27_som1_ek.dts | 2 +- > 2 files changed, 19 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/dts/at91-sama5d27_som1.dtsi > > diff --git a/arch/arm/dts/at91-sama5d27_som1.dtsi b/arch/arm/dts/at91-sama5d27_som1.dtsi > new file mode 100644 > index 000000000000..0d84c45f9263 > --- /dev/null > +++ b/arch/arm/dts/at91-sama5d27_som1.dtsi > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > + > +#include "sama5d2.dtsi" > + > +&macb0 { > + nvmem-cells = <&macaddr>; > + nvmem-cell-names = "mac-address"; > +}; > + > +&{/ahb/apb/i2c@f8028000/at24@50} { > + #address-cells = <1>; > + #size-cells = <1>; > + > + macaddr: mac-address@fa { > + reg = <0xfa 6>; > + label = "mac-address"; > + }; > +}; > diff --git a/arch/arm/dts/at91-sama5d27_som1_ek.dts b/arch/arm/dts/at91-sama5d27_som1_ek.dts > index 97a326dd2b26..1a704b42680f 100644 > --- a/arch/arm/dts/at91-sama5d27_som1_ek.dts > +++ b/arch/arm/dts/at91-sama5d27_som1_ek.dts > @@ -4,7 +4,7 @@ > */ > > #include <arm/at91-sama5d27_som1_ek.dts> > -#include "sama5d2.dtsi" > +#include "at91-sama5d27_som1.dtsi" > > / { > chosen { > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM 2021-09-22 10:54 ` Ahmad Fatoum @ 2021-10-04 13:10 ` Sascha Hauer 0 siblings, 0 replies; 6+ messages in thread From: Sascha Hauer @ 2021-10-04 13:10 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox, Alexander Dahl On Wed, Sep 22, 2021 at 12:54:25PM +0200, Ahmad Fatoum wrote: > Hello Sascha, > > On 22.06.21 10:08, Ahmad Fatoum wrote: > > With the latest NVMEM enhancements merged, barebox networking core now > > always consults NVMEM cells referenced in the network controller > > device tree node before it falls back to randomizing a new address. > > > > The SAM5D27-SOM1 has a 256 byte EEPROM, which holds a MAC address in its > > last 6 bytes. Describe this in the device tree, so boards using the SoM > > will get an unique MAC address assigned and fixed up into the kernel > > device tree. This change can be dropped again when/if the change is > > submitted and applied upstream. > > This patch seems to have slipped through the cracks. Applied now, thanks Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-04 13:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-22 8:08 [PATCH] ARM: at91: sama5d27_som1_ek: populate MAC address from EEPROM Ahmad Fatoum 2021-06-22 8:35 ` Alexander Dahl 2021-06-22 9:07 ` Ahmad Fatoum 2021-06-23 6:27 ` Alexander Dahl 2021-09-22 10:54 ` Ahmad Fatoum 2021-10-04 13:10 ` Sascha Hauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox