[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

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

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/mach-mxs/mach-mxs.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index 170a930..71d47f5 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -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;
-- 
1.7.10.4

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Shawn Guo]

On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
> 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.

> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> ---
>  arch/arm/mach-mxs/mach-mxs.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
> index 170a930..71d47f5 100644
> --- a/arch/arm/mach-mxs/mach-mxs.c
> +++ b/arch/arm/mach-mxs/mach-mxs.c
> @@ -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

>  	mxsfb_pdata.mode_list = m28evk_video_modes;
>  	mxsfb_pdata.mode_count = ARRAY_SIZE(m28evk_video_modes);
>  	mxsfb_pdata.default_bpp = 16;
> -- 
> 1.7.10.4
> 

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Shawn Guo,

> On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
> > 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

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Maxime Ripard]

Hi Shawn,

Le 19/09/2012 05:15, Shawn Guo a ?crit :
> On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
>> 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.
> 
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> ---
>>  arch/arm/mach-mxs/mach-mxs.c |    2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
>> index 170a930..71d47f5 100644
>> --- a/arch/arm/mach-mxs/mach-mxs.c
>> +++ b/arch/arm/mach-mxs/mach-mxs.c
>> @@ -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?

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


-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Maxime Ripard,

> Hi Shawn,
> 
> Le 19/09/2012 05:15, Shawn Guo a ?crit :
> > On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
> >> 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.
> > 
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> >> Cc: Shawn Guo <shawn.guo@linaro.org>
> >> ---
> >> 
> >>  arch/arm/mach-mxs/mach-mxs.c |    2 --
> >>  1 file changed, 2 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
> >> index 170a930..71d47f5 100644
> >> --- a/arch/arm/mach-mxs/mach-mxs.c
> >> +++ b/arch/arm/mach-mxs/mach-mxs.c
> >> @@ -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?
> 
> 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?

> Maxime

Best regards,
Marek Vasut

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Maxime Ripard]

Hi Marek,

Le 19/09/2012 12:50, Marek Vasut a ?crit :
>> Le 19/09/2012 05:15, Shawn Guo a ?crit :
>>> On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
>>>> 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.
>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>>>> Cc: Shawn Guo <shawn.guo@linaro.org>
>>>> ---
>>>>
>>>>  arch/arm/mach-mxs/mach-mxs.c |    2 --
>>>>  1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
>>>> index 170a930..71d47f5 100644
>>>> --- a/arch/arm/mach-mxs/mach-mxs.c
>>>> +++ b/arch/arm/mach-mxs/mach-mxs.c
>>>> @@ -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?
>>
>> 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

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Maxime Ripard,

> Hi Marek,
> 
> Le 19/09/2012 12:50, Marek Vasut a ?crit :
> >> Le 19/09/2012 05:15, Shawn Guo a ?crit :
> >>> On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
> >>>> 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.
> >>> 
> >>>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> >>>> Cc: Shawn Guo <shawn.guo@linaro.org>
> >>>> ---
> >>>> 
> >>>>  arch/arm/mach-mxs/mach-mxs.c |    2 --
> >>>>  1 file changed, 2 deletions(-)
> >>>> 
> >>>> diff --git a/arch/arm/mach-mxs/mach-mxs.c
> >>>> b/arch/arm/mach-mxs/mach-mxs.c index 170a930..71d47f5 100644
> >>>> --- a/arch/arm/mach-mxs/mach-mxs.c
> >>>> +++ b/arch/arm/mach-mxs/mach-mxs.c
> >>>> @@ -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?
> >> 
> >> 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.

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.

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

> Maxime

Best regards,
Marek Vasut

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Maxime Ripard]

Le 19/09/2012 13:25, Marek Vasut a ?crit :
>> Le 19/09/2012 12:50, Marek Vasut a ?crit :
>>>> Le 19/09/2012 05:15, Shawn Guo a ?crit :
>>>>> On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
>>>>>> 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.
>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>>>>>> Cc: Shawn Guo <shawn.guo@linaro.org>
>>>>>> ---
>>>>>>
>>>>>>  arch/arm/mach-mxs/mach-mxs.c |    2 --
>>>>>>  1 file changed, 2 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-mxs/mach-mxs.c
>>>>>> b/arch/arm/mach-mxs/mach-mxs.c index 170a930..71d47f5 100644
>>>>>> --- a/arch/arm/mach-mxs/mach-mxs.c
>>>>>> +++ b/arch/arm/mach-mxs/mach-mxs.c
>>>>>> @@ -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?
>>>>
>>>> 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.
> 
> 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 :)

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

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Maxime Ripard,

> Le 19/09/2012 13:25, Marek Vasut a ?crit :
> >> Le 19/09/2012 12:50, Marek Vasut a ?crit :
> >>>> Le 19/09/2012 05:15, Shawn Guo a ?crit :
> >>>>> On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
> >>>>>> 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.
> >>>>> 
> >>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>>>>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> >>>>>> Cc: Shawn Guo <shawn.guo@linaro.org>
> >>>>>> ---
> >>>>>> 
> >>>>>>  arch/arm/mach-mxs/mach-mxs.c |    2 --
> >>>>>>  1 file changed, 2 deletions(-)
> >>>>>> 
> >>>>>> diff --git a/arch/arm/mach-mxs/mach-mxs.c
> >>>>>> b/arch/arm/mach-mxs/mach-mxs.c index 170a930..71d47f5 100644
> >>>>>> --- a/arch/arm/mach-mxs/mach-mxs.c
> >>>>>> +++ b/arch/arm/mach-mxs/mach-mxs.c
> >>>>>> @@ -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?
> >>>> 
> >>>> 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.
> > 
> > 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.

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

> 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

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Shawn Guo]

On Wed, Sep 19, 2012 at 12:37:17AM +0200, Marek Vasut wrote:
> 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.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>

Applied, thanks.

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Wolfram Sang]

Hi Marek,

> I dunno what barebox is,

You're pretty bad at lying...

> but it seems really crap.

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

Regards,

   Wolfram

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

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

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?

> Hi Marek,
> 
> > 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?

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

> Regards,
> 
>    Wolfram

Best regards,
Marek Vasut

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Wolfram Sang]

Hi Marek,

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

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

> > > but it seems really crap.
> > 
> > ...and marketing, too :) Why the hate?
> 
> Marketing and hate has no place in here.

\o/ Happy to hear that.

> 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

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

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Wolfram Sang,

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

[...]

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

> Regards,
> 
>    Wolfram

Best regards,
Marek Vasut

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Maxime Ripard]

Hi Marek,

Le 20/09/2012 15:10, Marek Vasut a ?crit :
>>> 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.

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Maxime Ripard,

> Hi Marek,
> 
> Le 20/09/2012 15:10, Marek Vasut a ?crit :
> >>> 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.

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

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

> 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

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Wolfgang Denk]

Dear Maxime,

In message <505B1FC2.2090609@free-electrons.com> you wrote:
> 
> > 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.

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

> 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

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

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Gregory CLEMENT]

2012/9/20 Wolfgang Denk <wd@denx.de>:
> Dear Maxime,
>
> In message <505B1FC2.2090609@free-electrons.com> you wrote:
>>
>> > 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.
>
>> 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.
>
>> 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?

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



-- 
Gregory CLEMENT

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Gregory CLEMENT,

> 2012/9/20 Wolfgang Denk <wd@denx.de>:
> > Dear Maxime,
> > 
> > In message <505B1FC2.2090609@free-electrons.com> you wrote:
> >> > 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.
> > 
> >> 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.
> > 
> >> 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).

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

Best regards,
Marek Vasut

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Maxime Ripard,

> Hi Marek,
> 
> Le 20/09/2012 15:10, Marek Vasut a ?crit :
> >>> 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.

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

diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index 71d47f5..4f014ef 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -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

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Maxime Ripard]

Le 20/09/2012 18:33, Marek Vasut a ?crit :
> Dear Maxime Ripard,
> 
>> Hi Marek,
>>
>> Le 20/09/2012 15:10, Marek Vasut a ?crit :
>>>>> 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.
> 
> So what do you think about this patch (skip loading from OCOTP if it's already 
> present in DT):
> 
> diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
> index 71d47f5..4f014ef 100644
> --- a/arch/arm/mach-mxs/mach-mxs.c
> +++ b/arch/arm/mach-mxs/mach-mxs.c
> @@ -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


-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Wolfgang Denk]

Dear Gregory,

In message <CADx9zJCnQCnGgvO6ui7m1buAt2S+OPyQVgyEBwt-u2ARcqh94Q@mail.gmail.com> you wrote:
>
> > 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.
...
> 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

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

[PATCH] ARM: mxs: m28evk: Disable OCOTP OUI loading

[Marek Vasut]

Dear Maxime Ripard,

> Le 20/09/2012 18:33, Marek Vasut a ?crit :
> > Dear Maxime Ripard,
> > 
> >> Hi Marek,
> >> 
> >> Le 20/09/2012 15:10, Marek Vasut a ?crit :
> >>>>> 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.
> > 
> > So what do you think about this patch (skip loading from OCOTP if it's
> > already present in DT):
> > 
> > diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
> > index 71d47f5..4f014ef 100644
> > --- a/arch/arm/mach-mxs/mach-mxs.c
> > +++ b/arch/arm/mach-mxs/mach-mxs.c
> > @@ -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.

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!

> You can add my Acked-by if you want.
> 
> Maxime

Best regards,
Marek Vasut