* [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