All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mips/alchemy: define eth platform devices in the correct order
@ 2010-07-17 14:38 Wolfgang Grandegger
  2010-07-17 17:01   ` Manuel Lauss
  2010-07-23 16:34 ` Ralf Baechle
  0 siblings, 2 replies; 6+ messages in thread
From: Wolfgang Grandegger @ 2010-07-17 14:38 UTC (permalink / raw)
  To: linux-mips; +Cc: Wolfgang Grandegger

From: Wolfgang Grandegger <wg@denx.de>

Currently, the eth devices are probed in the inverse order, first
au1xxx_eth1_device and then au1xxx_eth0_device. On the GPR board,
this makes trouble:

  # ifconfig|grep HWaddr
  eth0      Link encap:Ethernet  HWaddr 00:50:C2:0C:30:01
  eth1      Link encap:Ethernet  HWaddr 66:22:01:80:38:10

A bogous ethernet hwaddr is assigned to the first device and
au1xxx_eth0_device is mapped to eth1, which even does not work
properly. With this patch, the problems are gone:

  # ifconfig|grep HWaddr
  eth0      Link encap:Ethernet  HWaddr 66:22:11:32:38:10
  eth1      Link encap:Ethernet  HWaddr 66:22:11:32:38:11

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
---
 arch/mips/alchemy/common/platform.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
index 2580e77..f9e5622 100644
--- a/arch/mips/alchemy/common/platform.c
+++ b/arch/mips/alchemy/common/platform.c
@@ -435,20 +435,21 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
 static int __init au1xxx_platform_init(void)
 {
 	unsigned int uartclk = get_au1x00_uart_baud_base() * 16;
-	int i;
+	int err, i;
 
 	/* Fill up uartclk. */
 	for (i = 0; au1x00_uart_data[i].flags; i++)
 		au1x00_uart_data[i].uartclk = uartclk;
 
+	err = platform_add_devices(au1xxx_platform_devices,
+				   ARRAY_SIZE(au1xxx_platform_devices));
 #ifndef CONFIG_SOC_AU1100
 	/* Register second MAC if enabled in pinfunc */
-	if (!(au_readl(SYS_PINFUNC) & (u32)SYS_PF_NI2))
+	if (!err && !(au_readl(SYS_PINFUNC) & (u32)SYS_PF_NI2))
 		platform_device_register(&au1xxx_eth1_device);
 #endif
 
-	return platform_add_devices(au1xxx_platform_devices,
-				    ARRAY_SIZE(au1xxx_platform_devices));
+	return err;
 }
 
 arch_initcall(au1xxx_platform_init);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mips/alchemy: define eth platform devices in the correct  order
@ 2010-07-17 17:01   ` Manuel Lauss
  0 siblings, 0 replies; 6+ messages in thread
From: Manuel Lauss @ 2010-07-17 17:01 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: linux-mips, Wolfgang Grandegger

Servus Wolfgang,

On Sat, Jul 17, 2010 at 4:38 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> From: Wolfgang Grandegger <wg@denx.de>
>
> Currently, the eth devices are probed in the inverse order, first
> au1xxx_eth1_device and then au1xxx_eth0_device. On the GPR board,
> this makes trouble:
>
>  # ifconfig|grep HWaddr
>  eth0      Link encap:Ethernet  HWaddr 00:50:C2:0C:30:01
>  eth1      Link encap:Ethernet  HWaddr 66:22:01:80:38:10
>
> A bogous ethernet hwaddr is assigned to the first device and
> au1xxx_eth0_device is mapped to eth1, which even does not work
> properly. With this patch, the problems are gone:
>
>  # ifconfig|grep HWaddr
>  eth0      Link encap:Ethernet  HWaddr 66:22:11:32:38:10
>  eth1      Link encap:Ethernet  HWaddr 66:22:11:32:38:11

Interesting.  I don't disagree with the patch; what do you think about
passing MAC address via platform_data?   I don't particularly like
how the driver is trying to get a MAC address using the prom interface.

I'll try to cook something up.

Manuel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mips/alchemy: define eth platform devices in the correct order
@ 2010-07-17 17:01   ` Manuel Lauss
  0 siblings, 0 replies; 6+ messages in thread
From: Manuel Lauss @ 2010-07-17 17:01 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: linux-mips, Wolfgang Grandegger

Servus Wolfgang,

On Sat, Jul 17, 2010 at 4:38 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> From: Wolfgang Grandegger <wg@denx.de>
>
> Currently, the eth devices are probed in the inverse order, first
> au1xxx_eth1_device and then au1xxx_eth0_device. On the GPR board,
> this makes trouble:
>
>  # ifconfig|grep HWaddr
>  eth0      Link encap:Ethernet  HWaddr 00:50:C2:0C:30:01
>  eth1      Link encap:Ethernet  HWaddr 66:22:01:80:38:10
>
> A bogous ethernet hwaddr is assigned to the first device and
> au1xxx_eth0_device is mapped to eth1, which even does not work
> properly. With this patch, the problems are gone:
>
>  # ifconfig|grep HWaddr
>  eth0      Link encap:Ethernet  HWaddr 66:22:11:32:38:10
>  eth1      Link encap:Ethernet  HWaddr 66:22:11:32:38:11

Interesting.  I don't disagree with the patch; what do you think about
passing MAC address via platform_data?   I don't particularly like
how the driver is trying to get a MAC address using the prom interface.

I'll try to cook something up.

Manuel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mips/alchemy: define eth platform devices in the correct order
  2010-07-17 17:01   ` Manuel Lauss
  (?)
@ 2010-07-17 17:17   ` Wolfgang Grandegger
  2010-07-17 17:37     ` Florian Fainelli
  -1 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Grandegger @ 2010-07-17 17:17 UTC (permalink / raw)
  To: Manuel Lauss; +Cc: linux-mips, Wolfgang Grandegger

On 07/17/2010 07:01 PM, Manuel Lauss wrote:
> Servus Wolfgang,
> 
> On Sat, Jul 17, 2010 at 4:38 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> From: Wolfgang Grandegger <wg@denx.de>
>>
>> Currently, the eth devices are probed in the inverse order, first
>> au1xxx_eth1_device and then au1xxx_eth0_device. On the GPR board,
>> this makes trouble:
>>
>>  # ifconfig|grep HWaddr
>>  eth0      Link encap:Ethernet  HWaddr 00:50:C2:0C:30:01
>>  eth1      Link encap:Ethernet  HWaddr 66:22:01:80:38:10
>>
>> A bogous ethernet hwaddr is assigned to the first device and
>> au1xxx_eth0_device is mapped to eth1, which even does not work
>> properly. With this patch, the problems are gone:
>>
>>  # ifconfig|grep HWaddr
>>  eth0      Link encap:Ethernet  HWaddr 66:22:11:32:38:10
>>  eth1      Link encap:Ethernet  HWaddr 66:22:11:32:38:11
> 
> Interesting.  I don't disagree with the patch; what do you think about
> passing MAC address via platform_data?   I don't particularly like
> how the driver is trying to get a MAC address using the prom interface.

Well, I don't think it's a good idea. Each board should have a different
mac address and it's nomally stored somewhere in the boards non-volatile
storage during board bringup.

> I'll try to cook something up.

But not via platform data, please. Or have I missed something. With the
flat device tree (as used for PowerPC) the situation is different
because the boot-loader can fixup the MAC address before booting Linux.

Wolfgang.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mips/alchemy: define eth platform devices in the correct order
  2010-07-17 17:17   ` Wolfgang Grandegger
@ 2010-07-17 17:37     ` Florian Fainelli
  0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2010-07-17 17:37 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: Manuel Lauss, linux-mips, Wolfgang Grandegger

Hi 

Le Saturday 17 July 2010 19:17:52, Wolfgang Grandegger a écrit :
> On 07/17/2010 07:01 PM, Manuel Lauss wrote:
> > Servus Wolfgang,
> > 
> > On Sat, Jul 17, 2010 at 4:38 PM, Wolfgang Grandegger <wg@grandegger.com> 
wrote:
> >> From: Wolfgang Grandegger <wg@denx.de>
> >> 
> >> Currently, the eth devices are probed in the inverse order, first
> >> au1xxx_eth1_device and then au1xxx_eth0_device. On the GPR board,
> >> 
> >> this makes trouble:
> >>  # ifconfig|grep HWaddr
> >>  eth0      Link encap:Ethernet  HWaddr 00:50:C2:0C:30:01
> >>  eth1      Link encap:Ethernet  HWaddr 66:22:01:80:38:10
> >> 
> >> A bogous ethernet hwaddr is assigned to the first device and
> >> au1xxx_eth0_device is mapped to eth1, which even does not work

Most likely prom_get_ethernet_addr is failing for the first device because 
pdev->id == 1 that is why you get such an address, take a look at au1000-
eth.c.

> >> 
> >> properly. With this patch, the problems are gone:
> >>  # ifconfig|grep HWaddr
> >>  eth0      Link encap:Ethernet  HWaddr 66:22:11:32:38:10
> >>  eth1      Link encap:Ethernet  HWaddr 66:22:11:32:38:11
> > 
> > Interesting.  I don't disagree with the patch; what do you think about
> > passing MAC address via platform_data?   I don't particularly like
> > how the driver is trying to get a MAC address using the prom interface.

The patch is actually good, because there are no reasons to register the 
second MAC before the first one, sorry about that. However, the real fix also 
involves au1000-eth,c in au1000_probe(). We currently only handle the case 
where the pdev->id is 0, not 1. When pdev->id == 1 you end up defaulting to 
the default au1000-eth defined MAC address. So I would do it that way:

- make the board code get the ethernet MAC address for the given adapter
- pass it via platform_data
- au1000-eth checks for its validity or generates a random one

> 
> Well, I don't think it's a good idea. Each board should have a different
> mac address and it's nomally stored somewhere in the boards non-volatile
> storage during board bringup.
> 
> > I'll try to cook something up.
> 
> But not via platform data, please. Or have I missed something. With the
> flat device tree (as used for PowerPC) the situation is different
> because the boot-loader can fixup the MAC address before booting Linux.

MIPS has no mainlined support for DT yet, but still, you would have to cope 
with existing boards running YAMON as a bootloader, so using prom_getenv() is 
imho a good solution.
--
Florian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mips/alchemy: define eth platform devices in the correct order
  2010-07-17 14:38 [PATCH] mips/alchemy: define eth platform devices in the correct order Wolfgang Grandegger
  2010-07-17 17:01   ` Manuel Lauss
@ 2010-07-23 16:34 ` Ralf Baechle
  1 sibling, 0 replies; 6+ messages in thread
From: Ralf Baechle @ 2010-07-23 16:34 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: linux-mips, Wolfgang Grandegger

On Sat, Jul 17, 2010 at 04:38:48PM +0200, Wolfgang Grandegger wrote:

Thanks, applied.

  Ralf

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-07-23 16:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-17 14:38 [PATCH] mips/alchemy: define eth platform devices in the correct order Wolfgang Grandegger
2010-07-17 17:01 ` Manuel Lauss
2010-07-17 17:01   ` Manuel Lauss
2010-07-17 17:17   ` Wolfgang Grandegger
2010-07-17 17:37     ` Florian Fainelli
2010-07-23 16:34 ` Ralf Baechle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.