Skip to contentSkip to conversation

ARM: mxs: m28evk: Disable OCOTP OUI loading

PATCH New Marek Vasut 6 participants 23 messages Sep 18, 2012 – Sep 20, 2012

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>
---
 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)
arch/arm/mach-mxs/mach-mxs.c +0 -2
index 170a930..71d47f5 100644
@@ -255,8 +255,6 @@ static void __init imx28_evk_post_init(void)
 
 static void __init m28evk_init(void)
 {
-	update_fec_mac_prop(OUI_DENX);
-
 	mxsfb_pdata.mode_list = m28evk_video_modes;
 	mxsfb_pdata.mode_count = ARRAY_SIZE(m28evk_video_modes);
 	mxsfb_pdata.default_bpp = 16;
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Quoted 5 lines · Marek Vasut

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Ok, since you are the maintainer of m28evk board, it could be your call to do so. While for imx28-evk, the kernel at least runs with bootlets which is not DT capable.

Quoted 12 lines
Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>
---
 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)
arch/arm/mach-mxs/mach-mxs.c +0 -0
index 170a930..71d47f5 100644

arch/arm/mach-mxs/mach-mxs.c static void __init imx28_evk_post_init(void) · line 259

Quoted 5 lines
arch/arm/mach-mxs/mach-mxs.c +0 -2
@@ -255,8 +255,6 @@ static void __init imx28_evk_post_init(void)
 
 static void __init m28evk_init(void)
 {
-	update_fec_mac_prop(OUI_DENX);
-

Since it's the only user of OUI_DENX, can we remove enum mac_oui completely and make update_fec_mac_prop a fsl specific call?

Shawn

Quoted 6 lines
arch/arm/mach-mxs/mach-mxs.c +0 -0
 	mxsfb_pdata.mode_list = m28evk_video_modes;
 	mxsfb_pdata.mode_count = ARRAY_SIZE(m28evk_video_modes);
 	mxsfb_pdata.default_bpp = 16;
Signature · 1 line
1.7.10.4
#2 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Shawn Guo,

Quoted 9 lines · Marek Vasut
Quoted 4 lines

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Ok, since you are the maintainer of m28evk board, it could be your call to do so. While for imx28-evk, the kernel at least runs with bootlets which is not DT capable.

I believe most of the mx28evks run mainline U-Boot these days. Especially since there is much better support in upstream u-boot than in freescale one. So what about dropping this function altogether or make it DT-configurable?

[...]

Best regards,
Marek Vasut

#3 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

In reply to Shawn Guo

Hi Shawn,

Le 19/09/2012 05:15, Shawn Guo a ?crit :

Quoted 23 lines · Marek Vasut
Quoted 5 lines

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Ok, since you are the maintainer of m28evk board, it could be your call to do so. While for imx28-evk, the kernel at least runs with bootlets which is not DT capable.

Quoted 13 lines
Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>
---
 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)
arch/arm/mach-mxs/mach-mxs.c +0 -0
index 170a930..71d47f5 100644
@@ -255,8 +255,6 @@ static void __init imx28_evk_post_init(void)
 
Quoted 7 lines

arch/arm/mach-mxs/mach-mxs.c static void __init imx28_evk_post_init(void) · line 259

Quoted 4 lines
arch/arm/mach-mxs/mach-mxs.c +0 -2
 static void __init m28evk_init(void)
 {
-	update_fec_mac_prop(OUI_DENX);
-

Since it's the only user of OUI_DENX, can we remove enum mac_oui completely and make update_fec_mac_prop a fsl specific call?

I have a patch on the way that adds the Crystalfontz OUI for the CFA-10049, so it will be quite useful to me in the future.

Maxime

Signature · 4 lines
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#4 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Maxime Ripard,

Quoted 30 lines

Hi Shawn,

Le 19/09/2012 05:15, Shawn Guo a ?crit :

Quoted 27 lines · Marek Vasut
Quoted 4 lines

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Ok, since you are the maintainer of m28evk board, it could be your call to do so. While for imx28-evk, the kernel at least runs with bootlets which is not DT capable.

Quoted 17 lines
Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>

---

 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)
arch/arm/mach-mxs/mach-mxs.c +0 -0
index 170a930..71d47f5 100644
@@ -255,8 +255,6 @@ static void __init imx28_evk_post_init(void)
 
 static void __init m28evk_init(void)
 {
 
Quoted 8 lines
Quoted 5 lines

arch/arm/mach-mxs/mach-mxs.c static void __init imx28_evk_post_init(void) · line 259

Quoted 2 lines
arch/arm/mach-mxs/mach-mxs.c +0 -2
-	update_fec_mac_prop(OUI_DENX);
-

Since it's the only user of OUI_DENX, can we remove enum mac_oui completely and make update_fec_mac_prop a fsl specific call?

I have a patch on the way that adds the Crystalfontz OUI for the CFA-10049, so it will be quite useful to me in the future.

Do you use U-Boot on that board?

Quoted 1 line

Maxime

Best regards,
Marek Vasut

#5 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Hi Marek,

Le 19/09/2012 12:50, Marek Vasut a ?crit :

Quoted 34 lines
Quoted 34 lines

Le 19/09/2012 05:15, Shawn Guo a ?crit :

Quoted 32 lines · Marek Vasut
Quoted 4 lines

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Ok, since you are the maintainer of m28evk board, it could be your call to do so. While for imx28-evk, the kernel at least runs with bootlets which is not DT capable.

Quoted 13 lines
Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>

---

 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)
arch/arm/mach-mxs/mach-mxs.c +0 -0
index 170a930..71d47f5 100644

arch/arm/mach-mxs/mach-mxs.c static void __init imx28_evk_post_init(void) · line 260

Quoted 6 lines
arch/arm/mach-mxs/mach-mxs.c +0 -2
@@ -255,8 +255,6 @@ static void __init imx28_evk_post_init(void)
 
 static void __init m28evk_init(void)
 {
 
-	update_fec_mac_prop(OUI_DENX);
-

Since it's the only user of OUI_DENX, can we remove enum mac_oui completely and make update_fec_mac_prop a fsl specific call?

Quoted 4 lines
Quoted 2 lines

I have a patch on the way that adds the Crystalfontz OUI for the CFA-10049, so it will be quite useful to me in the future.

Do you use U-Boot on that board?

No, we use barebox for it. And since the CFA-10049 is an expansion board to the CFA-10036, we only have support for the CFA-10036 in barebox, and support the devices present in the CFA-10049 through loading a different device tree.

So basically, I'd like to avoid to get the mac from the ocotp in barebox, since I won't need it if the CFA-10049 isn't plugged in.

Moreover, I've found no clue that barebox adds the local-mac-address or the mac-address fields to the device tree, so this assumption seems quite u-boot specific.

tl;dr: I found the current way of handling the mac address very convenient, and I wouldn't like to see it removed.

Maxime

Signature · 4 lines
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#6 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Maxime Ripard,

Quoted 47 lines

Hi Marek,

Le 19/09/2012 12:50, Marek Vasut a ?crit :

Quoted 34 lines
Quoted 34 lines

Le 19/09/2012 05:15, Shawn Guo a ?crit :

Quoted 32 lines · Marek Vasut
Quoted 4 lines

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Ok, since you are the maintainer of m28evk board, it could be your call to do so. While for imx28-evk, the kernel at least runs with bootlets which is not DT capable.

Quoted 13 lines
Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>

---

 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)
(quoted diff) +0 -0

b/arch/arm/mach-mxs/mach-mxs.c index 170a930..71d47f5 100644

arch/arm/mach-mxs/mach-mxs.c static void __init imx28_evk_post_init(void) · line 260

Quoted 6 lines
arch/arm/mach-mxs/mach-mxs.c +0 -2
@@ -255,8 +255,6 @@ static void __init imx28_evk_post_init(void)
 
 static void __init m28evk_init(void)
 {
 
-	update_fec_mac_prop(OUI_DENX);
-

Since it's the only user of OUI_DENX, can we remove enum mac_oui completely and make update_fec_mac_prop a fsl specific call?

Quoted 4 lines
Quoted 2 lines

I have a patch on the way that adds the Crystalfontz OUI for the CFA-10049, so it will be quite useful to me in the future.

Do you use U-Boot on that board?

No, we use barebox for it. And since the CFA-10049 is an expansion board to the CFA-10036, we only have support for the CFA-10036 in barebox, and support the devices present in the CFA-10049 through loading a different device tree.

Quoted 6 lines

So basically, I'd like to avoid to get the mac from the ocotp in barebox, since I won't need it if the CFA-10049 isn't plugged in.

Moreover, I've found no clue that barebox adds the local-mac-address or the mac-address fields to the device tree, so this assumption seems quite u-boot specific.

I dunno what barebox is, but it seems really crap. Don't use crap. The "mac- address" and "local-mac-address" nodes must be set up by the bootloader, if barebox or whatever doesn't set it, it's broken.

U-Boot did that since PPC days (so basically all PPCs do that). And the FDT got onto ARM from PPCs. And FEC was used on PPCs too, thus I see no point to diverge on ARM from this method that was coined by years of development. It's no recent whim nor in any way non-standard.

Quoted 2 lines

tl;dr: I found the current way of handling the mac address very convenient, and I wouldn't like to see it removed.

I'd say, move that to bootloader and don't polute Linux with it. That's where it belongs.

Quoted 1 line

Maxime

Best regards,
Marek Vasut

#7 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Le 19/09/2012 13:25, Marek Vasut a ?crit :

Quoted 48 lines
Quoted 48 lines

Le 19/09/2012 12:50, Marek Vasut a ?crit :

Quoted 34 lines
Quoted 34 lines

Le 19/09/2012 05:15, Shawn Guo a ?crit :

Quoted 32 lines · Marek Vasut
Quoted 4 lines

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Ok, since you are the maintainer of m28evk board, it could be your call to do so. While for imx28-evk, the kernel at least runs with bootlets which is not DT capable.

Quoted 13 lines
Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>

---

 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)
(quoted diff) +0 -0

b/arch/arm/mach-mxs/mach-mxs.c index 170a930..71d47f5 100644

arch/arm/mach-mxs/mach-mxs.c static void __init imx28_evk_post_init(void) · line 260

Quoted 6 lines
arch/arm/mach-mxs/mach-mxs.c +0 -2
@@ -255,8 +255,6 @@ static void __init imx28_evk_post_init(void)
 
 static void __init m28evk_init(void)
 {
 
-	update_fec_mac_prop(OUI_DENX);
-

Since it's the only user of OUI_DENX, can we remove enum mac_oui completely and make update_fec_mac_prop a fsl specific call?

Quoted 4 lines
Quoted 2 lines

I have a patch on the way that adds the Crystalfontz OUI for the CFA-10049, so it will be quite useful to me in the future.

Do you use U-Boot on that board?

No, we use barebox for it. And since the CFA-10049 is an expansion board to the CFA-10036, we only have support for the CFA-10036 in barebox, and support the devices present in the CFA-10049 through loading a different device tree.

So basically, I'd like to avoid to get the mac from the ocotp in barebox, since I won't need it if the CFA-10049 isn't plugged in.

Quoted 7 lines
Quoted 3 lines

Moreover, I've found no clue that barebox adds the local-mac-address or the mac-address fields to the device tree, so this assumption seems quite u-boot specific.

I dunno what barebox is, but it seems really crap. Don't use crap. The "mac- address" and "local-mac-address" nodes must be set up by the bootloader, if barebox or whatever doesn't set it, it's broken.

I'm pretty sure this will bring a nice troll :)

Quoted 4 lines

U-Boot did that since PPC days (so basically all PPCs do that). And the FDT got onto ARM from PPCs. And FEC was used on PPCs too, thus I see no point to diverge on ARM from this method that was coined by years of development. It's no recent whim nor in any way non-standard.

This is a good feature, I don't doubt that. But by removing the ocotp reading from linux, you also remove the ability for a vendor to store the MAC in the OCOTP if it's board uses a bootloader that doesn't read the OCOTP and doesn't handle the device tree properly.

Basically, every bootloader except U-boot. Even the bootlets from Freescale, even another random bootloader using an appended device tree, etc.

I'm sorry, I disagree on the total removal of the update_fec_mac_prop function. But I get your point, and I have no objection to your current patch actually. The only thing I'm saying is please, don't remove it for everyone.

Signature · 4 lines
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#8 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Maxime Ripard,

Quoted 53 lines

Le 19/09/2012 13:25, Marek Vasut a ?crit :

Quoted 52 lines
Quoted 51 lines

Le 19/09/2012 12:50, Marek Vasut a ?crit :

Quoted 34 lines
Quoted 34 lines

Le 19/09/2012 05:15, Shawn Guo a ?crit :

Quoted 32 lines · Marek Vasut
Quoted 4 lines

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Ok, since you are the maintainer of m28evk board, it could be your call to do so. While for imx28-evk, the kernel at least runs with bootlets which is not DT capable.

Quoted 13 lines
Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>

---

 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)
(quoted diff) +0 -0

b/arch/arm/mach-mxs/mach-mxs.c index 170a930..71d47f5 100644

arch/arm/mach-mxs/mach-mxs.c static void __init imx28_evk_post_init(void) · line 260

Quoted 6 lines
arch/arm/mach-mxs/mach-mxs.c +0 -2
@@ -255,8 +255,6 @@ static void __init imx28_evk_post_init(void)
 
 static void __init m28evk_init(void)
 {
 
-	update_fec_mac_prop(OUI_DENX);
-

Since it's the only user of OUI_DENX, can we remove enum mac_oui completely and make update_fec_mac_prop a fsl specific call?

Quoted 4 lines
Quoted 2 lines

I have a patch on the way that adds the Crystalfontz OUI for the CFA-10049, so it will be quite useful to me in the future.

Do you use U-Boot on that board?

No, we use barebox for it. And since the CFA-10049 is an expansion board to the CFA-10036, we only have support for the CFA-10036 in barebox, and support the devices present in the CFA-10049 through loading a different device tree.

So basically, I'd like to avoid to get the mac from the ocotp in barebox, since I won't need it if the CFA-10049 isn't plugged in.

Moreover, I've found no clue that barebox adds the local-mac-address or the mac-address fields to the device tree, so this assumption seems quite u-boot specific.

Quoted 5 lines
Quoted 3 lines

I dunno what barebox is, but it seems really crap. Don't use crap. The "mac- address" and "local-mac-address" nodes must be set up by the bootloader, if barebox or whatever doesn't set it, it's broken.

I'm pretty sure this will bring a nice troll :)

I didn't intend to troll, sorry if it sounded that way.

Quoted 9 lines
Quoted 4 lines

U-Boot did that since PPC days (so basically all PPCs do that). And the FDT got onto ARM from PPCs. And FEC was used on PPCs too, thus I see no point to diverge on ARM from this method that was coined by years of development. It's no recent whim nor in any way non-standard.

This is a good feature, I don't doubt that. But by removing the ocotp reading from linux, you also remove the ability for a vendor to store the MAC in the OCOTP if it's board uses a bootloader that doesn't read the OCOTP and doesn't handle the device tree properly.

Isn't that a bootloader problem?

Quoted 8 lines

Basically, every bootloader except U-boot. Even the bootlets from Freescale, even another random bootloader using an appended device tree, etc.

I'm sorry, I disagree on the total removal of the update_fec_mac_prop function. But I get your point, and I have no objection to your current patch actually. The only thing I'm saying is please, don't remove it for everyone.

Ok, I think we can agree that having the workaround present for broken bootloaders is beneficial. But please, fix the bootloader ASAP.

Off and out.

Best regards,
Marek Vasut

#9 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

In reply to Marek Vasut

Hi Marek,

Quoted 1 line

I dunno what barebox is,

You're pretty bad at lying...

Quoted 1 line

but it seems really crap.

...and marketing, too :) Why the hate?

Regards,

Wolfram

Signature · 9 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120920/c33c45d4/attachment.sig>
#10 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Wolfram Sang,

btw. Wolfram, you are getting pretty personal here. I really wanted to close this thread as it was a closed issue for me, we reached agreement, can we do that or take this nonsense off-list please?

Quoted 5 lines

Hi Marek,

Quoted 1 line

I dunno what barebox is,

You're pretty bad at lying...

I'm still a human, I forget things ... aren't you growing paranoid Wolfram?

Quoted 3 lines
Quoted 1 line

but it seems really crap.

...and marketing, too :) Why the hate?

Marketing and hate has no place in here.

If barebox can't handle even basic DT fixup, it's broken.

Quoted 3 lines

Regards,

Wolfram

Best regards,
Marek Vasut

#11 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Hi Marek,

Quoted 3 lines

btw. Wolfram, you are getting pretty personal here. I really wanted to close this thread as it was a closed issue for me, we reached agreement, can we do that or take this nonsense off-list please?

You ranted, you asked for it :) Given the tone of the rant, I still think my answer was moderate.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

I dunno what barebox is,

You're pretty bad at lying...

I'm still a human, I forget things ... aren't you growing paranoid Wolfram?

Nope. Of course, I can't "prove" that you did not forget, but since I could easily point to a number of threads where you comment on barebox, it is simply not convincing.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

but it seems really crap.

...and marketing, too :) Why the hate?

Marketing and hate has no place in here.

\o/ Happy to hear that.

Quoted 1 line

If barebox can't handle even basic DT fixup, it's broken.

It can. It maybe was just not needed up to now, dunno.

Regards,

Wolfram

Signature · 9 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120920/1f0d1a5b/attachment.sig>
#12 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Wolfram Sang,

Pardon my tone, I wasn't exactly picky about the wording when replying.

[...]

Quoted 3 lines
Quoted 1 line

If barebox can't handle even basic DT fixup, it's broken.

It can. It maybe was just not needed up to now, dunno.

Fix it and send patch, so this problem doesn't spread.

Quoted 3 lines

Regards,

Wolfram

Best regards,
Marek Vasut

#13 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Hi Marek,

Le 20/09/2012 15:10, Marek Vasut a ?crit :

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

If barebox can't handle even basic DT fixup, it's broken.

It can. It maybe was just not needed up to now, dunno.

Fix it and send patch, so this problem doesn't spread.

I'm sorry, but you still miss the point.

If someone wants to use another bootloader than U-boot (or a possible patched barebox), or none other than the bootlets to boot directly the Linux (with an appended device tree), you will still have no way to get the NIC from the OCOTP, and I'm sorry, but it is just wrong.

The kernel shouldn't rely on a particular feature of a given bootloader.

Signature · 4 lines
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#14 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Maxime Ripard,

Quoted 10 lines

Hi Marek,

Le 20/09/2012 15:10, Marek Vasut a ?crit :

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

If barebox can't handle even basic DT fixup, it's broken.

It can. It maybe was just not needed up to now, dunno.

Fix it and send patch, so this problem doesn't spread.

I'm sorry, but you still miss the point.

I do see the point, I'm just trying to understand how to best avoid this issue.

Let's drop the part "remove update_fec_mac_prop()" completely from this discussion, that's not happening and we agree on that.

Quoted 4 lines

If someone wants to use another bootloader than U-boot (or a possible patched barebox), or none other than the bootlets to boot directly the Linux (with an appended device tree), you will still have no way to get the NIC from the OCOTP

I wholeheartedly agree!

Quoted 1 line

and I'm sorry, but it is just wrong.

I've been pondering about this issue for a while actually. Right now, the update_fec_mac_prop() unconditionally overwrites the bootloader-passed setting, yes or am I wrong?

Quoted 1 line

The kernel shouldn't rely on a particular feature of a given bootloader.

But the device tree given to the kernel should be complete, no?

Best regards,
Marek Vasut

#15 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

In reply to Maxime Ripard

Dear Maxime,

Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 2 lines

Fix it and send patch, so this problem doesn't spread.

I'm sorry, but you still miss the point.

I'm not so sure about this.

Quoted 4 lines

If someone wants to use another bootloader than U-boot (or a possible patched barebox), or none other than the bootlets to boot directly the Linux (with an appended device tree), you will still have no way to get the NIC from the OCOTP, and I'm sorry, but it is just wrong.

With device tree supoort enabled, the regular way to pass the MAC address to a network interface (that needs one) is through the device tree.

There are several options how such information gets there. Boot loaders with built-in DT support may fill in the MAC address information into the DT before passing it to the Linux kernel.

If your boot method does not allow such dynamic adjustment, you ould provide a statically configured device tree, which already includes the MAC address.

Quoted 1 line

The kernel shouldn't rely on a particular feature of a given bootloader.

100% agreed. And it does not. It always uses the respective information from the device tree.

So when you use bootlets, all you need to do is to insert the respective MAC address entry to your device tree definition.

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Every program has at least one bug and can be shortened by  at  least
one  instruction  --  from  which,  by induction, one can deduce that
every program can be reduced to one instruction which doesn't work.
#16 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

2012/9/20 Wolfgang Denk <[email protected]>:

Quoted 23 lines

Dear Maxime,

Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 2 lines

Fix it and send patch, so this problem doesn't spread.

I'm sorry, but you still miss the point.

I'm not so sure about this.

Quoted 4 lines

If someone wants to use another bootloader than U-boot (or a possible patched barebox), or none other than the bootlets to boot directly the Linux (with an appended device tree), you will still have no way to get the NIC from the OCOTP, and I'm sorry, but it is just wrong.

With device tree supoort enabled, the regular way to pass the MAC address to a network interface (that needs one) is through the device tree.

There are several options how such information gets there. Boot loaders with built-in DT support may fill in the MAC address information into the DT before passing it to the Linux kernel.

Quoted 8 lines

If your boot method does not allow such dynamic adjustment, you ould provide a statically configured device tree, which already includes the MAC address.

Quoted 1 line

The kernel shouldn't rely on a particular feature of a given bootloader.

100% agreed. And it does not. It always uses the respective information from the device tree.

I thought that device tree should provide information that we can't get by the hardware itslef. Here, if the hardware can provide the MAC address, why should we ignore it?

Quoted 23 lines

So when you use bootlets, all you need to do is to insert the respective MAC address entry to your device tree definition.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Every program has at least one bug and can be shortened by at least one instruction -- from which, by induction, one can deduce that every program can be reduced to one instruction which doesn't work.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 1 line
Gregory CLEMENT
#17 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Gregory CLEMENT,

Quoted 27 lines

2012/9/20 Wolfgang Denk <[email protected]>:

Quoted 26 lines

Dear Maxime,

Quoted 3 lines · In message <[email protected]> you wrote:
Quoted 1 line

Fix it and send patch, so this problem doesn't spread.

I'm sorry, but you still miss the point.

I'm not so sure about this.

Quoted 4 lines

If someone wants to use another bootloader than U-boot (or a possible patched barebox), or none other than the bootlets to boot directly the Linux (with an appended device tree), you will still have no way to get the NIC from the OCOTP, and I'm sorry, but it is just wrong.

With device tree supoort enabled, the regular way to pass the MAC address to a network interface (that needs one) is through the device tree.

There are several options how such information gets there. Boot loaders with built-in DT support may fill in the MAC address information into the DT before passing it to the Linux kernel.

If your boot method does not allow such dynamic adjustment, you ould provide a statically configured device tree, which already includes the MAC address.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

The kernel shouldn't rely on a particular feature of a given bootloader.

100% agreed. And it does not. It always uses the respective information from the device tree.

I thought that device tree should provide information that we can't get by the hardware itslef. Here, if the hardware can provide the MAC address, why should we ignore it?

That is exactly the core problem here.

FTR: The FEC can not provide the MAC address, the hardware does not have any EEPROM or any other means to store it's unique mac address in itself, so it must be supplied from the outside (that is, DT or OCOTP or elsehow).

Quoted 17 lines
Quoted 17 lines

So when you use bootlets, all you need to do is to insert the respective MAC address entry to your device tree definition.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Every program has at least one bug and can be shortened by at least one instruction -- from which, by induction, one can deduce that every program can be reduced to one instruction which doesn't work.

Quoted 4 lines
Quoted 4 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Best regards,
Marek Vasut

#18 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

In reply to Gregory CLEMENT

Dear Gregory,

Quoted 4 lines · In message <CADx9zJCnQCnGgvO6ui7m1buAt2S+OPyQVgyEBwt-u2ARcqh94Q@mail.gmail.com>…
Quoted 4 lines

With device tree supoort enabled, the regular way to pass the MAC address to a network interface (that needs one) is through the device tree.

...

Quoted 3 lines

I thought that device tree should provide information that we can't get by the hardware itslef. Here, if the hardware can provide the MAC address, why should we ignore it?

That was the "that needs one" part above.

Yet, using the DT is still the generic way to do this, and should be fully supported. And assuming there might be simple bootloaders (like bootlets) that are not capable to write such data to any device specific storage, this should probably even be the default.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Mirrors should reflect a little before throwing back images.
- Jean Cocteau
#19 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

In reply to Maxime Ripard

Dear Maxime Ripard,

Quoted 9 lines

Hi Marek,

Le 20/09/2012 15:10, Marek Vasut a ?crit :

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

If barebox can't handle even basic DT fixup, it's broken.

It can. It maybe was just not needed up to now, dunno.

Fix it and send patch, so this problem doesn't spread.

Quoted 8 lines

I'm sorry, but you still miss the point.

If someone wants to use another bootloader than U-boot (or a possible patched barebox), or none other than the bootlets to boot directly the Linux (with an appended device tree), you will still have no way to get the NIC from the OCOTP, and I'm sorry, but it is just wrong.

The kernel shouldn't rely on a particular feature of a given bootloader.

So what do you think about this patch (skip loading from OCOTP if it's already present in DT):

arch/arm/mach-mxs/mach-mxs.c +4 -0
index 71d47f5..4f014ef 100644
@@ -175,6 +175,10 @@ static void __init update_fec_mac_prop(enum mac_oui oui)
                np = of_find_compatible_node(from, NULL, "fsl,imx28-fec");
                if (!np)
                        return;
+
+               if (of_get_property(np, "local-mac-address", NULL))
+                       continue;
+
                from = np;
 
                newmac = kzalloc(sizeof(*newmac) + 6, GFP_KERNEL);

Best regards,
Marek Vasut

#20 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Le 20/09/2012 18:33, Marek Vasut a ?crit :

Quoted 28 lines

Dear Maxime Ripard,

Quoted 9 lines

Hi Marek,

Le 20/09/2012 15:10, Marek Vasut a ?crit :

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

If barebox can't handle even basic DT fixup, it's broken.

It can. It maybe was just not needed up to now, dunno.

Fix it and send patch, so this problem doesn't spread.

Quoted 8 lines

I'm sorry, but you still miss the point.

If someone wants to use another bootloader than U-boot (or a possible patched barebox), or none other than the bootlets to boot directly the Linux (with an appended device tree), you will still have no way to get the NIC from the OCOTP, and I'm sorry, but it is just wrong.

The kernel shouldn't rely on a particular feature of a given bootloader.

So what do you think about this patch (skip loading from OCOTP if it's already present in DT):

arch/arm/mach-mxs/mach-mxs.c +0 -0
index 71d47f5..4f014ef 100644

arch/arm/mach-mxs/mach-mxs.c static void __init update_fec_mac_prop(enum mac_oui oui) · line 184

Quoted 10 lines
arch/arm/mach-mxs/mach-mxs.c +4 -0
@@ -175,6 +175,10 @@ static void __init update_fec_mac_prop(enum mac_oui oui)
                np = of_find_compatible_node(from, NULL, "fsl,imx28-fec");
                if (!np)
                        return;
+
+               if (of_get_property(np, "local-mac-address", NULL))
+                       continue;
+
                from = np;
 
                newmac = kzalloc(sizeof(*newmac) + 6, GFP_KERNEL);

That definitely looks like the best solution, yes.

You can add my Acked-by if you want.

Maxime

Signature · 4 lines
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#21 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

Dear Maxime Ripard,

Quoted 41 lines

Le 20/09/2012 18:33, Marek Vasut a ?crit :

Quoted 40 lines

Dear Maxime Ripard,

Quoted 9 lines

Hi Marek,

Le 20/09/2012 15:10, Marek Vasut a ?crit :

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

If barebox can't handle even basic DT fixup, it's broken.

It can. It maybe was just not needed up to now, dunno.

Fix it and send patch, so this problem doesn't spread.

Quoted 8 lines

I'm sorry, but you still miss the point.

If someone wants to use another bootloader than U-boot (or a possible patched barebox), or none other than the bootlets to boot directly the Linux (with an appended device tree), you will still have no way to get the NIC from the OCOTP, and I'm sorry, but it is just wrong.

The kernel shouldn't rely on a particular feature of a given bootloader.

So what do you think about this patch (skip loading from OCOTP if it's already present in DT):

arch/arm/mach-mxs/mach-mxs.c +4 -0
index 71d47f5..4f014ef 100644
@@ -175,6 +175,10 @@ static void __init update_fec_mac_prop(enum mac_oui
 oui)
 
                np = of_find_compatible_node(from, NULL,
                "fsl,imx28-fec"); if (!np)
 
                        return;
 
+
+               if (of_get_property(np, "local-mac-address", NULL))
+                       continue;
+
Quoted 5 lines
Quoted 3 lines
                from = np;
                
                newmac = kzalloc(sizeof(*newmac) + 6, GFP_KERNEL);

That definitely looks like the best solution, yes.

Two things I'm not sure about:

1) Use of_get_property() for this task or some other of_*() ?
2) I'll send it as a separate patch

Thanks!

Quoted 3 lines

You can add my Acked-by if you want.

Maxime

Best regards,
Marek Vasut

Applied
#22 lore raw

PATCH 1/1 ARM: mxs: m28evk: Disable OCOTP OUI loading

In reply to Marek Vasut
Quoted 8 lines · Marek Vasut

Don't load the FEC MAC address from OCOTP, but use the one supplied via device tree by U-Boot. This is the preferred way, every DT-capable bootloader does set up "mac-address" and "local-mac-address" properties into the DT passed to the kernel.

Cc Fabio Estevam <[email protected]>
Cc Shawn Guo <[email protected]>

Applied, thanks.