r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Francois Romieu]

Linus Torvalds <torvalds@osdl.org> :
[regression related to r8169 MAC address change]
> Francois ? Jeff ?

Go revert it.

Despite what I claimed, I can not find a third-party confirmation by email
that it works elsewhere.

It would probably be enough to remove the call to __rtl8169_set_mac_addr()
in rtl8169_hw_start() though.

In place of the test suggested in bugzilla, I'd rather see Guennadi test
the thing below (acked on netdev by the initial victim if someone wonders
why it has not changed the status of bugzilla so far):

>From f092d907f78e81846dfaf1008a6409c3c4b58f27 Mon Sep 17 00:00:00 2001
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 21 Oct 2006 21:07:36 +0200
Subject: [PATCH] r8169: perform a PHY reset before any other operation at boot time

Realtek's 8139/810x (0x8136) PCI-E comes with a touchy PHY.
A big heavy reset seems to calm it down.

Fix for http://bugzilla.kernel.org/show_bug.cgi?id=7378.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>
---
 drivers/net/r8169.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index f1c7575..927fc17 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1440,6 +1440,22 @@ static void rtl8169_release_board(struct
 	free_netdev(dev);
 }
 
+static void rtl8169_phy_reset(struct net_device *dev,
+			      struct rtl8169_private *tp)
+{
+	void __iomem *ioaddr = tp->mmio_addr;
+	int i;
+
+	tp->phy_reset_enable(ioaddr);
+	for (i = 0; i < 100; i++) {
+		if (!tp->phy_reset_pending(ioaddr))
+			return;
+		msleep(1);
+	}
+	if (netif_msg_link(tp))
+		printk(KERN_ERR "%s: PHY reset failed.\n", dev->name);
+}
+
 static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
 {
 	void __iomem *ioaddr = tp->mmio_addr;
@@ -1468,6 +1484,8 @@ static void rtl8169_init_phy(struct net_
 
 	rtl8169_link_option(board_idx, &autoneg, &speed, &duplex);
 
+	rtl8169_phy_reset(dev, tp);
+
 	rtl8169_set_speed(dev, autoneg, speed, duplex);
 
 	if ((RTL_R8(PHYstatus) & TBI_Enable) && netif_msg_link(tp))
-- 
1.4.2.4

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Guennadi Liakhovetski]

On Sun, 29 Oct 2006, Francois Romieu wrote:

> Linus Torvalds <torvalds@osdl.org> :
> [regression related to r8169 MAC address change]
> > Francois ? Jeff ?
> 
> Go revert it.
> 
> Despite what I claimed, I can not find a third-party confirmation by email
> that it works elsewhere.
> 
> It would probably be enough to remove the call to __rtl8169_set_mac_addr()
> in rtl8169_hw_start() though.
> 
> In place of the test suggested in bugzilla, I'd rather see Guennadi test
> the thing below (acked on netdev by the initial victim if someone wonders
> why it has not changed the status of bugzilla so far):

AFAIU, you wanted it applied on the top of the "non-working" kernel 
(2.6.19-rc2-ish)? No, it didn't work. And, worse yet, I think, it is after 
testing that patch that the interface got into a state, when netconsole 
worked, ping worked, but ssh didn't. A poweroff was needed to recover. In 
case you still need it, here's the info you requested:

00:0b.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8169 Gigabit Ethernet (rev 10)
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 128 (8000ns min, 16000ns max), Cache Line Size: 32 bytes
        Interrupt: pin A routed to IRQ 16
        Region 0: I/O ports at febfff00 [size=256]
        Region 1: Memory at bffffc00 (32-bit, non-prefetchable) [size=256]
        Capabilities: [dc] Power Management version 0
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: ec 10 69 81 17 00 b0 02 10 00 00 02 08 80 00 00
10: 01 ff bf 00 00 fc ff bf 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 dc 00 00 00 00 00 00 00 10 01 20 40

dmesg when it didn't work (I do use netconsole, don't think it matters?):

r8169 Gigabit Ethernet driver 2.2LK loaded
eth0: RTL8169s/8110s at 0xc9004c00, 00:0d:0b:99:44:70, IRQ 16
netconsole: device eth0 not up yet, forcing it
r8169: eth0: link down
r8169: eth0: link up

The same when it's working.

Yes, just commenting out the line

	__rtl8169_set_mac_addr(dev, ioaddr);

fixes it (without the patch from your previous email).

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Francois Romieu]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> :
[...]
> AFAIU, you wanted it applied on the top of the "non-working" kernel 
> (2.6.19-rc2-ish)?

No. Please apply it on top of a 2.6.19-rc3 where the mac address change
feature has been reverted (or where __rtl8169_set_mac_addr has been
commented out at your option). 

[...]
> dmesg when it didn't work (I do use netconsole, don't think it matters?):

I'd rather fix everything else without netconsole first. It will make
my life simpler.

-- 
Ueimor

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Oleg Verych]

On 2006-10-29, Francois Romieu wrote:
>
> Linus Torvalds <torvalds@osdl.org> :
> [regression related to r8169 MAC address change]
>> Francois ? Jeff ?
>
> Go revert it.
>
> Despite what I claimed, I can not find a third-party confirmation by email
> that it works elsewhere.

It works for me:
,--
|root@flower:/home/olecom# ip l set eth0 addr 00:00:00:00:00:02
|root@flower:/home/olecom# ip l set eth0 addr 00:00:00:00:00:03
|root@flower:/home/olecom# ip l set eth0 addr 00:00:00:00:00:04
|root@flower:/home/olecom# ip l set eth0 addr 04:44:44:44:44:04
|root@flower:/home/olecom# ip l show
|1: lo: <LOOPBACK,UP,10000> mtu 16436 qdisc noqueue
|    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
|    2: eth0: <BROADCAST,MULTICAST,UP,10000> mtu 1500 qdisc pfifo_fast qlen 1000
|    link/ether 04:44:44:44:44:04 brd ff:ff:ff:ff:ff:ff
|root@flower:/home/olecom# lspci -v | grep Realtek
|    02:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
|RTL8111/8168B PCI Express Gigabit Ethernet controller (rev 01)
|root@flower:/home/olecom#
|root@flower:/home/olecom# uname -a
|Linux flower 2.6.19-rc2-vanilla-ai #4 SMP PREEMPT Tue Oct 17 02:19:16
|UTC 2006 x86_64 GNU/Linux
|root@flower:/home/olecom#
`--
____

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Guennadi Liakhovetski]

On Mon, 30 Oct 2006, Francois Romieu wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> :
> [...]
> > AFAIU, you wanted it applied on the top of the "non-working" kernel 
> > (2.6.19-rc2-ish)?
> 
> No. Please apply it on top of a 2.6.19-rc3 where the mac address change
> feature has been reverted (or where __rtl8169_set_mac_addr has been
> commented out at your option). 

Ok, with just __rtl8169_set_mac_addr disabled it works. With netconsole 
disabled, and your phy_reset patch applied it seems to still work. The 
printk

+		printk(KERN_ERR "%s: PHY reset failed.\n", dev->name);

doesn't get printed. If I uncomment __rtl8169_set_mac_addr it stops 
working again. What does it tell us about the original set_mac_address 
problem?

I haven't said it's an on-board chip, not a plug-in card. Don't know how 
setting the mac address worked in your configuration, but if it is storred 
in a prom, maybe it is just missing on my board?

The kernel is not 2.6.19-rc3 either. It is a clone of the powerpc git some 
time shortly after 2.6.19-rc2.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Guennadi Liakhovetski]

On Mon, 30 Oct 2006, Guennadi Liakhovetski wrote:

> Ok, with just __rtl8169_set_mac_addr disabled it works. With netconsole 
> disabled, and your phy_reset patch applied it seems to still work. The 

The "seems" above was the key word. Once again I had a case, when after 
re-compiling the kernel again with the disabled call to 
__rtl8169_set_mac_addr only ping worked. And a power-off was required to 
recover. So, that phy_reset doesn't seem to be very safe either.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Francois Romieu]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> :
[...]
> doesn't get printed. If I uncomment __rtl8169_set_mac_addr it stops 
> working again. What does it tell us about the original set_mac_address 
> problem?

Probably that it is issued too early/bluntly. I'll redo it later.

[...]
> The kernel is not 2.6.19-rc3 either. It is a clone of the powerpc git some 
> time shortly after 2.6.19-rc2.

You miss 73f5e28b336772c4b08ee82e5bf28ab872898ee1 and
733b736c91dd2c556f35dffdcf77e667cf10cefc. It should not matter.

-- 
Ueimor

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Francois Romieu]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> :
[...]
> The "seems" above was the key word. Once again I had a case, when after 
> re-compiling the kernel again with the disabled call to 
> __rtl8169_set_mac_addr only ping worked. And a power-off was required to 
> recover. So, that phy_reset doesn't seem to be very safe either.

Can you replace phy_reset by the patch below and try it twice ?

It's interesting to know if it does not always behave the same.

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index f1c7575..4b05dea 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -570,8 +570,8 @@ static void rtl8169_xmii_reset_enable(vo
 {
 	unsigned int val;
 
-	val = (mdio_read(ioaddr, MII_BMCR) | BMCR_RESET) & 0xffff;
-	mdio_write(ioaddr, MII_BMCR, val);
+	mdio_write(ioaddr, MII_BMCR, BMCR_RESET);
+	val = mdio_read(ioaddr, MII_BMCR);
 }
 
 static void rtl8169_check_link_status(struct net_device *dev,
@@ -1440,6 +1440,22 @@ static void rtl8169_release_board(struct
 	free_netdev(dev);
 }
 
+static void rtl8169_phy_reset(struct net_device *dev,
+			      struct rtl8169_private *tp)
+{
+	void __iomem *ioaddr = tp->mmio_addr;
+	int i;
+
+	tp->phy_reset_enable(ioaddr);
+	for (i = 0; i < 100; i++) {
+		if (!tp->phy_reset_pending(ioaddr))
+			return;
+		msleep(1);
+	}
+	if (netif_msg_link(tp))
+		printk(KERN_ERR "%s: PHY reset failed.\n", dev->name);
+}
+
 static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
 {
 	void __iomem *ioaddr = tp->mmio_addr;
@@ -1468,6 +1484,8 @@ static void rtl8169_init_phy(struct net_
 
 	rtl8169_link_option(board_idx, &autoneg, &speed, &duplex);
 
+	rtl8169_phy_reset(dev, tp);
+
 	rtl8169_set_speed(dev, autoneg, speed, duplex);
 
 	if ((RTL_R8(PHYstatus) & TBI_Enable) && netif_msg_link(tp))

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Guennadi Liakhovetski]

On Tue, 31 Oct 2006, Francois Romieu wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> :
> [...]
> > The "seems" above was the key word. Once again I had a case, when after 
> > re-compiling the kernel again with the disabled call to 
> > __rtl8169_set_mac_addr only ping worked. And a power-off was required to 
> > recover. So, that phy_reset doesn't seem to be very safe either.
> 
> Can you replace phy_reset by the patch below and try it twice ?
> 
> It's interesting to know if it does not always behave the same.

Well, with that one I booted 3 times, all 3 times it worked. I'll leave it 
in to see if it ever fails. So, what does it tell us about the 
set_mac_address thing?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Francois Romieu]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> :
[...]
> Well, with that one I booted 3 times, all 3 times it worked. I'll leave it 

Thanks.

Let's cross fingers.

> in to see if it ever fails. So, what does it tell us about the 
> set_mac_address thing?

It tells nothing more about the set_mac_address thing. If people need 
MAC address change support, I can surely hack something and keep a
patch for future reference. Imho it is anything but 2.6.19 material
though.

The patch that I sent to you on 2006/10/29 was enough to fix the link
detection issues experienced with the 0x8136 chipset (1. Darren Salt
on netdev {25/26/31}/08/2006 and {21/22}/10/2006, 2. Syed Azam on BZ,
see http://bugzilla.kernel.org/show_bug.cgi?id=7378).

Your computer was good at spotting issues with the MAC address stuff,
so it was the perfect candidate to test pending fixes for different
problems. As you noticed, it was not exactly safe to feed the MII
control register with some potentially uninitialized stuff, whence
the patch from yesterday.

It remains to be seen if:
- it still does the job for the 0x8136 
- it does not induce new regressions in existing 8169

o Darren and Syed, are your 0x8136 still happy with the patch
  0001-r8169-perform-a-PHY-reset-before-any-other-operation-at-boot-time.txt
  at http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.19-rc4/r8169
  on top of 2.6.19-rc4 ?

o Darren, still ok to keep your S-o-b in it ?

o Could the r8169 users out there check that the same patch does not add
  new regressions to their favorite 2.6.19-rc4 ?

o Lennert, can you apply the two patches
  - 0001-r8169-perform-a-PHY-reset-before-any-other-operation-at-boot-time.txt
  - 0002-r8169-more-magic.txt
  at http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.19-rc4/r8169 against
  2.6.19-rc4 (2.6.19-rc4 reverted the MAC address changes) and see if the
  n2100 board still needs to remove the SYSErr handler ?

-- 
Ueimor

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Guennadi Liakhovetski]

On Wed, 1 Nov 2006, Francois Romieu wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> :
> 
> > in to see if it ever fails. So, what does it tell us about the 
> > set_mac_address thing?
> 
> It tells nothing more about the set_mac_address thing. If people need 
> MAC address change support, I can surely hack something and keep a
> patch for future reference. Imho it is anything but 2.6.19 material
> though.

Aha, ok, thanks. Just noticed that the set_mac_address has been reverted 
in -rc4, so, that's resolved. Good.

> Your computer was good at spotting issues with the MAC address stuff,
> so it was the perfect candidate to test pending fixes for different
> problems. As you noticed, it was not exactly safe to feed the MII
> control register with some potentially uninitialized stuff, whence
> the patch from yesterday.

Glad it was useful. I have to warn you though, that that "computer" is not 
very actively used ATM and doesn't stay on for too long. However, if you 
can suggest a way to stress test that phy reset thingie, I could run some 
overnight test.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Lennert Buytenhek]

On Wed, Nov 01, 2006 at 12:05:38AM +0100, Francois Romieu wrote:

> o Lennert, can you apply the two patches
>   - 0001-r8169-perform-a-PHY-reset-before-any-other-operation-at-boot-time.txt
>   - 0002-r8169-more-magic.txt
>   at http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.19-rc4/r8169 against
>   2.6.19-rc4 (2.6.19-rc4 reverted the MAC address changes) and see if the
>   n2100 board still needs to remove the SYSErr handler ?

2.6.19-rc4 + these two patches => doesn't work

2.6.19-rc4 + these two patches + SYSErr removal => works

For reference:
* 2.6.19-rc4 + SYSErr removal => works

So, while these two patches don't fix the problem, they don't seem
to be making things worse, something the MAC address change did do.


cheers,
Lennert

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Darren Salt]

This message is in MIME format which your mailer apparently does not support.
You either require a newer version of your software which supports MIME, or
a separate MIME decoding utility.  Alternatively, ask the sender of this
message to resend it in a different format.

--163681386--1739281754--1718250983
Content-Type: text/plain; charset=us-ascii

I demand that Francois Romieu may or may not have written...

[snip]
> o Darren and Syed, are your 0x8136 still happy with the patch
>   0001-r8169-perform-a-PHY-reset-before-any-other-operation-at-boot-time.txt
>   at http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.19-rc4/r8169
>   on top of 2.6.19-rc4 ?

It is.

I then tried 0002-r8169-more-magic.txt, mainly to see that it doesn't cause
any problems. Unfortunately, it did... however, a little reading showed that
you've stopped enabling transmit and receive for all but four of the chip
types supported by the driver.

An incremental fix is attached.

> o Darren, still ok to keep your S-o-b in it ?

Yes.

[snip]
-- 
| Darren Salt    | linux or ds at              | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
| + Buy less and make it last longer.         INDUSTRY CAUSES GLOBAL WARMING.

Never have a drink when you are feeling sorry for yourself.

--163681386--1739281754--1718250983
Content-Type: text/plain; charset=iso-8859-1; name="0003-r8169-fix-more-magic.patch"
Content-Disposition: attachment; filename="0003-r8169-fix-more-magic.patch"

r8169: fix more magic so that 8101e etc. work again

r8169-more-magic killed transmit & receive on my laptop's RTL8101e. Turns out
to be that the enabling of these functions had been unitnentionally removed.

(This is one of two possible fixes, the other being the removal of the if()
guarding the other tx/rx-enable call. Both work here.)

Signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>

--- drivers/net/r8169.c~	2006-11-01 19:53:02.000000000 +0000
+++ drivers/net/r8169.c	2006-11-01 19:54:16.000000000 +0000
@@ -1921,7 +1921,10 @@
 	    (tp->mac_version != RTL_GIGA_MAC_VER_02) &&
 	    (tp->mac_version != RTL_GIGA_MAC_VER_03) &&
 	    (tp->mac_version != RTL_GIGA_MAC_VER_04))
+	{
 		rtl8169_set_rx_tx_config_registers(tp);
+		RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+	}
 
 	RTL_W8(Cfg9346, Cfg9346_Lock);
 

--163681386--1739281754--1718250983--

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Francois Romieu]

Darren Salt <linux@youmustbejoking.demon.co.uk> :
[...]
> (This is one of two possible fixes, the other being the removal of the if()
> guarding the other tx/rx-enable call. Both work here.)

I'll update the patch with your change but the removal of the if() would
not match Realtek's init sequence.

Lennert, I have compared 2.6.19-rc4 + 0001-r8169-perform-a-PHY-reset-etc
with the serie of patches against 2.6.18-rc4 which was reported to work
on your n2100 (thread on netdev around 05/09/2006). Can you:

- apply the patch below on top of 2.6.19-rc4 + 0001 and see if it works ?
  Don't apply 0002, it is not required.

- if it works (it should if I have not messed it again), can you check
  that it still works if you do not apply the first hunk ? It should as
  well.

If everything went fine this far, it would be nice to know if both the
move of the write to ChipCmd and the mdio_write are required to fix
your issue (I'd expect the move alone to not be enough as 0002 got this
part right for the 8110sb).

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 50b753d..b2fdbb8 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -491,7 +491,7 @@ static int rtl8169_poll(struct net_devic
 #endif
 
 static const u16 rtl8169_intr_mask =
-	SYSErr | LinkChg | RxOverflow | RxFIFOOver | TxErr | TxOK | RxErr | RxOK;
+	LinkChg | RxOverflow | RxFIFOOver | TxErr | TxOK | RxErr | RxOK;
 static const u16 rtl8169_napi_event =
 	RxOK | RxOverflow | RxFIFOOver | TxOK | TxErr;
 static const unsigned int rtl8169_rx_config =
@@ -1283,11 +1283,6 @@ static void rtl8169_hw_phy_config(struct
 	/* Shazam ! */
 
 	if (tp->mac_version == RTL_GIGA_MAC_VER_04) {
-		mdio_write(ioaddr, 31, 0x0001);
-		mdio_write(ioaddr,  9, 0x273a);
-		mdio_write(ioaddr, 14, 0x7bfb);
-		mdio_write(ioaddr, 27, 0x841e);
-
 		mdio_write(ioaddr, 31, 0x0002);
 		mdio_write(ioaddr,  1, 0x90d0);
 		mdio_write(ioaddr, 31, 0x0000);
@@ -1855,6 +1850,8 @@ rtl8169_hw_start(struct net_device *dev)
 
 
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
+
+	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
 	RTL_W8(EarlyTxThres, EarlyTxThld);
 
 	/* Low hurts. Let's disable the filtering. */
@@ -1895,7 +1892,7 @@ rtl8169_hw_start(struct net_device *dev)
 	RTL_W32(TxDescStartAddrLow, ((u64) tp->TxPhyAddr & DMA_32BIT_MASK));
 	RTL_W32(RxDescAddrHigh, ((u64) tp->RxPhyAddr >> 32));
 	RTL_W32(RxDescAddrLow, ((u64) tp->RxPhyAddr & DMA_32BIT_MASK));
-	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+
 	RTL_W8(Cfg9346, Cfg9346_Lock);
 
 	/* Initially a 10 us delay. Turned it into a PCI commit. - FR */

RE: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)

[Azam, Syed S]

Yes, It is still working.


Syed Azam
System Software Engineer
Hewlett-Packard Company
 
-----Original Message-----
From: Darren Salt [mailto:linux@youmustbejoking.demon.co.uk] 
Sent: Wednesday, November 01, 2006 1:01 PM
To: g.liakhovetski@gmx.de; romieu@fr.zoreil.com
Cc: torvalds@osdl.org; bunk@stusta.de; akpm@osdl.org; jgarzik@pobox.com;
linux-kernel@vger.kernel.org; tmattox@gmail.com; spiky.kiwi@gmail.com;
r.bhatia@ipax.at; Azam, Syed S; buytenh@wantstofly.org
Subject: Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known
regressions)

This message is in MIME format which your mailer apparently does not
support.
You either require a newer version of your software which supports MIME,
or
a separate MIME decoding utility.  Alternatively, ask the sender of this
message to resend it in a different format.

--163681386--1739281754--1718250983
Content-Type: text/plain; charset=us-ascii

I demand that Francois Romieu may or may not have written...

[snip]
> o Darren and Syed, are your 0x8136 still happy with the patch
>
0001-r8169-perform-a-PHY-reset-before-any-other-operation-at-boot-time.t
xt
>   at http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.19-rc4/r8169
>   on top of 2.6.19-rc4 ?

It is.

I then tried 0002-r8169-more-magic.txt, mainly to see that it doesn't
cause
any problems. Unfortunately, it did... however, a little reading showed
that
you've stopped enabling transmit and receive for all but four of the
chip
types supported by the driver.

An incremental fix is attached.

> o Darren, still ok to keep your S-o-b in it ?

Yes.

[snip]
-- 
| Darren Salt    | linux or ds at              | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
| + Buy less and make it last longer.         INDUSTRY CAUSES GLOBAL
WARMING.

Never have a drink when you are feeling sorry for yourself.

--163681386--1739281754--1718250983
Content-Type: text/plain; charset=iso-8859-1;
name="0003-r8169-fix-more-magic.patch"
Content-Disposition: attachment;
filename="0003-r8169-fix-more-magic.patch"

r8169: fix more magic so that 8101e etc. work again

r8169-more-magic killed transmit & receive on my laptop's RTL8101e.
Turns out
to be that the enabling of these functions had been unitnentionally
removed.

(This is one of two possible fixes, the other being the removal of the
if()
guarding the other tx/rx-enable call. Both work here.)

Signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>

--- drivers/net/r8169.c~	2006-11-01 19:53:02.000000000 +0000
+++ drivers/net/r8169.c	2006-11-01 19:54:16.000000000 +0000
@@ -1921,7 +1921,10 @@
 	    (tp->mac_version != RTL_GIGA_MAC_VER_02) &&
 	    (tp->mac_version != RTL_GIGA_MAC_VER_03) &&
 	    (tp->mac_version != RTL_GIGA_MAC_VER_04))
+	{
 		rtl8169_set_rx_tx_config_registers(tp);
+		RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+	}
 
 	RTL_W8(Cfg9346, Cfg9346_Lock);
 

--163681386--1739281754--1718250983--

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)]

[Riku Voipio]

Hi,

> Lennert, I have compared 2.6.19-rc4 + 0001-r8169-perform-a-PHY-reset-etc
> with the serie of patches against 2.6.18-rc4 which was reported to work
> on your n2100 (thread on netdev around 05/09/2006). Can you:
> 
> - apply the patch below on top of 2.6.19-rc4 + 0001 and see if it works ?
>   Don't apply 0002, it is not required.

I took 2.6.19-rc5 as there was no changes in this driver relative to -rc4. 
applied Francois's 0001-r8169-perform-a-PHY-reset.. and finally the
patch in this mail. And networking _does_not_ work on Thecus N2100.

mii-tool sees the link being connected and disconnected, but dhcp or ping
with static ip goes nowhere.

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)]

[Francois Romieu]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

Riku Voipio <riku.voipio@iki.fi> :
[...]
> I took 2.6.19-rc5 as there was no changes in this driver relative to -rc4. 
> applied Francois's 0001-r8169-perform-a-PHY-reset.. and finally the
> patch in this mail. And networking _does_not_ work on Thecus N2100.

It sucks.

Can you try against 2.6.18-rc4 the patch at:

http://www.fr.zoreil.com/people/francois/misc/20061109-2.6.18-rc4-r8169-test.patch

If it does not work, apply on top of 2.6.18-rc4 the serie available at:
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.18-rc4/r8169
plus the attached patch.

-- 
Ueimor

[-- Attachment #2: 0010-r8169-quirk-for-the-8110sb-on-arm-platform.txt --]
[-- Type: text/plain, Size: 1793 bytes --]

>From 20f33886d041e00cde9c6339ff4d724d992c99bc Mon Sep 17 00:00:00 2001
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Mon, 11 Sep 2006 20:10:58 +0200
Subject: [PATCH] r8169: quirk for the 8110sb on arm platform

Inverting the write ordering of the TxDescAddr{High/Low} registers
suffices to trigger a sabbat of PCI errors which make the device
completely dysfunctional. The issue has not been reported on a
different platform.

Switching from MMIO accesses to I/O ones as done in Realtek's
own driver fixes (papers over ?) the bug as well but I am not
thrilled to see everyone pay the I/O price for an obscure bug.

This is the minimal change to handle the issue.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Reported-by: Lennert Buytenhek <buytenh@wantstofly.org>
---
 drivers/net/r8169.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 7ab79db..582b116 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1868,10 +1868,15 @@ rtl8169_hw_start(struct net_device *dev)
 	 */
 	RTL_W16(IntrMitigate, 0x0000);
 
-	RTL_W32(TxDescStartAddrLow, ((u64) tp->TxPhyAddr & DMA_32BIT_MASK));
+	/*
+	 * Magic spell: some iop3xx ARM board needs the TxDescAddrHigh
+	 * register to be written before TxDescAddrLow to work.
+	 * Switching from MMIO to I/O access fixes the issue as well.
+	 */
 	RTL_W32(TxDescStartAddrHigh, ((u64) tp->TxPhyAddr >> 32));
-	RTL_W32(RxDescAddrLow, ((u64) tp->RxPhyAddr & DMA_32BIT_MASK));
+	RTL_W32(TxDescStartAddrLow, ((u64) tp->TxPhyAddr & DMA_32BIT_MASK));
 	RTL_W32(RxDescAddrHigh, ((u64) tp->RxPhyAddr >> 32));
+	RTL_W32(RxDescAddrLow, ((u64) tp->RxPhyAddr & DMA_32BIT_MASK));
 	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
 	RTL_W8(Cfg9346, Cfg9346_Lock);
 
-- 
1.4.2.4

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)]

[Lennert Buytenhek]

On Thu, Nov 09, 2006 at 11:13:38PM +0100, Francois Romieu wrote:

> > I took 2.6.19-rc5 as there was no changes in this driver relative to -rc4. 
> > applied Francois's 0001-r8169-perform-a-PHY-reset.. and finally the
> > patch in this mail. And networking _does_not_ work on Thecus N2100.
> 
> It sucks.
> 
> Can you try against 2.6.18-rc4 the patch at:
> 
> http://www.fr.zoreil.com/people/francois/misc/20061109-2.6.18-rc4-r8169-test.patch
> 
> If it does not work, apply on top of 2.6.18-rc4 the serie available at:
> http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.18-rc4/r8169
> plus the attached patch.

Wouldn't it be easier for all of us if we'd arrange a shell account
on an n2100 for you?

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)]

[Francois Romieu]

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

Lennert Buytenhek <buytenh@wantstofly.org> :
[...]
> Wouldn't it be easier for all of us if we'd arrange a shell account
> on an n2100 for you?

Ssh keyfile attached.

-- 
Ueimor

[-- Attachment #2: r8169-n2100.pub --]
[-- Type: text/plain, Size: 623 bytes --]

ssh-dss AAAAB3NzaC1kc3MAAACBAIQzLEqjuZ7G6v91JxdZ6S2l465mGzirY3qDfGDp9Qfiw8EhrEYrPGAbSnu0iTrBict5ONiVVwlwEbtTE8pJ/YCBqEpU25Im01AmjR4Usg6m20HV+4a9XSahrYvsMSrRJwLhqdp9oMC6XPtMbrq3Lee8UfWHk3xZLG7ekzXO9dOFAAAAFQCNT5WHpMwIUiQHXejPXLltZWA+6wAAAIBx8cOTbjuij12b25S02HM4sGXow32qxDYLLwNLGr4j/pXOX1RHqlvL5Ex4H6nBnYaWXnB9eRkitO3Ho7EUF0tqPbocug9DBa61gHdLhIKJCCUMXbTibVAwkeBV6bXgYGoWgCny80OGkN6b2XVLCWIeXpZEwS3mGHpIny+3LGTvIAAAAIBT2GApaPoj77NI00Pbs/ijofZuhXp0BF7K8zdAc374LWrdrc+ZIsTY8InK1jNAhl8m5LrTaD4cdDdVp2H4UVKsxs71KETmNlbKukU5zQyDZffICZmrkYhbRonE8aei5oYZ0YWUL4i1AbrQYhzS3810QERGyuw0IpespabiyOU4xg== romieu@electric-eye.fr.zoreil.com

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)]

[Martin Michlmayr]

* Francois Romieu <romieu@fr.zoreil.com> [2006-11-10 19:59]:
> > Wouldn't it be easier for all of us if we'd arrange a shell account
> > on an n2100 for you?
> Ssh keyfile attached.

Did you have any success?
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions)]

[Francois Romieu]

Martin Michlmayr <tbm@cyrius.com> :
[...]
> Did you have any success ?

I was not able to reproduce the results expected from a driver close to
the one which was supposed to work back with 2.6.18-rc4. OTOH, as Lennert
noticed, simply ignoring the PCI data parity errors seems to do the trick.
There are quite a lot of these errors though.

I'll do one or two more tests but the driver will surely be simply added
a big knob to control its behavior when parity errors appear.

-- 
Ueimor

r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Francois Romieu]

Francois Romieu <romieu@fr.zoreil.com> :
[scrouitch]

You can apply the patch below and 'modprobe r8169 ignore_parity_err=1'.

It apparently does the job and it is not much too intrusive.

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 27f90b2..2b8c057 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -225,6 +225,7 @@ MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl
 
 static int rx_copybreak = 200;
 static int use_dac;
+static int ignore_parity_err;
 static struct {
 	u32 msg_enable;
 } debug = { -1 };
@@ -469,6 +470,8 @@ module_param(use_dac, int, 0);
 MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI slot.");
 module_param_named(debug, debug.msg_enable, int, 0);
 MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., 16=all)");
+module_param_named(ignore_parity_err, ignore_parity_err, bool, 0);
+MODULE_PARM_DESC(ignore_parity_err, "Ignore PCI parity error as target. Default: false");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(RTL8169_VERSION);
 
@@ -2332,12 +2335,17 @@ static void rtl8169_pcierr_interrupt(str
 	/*
 	 * The recovery sequence below admits a very elaborated explanation:
 	 * - it seems to work;
-	 * - I did not see what else could be done.
+	 * - I did not see what else could be done;
+	 * - it makes iop3xx happy.
 	 *
 	 * Feel free to adjust to your needs.
 	 */
-	pci_write_config_word(pdev, PCI_COMMAND,
-			      pci_cmd | PCI_COMMAND_SERR | PCI_COMMAND_PARITY);
+	if (ignore_parity_err)
+		pci_cmd &= ~PCI_COMMAND_PARITY;
+	else
+		pci_cmd |= PCI_COMMAND_SERR | PCI_COMMAND_PARITY;
+
+	pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);
 
 	pci_write_config_word(pdev, PCI_STATUS,
 		pci_status & (PCI_STATUS_DETECTED_PARITY |
@@ -2351,10 +2359,11 @@ static void rtl8169_pcierr_interrupt(str
 		tp->cp_cmd &= ~PCIDAC;
 		RTL_W16(CPlusCmd, tp->cp_cmd);
 		dev->features &= ~NETIF_F_HIGHDMA;
-		rtl8169_schedule_work(dev, rtl8169_reinit_task);
 	}
 
 	rtl8169_hw_reset(ioaddr);
+
+	rtl8169_schedule_work(dev, rtl8169_reinit_task);
 }
 
 static void

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Martin Michlmayr]

* Francois Romieu <romieu@fr.zoreil.com> [2006-11-23 00:16]:
> It apparently does the job and it is not much too intrusive.

I can confirm that it works.  Do you think there'll be a better fix in
the future?  Do you believe that the boot loader on the N2100 doesn't
initialize Ethernet properly or that this is a generic problem on iop
or with this particular RTL chip?  We have fairly good contacts with
the company producing the N2100 so if it's the former it could
probably be fixed. (Altough I'm not sure this is the case given that
Realtek's driver works).
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Francois Romieu]

Martin Michlmayr <tbm@cyrius.com> :
[...]
> Do you think there'll be a better fix in the future ?

It's the best trade-off that I can figure but there are surely more
knowledgeable people than me. The patch does not completely disable
the reporting of serious PCI errors. If the user knows that it is
otherwise safe, he can disable it: the error will then be reported
only once. I must confess that the history of the 8169 PCI errors is
not crystal clear.

> Do you believe that the boot loader on the N2100 doesn't
> initialize Ethernet properly or that this is a generic problem on iop
> or with this particular RTL chip?  We have fairly good contacts with
> the company producing the N2100 so if it's the former it could
> probably be fixed. (Altough I'm not sure this is the case given that
> Realtek's driver works).

Yes, switching from MM register accesses has been reported to fix/hide
the issue but it's a sledgehammer which does not tell what is going on
(side note: Realtek's driver does not enable the SYSErr irq).

So far I can only tell that 1) the 8169 reports a data parity error on
almost each received packet when it is not silenced 2) the error can
be ignored. If there really is an error and the chipset automatically
retries the transaction, one should expect some loss of efficiency but
it will not necessarily be easy to notice through software.

If I had unlimited resources/time/$$$, I would plug a PCI bus analyzer
and check what is going on. :o)

-- 
Ueimor

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Lennert Buytenhek]

On Thu, Nov 23, 2006 at 12:16:56AM +0100, Francois Romieu wrote:

> You can apply the patch below and 'modprobe r8169 ignore_parity_err=1'.

Is there a way we can have this done by default on the n2100?  I guess
that since it's a PCI device, there isn't much hope for that..?

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Francois Romieu]

Lennert Buytenhek <buytenh@wantstofly.org> :
[...]
> Is there a way we can have this done by default on the n2100?  I guess
> that since it's a PCI device, there isn't much hope for that..?

Do you mean an automagically tuned default value based on CONFIG_ARM ?

-- 
Ueimor

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Lennert Buytenhek]

On Fri, Dec 15, 2006 at 09:15:22PM +0100, Francois Romieu wrote:

> > Is there a way we can have this done by default on the n2100?  I guess
> > that since it's a PCI device, there isn't much hope for that..?
> 
> Do you mean an automagically tuned default value based on CONFIG_ARM ?

No, that wouldn't make sense, that's like making a workaround depend on
arch == i386.

I'm thinking that we should somehow enable this option on the n2100
built-in r8169 ports by default only.  Since the n2100 also has a mini-PCI
slot, and it is in theory possible to put an r8169 on a mini-PCI card,
the workaround probably shouldn't apply to those, so testing for
CONFIG_MACH_N2100 also isn't the right thing to do.

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Russell King]

On Fri, Dec 15, 2006 at 10:03:29PM +0100, Lennert Buytenhek wrote:
> On Fri, Dec 15, 2006 at 09:15:22PM +0100, Francois Romieu wrote:
> 
> > > Is there a way we can have this done by default on the n2100?  I guess
> > > that since it's a PCI device, there isn't much hope for that..?
> > 
> > Do you mean an automagically tuned default value based on CONFIG_ARM ?
> 
> No, that wouldn't make sense, that's like making a workaround depend on
> arch == i386.
> 
> I'm thinking that we should somehow enable this option on the n2100
> built-in r8169 ports by default only.  Since the n2100 also has a mini-PCI
> slot, and it is in theory possible to put an r8169 on a mini-PCI card,
> the workaround probably shouldn't apply to those, so testing for
> CONFIG_MACH_N2100 also isn't the right thing to do.

There is dev->broken_parity_status ... although exactly what the sematics
of that flag actually are seems to be rather vague - there's code which
sets it for the Mellanox Tavor device, but it seems to only be exposed
via sysfs - no code in drivers/pci seems to take any action based upon
this flag being set.

That rather raises the question about the usefulness of that quirk.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Francois Romieu]

Lennert Buytenhek <buytenh@wantstofly.org> :
[...]
> No, that wouldn't make sense, that's like making a workaround depend on
> arch == i386.
> 
> I'm thinking that we should somehow enable this option on the n2100
> built-in r8169 ports by default only.  Since the n2100 also has a mini-PCI
> slot, and it is in theory possible to put an r8169 on a mini-PCI card,
> the workaround probably shouldn't apply to those, so testing for
> CONFIG_MACH_N2100 also isn't the right thing to do.

Ok, I thought it was a useability thing.

Let aside a few configurations, the sensible default value for this
option is not clear. I have no objection against a patch but it seems
a bit academic as long as nobody complains (you can call me lazy too).

-- 
Ueimor

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Lennert Buytenhek]

On Sat, Dec 16, 2006 at 01:54:39AM +0100, Francois Romieu wrote:

> > No, that wouldn't make sense, that's like making a workaround depend on
> > arch == i386.
> > 
> > I'm thinking that we should somehow enable this option on the n2100
> > built-in r8169 ports by default only.  Since the n2100 also has a mini-PCI
> > slot, and it is in theory possible to put an r8169 on a mini-PCI card,
> > the workaround probably shouldn't apply to those, so testing for
> > CONFIG_MACH_N2100 also isn't the right thing to do.
> 
> Ok, I thought it was a useability thing.

It is.


> Let aside a few configurations, the sensible default value for this
> option is not clear. I have no objection against a patch but it seems
> a bit academic as long as nobody complains (you can call me lazy too).

I'm thinking that the entire option is just wrong.  It sucks for
distributors to have to make the choice between having it always on
and having it always off.  It sucks for end users compiling their
own kernels, because their ethernet won't work out of the box, and
they will have no idea what's wrong and what to do.

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Francois Romieu]

Lennert Buytenhek <buytenh@wantstofly.org> :
> On Sat, Dec 16, 2006 at 01:54:39AM +0100, Francois Romieu wrote:
[...]
> I'm thinking that the entire option is just wrong.  It sucks for
> distributors to have to make the choice between having it always on
> and having it always off.  It sucks for end users compiling their
> own kernels, because their ethernet won't work out of the box, and
> they will have no idea what's wrong and what to do.

I would have hoped that it was distributors's job to figure the details
of the configuration at run time from userspace when their package manager
updates the kernel. From there they can set whatever runtime option is
necessary in the configuration file of the modutils package.

However you are right that users may get screwed when they compile their
own kernel though. What about something like the patch below:

Date: Sat, 16 Dec 2006 22:36:06 +0100
Subject: [PATCH] r8169: Aunt Tillie got a Thecus N2100 for Xmas

She compiles kernel again. If only she had been offered an Internet
connexion and an introductory guide to Google too...

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/Kconfig |   11 +++++++++++
 drivers/net/r8169.c |    4 ++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 9de0eed..57432d9 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2085,6 +2085,17 @@ config R8169_VLAN
 	  
 	  If in doubt, say Y.
 
+config R8169_SYSERR_DISABLE
+	bool "Disable reporting of the PCI SysErr"
+	depends on R8169 && ARCH_IOP32X
+	---help---
+	  Without this option, the driver reports a lot of PCI SysErr and
+	  is completely unusable with the Thecus N2100. As an alternate
+	  solution, you can use the 'ignore_parity_err' parameter of the
+	  r8169 module to cure the problem at runtime.
+
+	  If in doubt, say N.
+
 config SIS190
 	tristate "SiS190/SiS191 gigabit ethernet support"
 	depends on PCI
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index f83b41d..5e4149e 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -225,7 +225,11 @@ MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl
 
 static int rx_copybreak = 200;
 static int use_dac;
+#ifdef CONFIG_R8169_SYSERR_DISABLE
+static int ignore_parity_err = 1;
+#else
 static int ignore_parity_err;
+#endif
 static struct {
 	u32 msg_enable;
 } debug = { -1 };
-- 
1.4.4.1

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Lennert Buytenhek]

On Fri, Dec 15, 2006 at 09:14:35PM +0000, Russell King wrote:

> > > > Is there a way we can have this done by default on the n2100?  I guess
> > > > that since it's a PCI device, there isn't much hope for that..?
> > > 
> > > Do you mean an automagically tuned default value based on CONFIG_ARM ?
> > 
> > No, that wouldn't make sense, that's like making a workaround depend on
> > arch == i386.
> > 
> > I'm thinking that we should somehow enable this option on the n2100
> > built-in r8169 ports by default only.  Since the n2100 also has a mini-PCI
> > slot, and it is in theory possible to put an r8169 on a mini-PCI card,
> > the workaround probably shouldn't apply to those, so testing for
> > CONFIG_MACH_N2100 also isn't the right thing to do.
> 
> There is dev->broken_parity_status ... although exactly what the sematics
> of that flag actually are seems to be rather vague - there's code which
> sets it for the Mellanox Tavor device, but it seems to only be exposed
> via sysfs - no code in drivers/pci seems to take any action based upon
> this flag being set.

Sounds good.  How about something like the patch below plus the
corresponding r8169 diff?


Index: linux-2.6.19/arch/arm/mach-iop32x/n2100.c
===================================================================
--- linux-2.6.19.orig/arch/arm/mach-iop32x/n2100.c
+++ linux-2.6.19/arch/arm/mach-iop32x/n2100.c
@@ -123,9 +123,13 @@ static struct hw_pci n2100_pci __initdat
 
 static int __init n2100_pci_init(void)
 {
-	if (machine_is_n2100())
+	if (machine_is_n2100()) {
 		pci_common_init(&n2100_pci);
 
+		pci_get_bus_and_slot(0, 0x08)->broken_parity_status = 1;
+		pci_get_bus_and_slot(0, 0x10)->broken_parity_status = 1;
+	}
+
 	return 0;
 }

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Francois Romieu]

Lennert Buytenhek <buytenh@wantstofly.org> :
[...]
> Sounds good.  How about something like the patch below plus the
> corresponding r8169 diff?

Go wild.

-- 
Ueimor

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Lennert Buytenhek]

On Sun, Dec 17, 2006 at 12:31:34AM +0100, Francois Romieu wrote:

> > Sounds good.  How about something like the patch below plus the
> > corresponding r8169 diff?
> 
> Go wild.

Martin/Riku, I'm pretty busy with other stuff at the moment, can you
give this (on top of 2.6.20-rc1) a spin?  


Index: linux-2.6.19/arch/arm/mach-iop32x/n2100.c
===================================================================
--- linux-2.6.19.orig/arch/arm/mach-iop32x/n2100.c
+++ linux-2.6.19/arch/arm/mach-iop32x/n2100.c
@@ -123,9 +123,13 @@ static struct hw_pci n2100_pci __initdat
 
 static int __init n2100_pci_init(void)
 {
-	if (machine_is_n2100())
+	if (machine_is_n2100()) {
 		pci_common_init(&n2100_pci);
 
+		pci_get_bus_and_slot(0, 0x08)->broken_parity_status = 1;
+		pci_get_bus_and_slot(0, 0x10)->broken_parity_status = 1;
+	}
+
 	return 0;
 }
 
Index: linux-2.6.19/drivers/net/r8169.c
===================================================================
--- linux-2.6.19.orig/drivers/net/r8169.c
+++ linux-2.6.19/drivers/net/r8169.c
@@ -225,7 +225,6 @@ MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl
 
 static int rx_copybreak = 200;
 static int use_dac;
-static int ignore_parity_err;
 static struct {
 	u32 msg_enable;
 } debug = { -1 };
@@ -471,8 +470,6 @@ module_param(use_dac, int, 0);
 MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI slot.");
 module_param_named(debug, debug.msg_enable, int, 0);
 MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., 16=all)");
-module_param_named(ignore_parity_err, ignore_parity_err, bool, 0);
-MODULE_PARM_DESC(ignore_parity_err, "Ignore PCI parity error as target. Default: false");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(RTL8169_VERSION);
 
@@ -2388,7 +2385,7 @@ static void rtl8169_pcierr_interrupt(str
 	 *
 	 * Feel free to adjust to your needs.
 	 */
-	if (ignore_parity_err)
+	if (pdev->broken_parity_status)
 		pci_cmd &= ~PCI_COMMAND_PARITY;
 	else
 		pci_cmd |= PCI_COMMAND_SERR | PCI_COMMAND_PARITY;

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Martin Michlmayr]

* Lennert Buytenhek <buytenh@wantstofly.org> [2006-12-17 00:52]:
> Martin/Riku, I'm pretty busy with other stuff at the moment, can you
> give this (on top of 2.6.20-rc1) a spin?

I'm currently travelling but I'll try in a few days.
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Riku Voipio]

> > bah. 2.6.20-git shows nothing (with or without Lennert's patch) after
> > the following:
 
> > Uncompressing Linux..........................................................................................done, booting the kernel.
> 
> Try the printascii()-in-printk() hack in my svn tree.

Thanks, that was priceless advice. I reverted 
da2c12a279ae225f3d4696f76cb3b32a5bec5bfb "[ARM] Clean up ioremap code"
and n2100 booted fine. I can now confirm Lennerts patch makes 
r8169 work without any module parameter work on Thecus n2100.

With ioremap cleanup code and printascii, the kernel boot looks
like following:

Uncompressing Linux............................................................................
<5>Linux version 2.6.20-rc1n2100 (nchip@watergate-2) (gcc version 4.1.0) #8 Sun Dec 17 22:09:06
CPU: XScale-80219 [69052e30] revision 0 (ARMv5TE), cr=0000397f
Machine: Thecus N2100
Memory policy: ECC disabled, Data cache writeback
<7>On node 0 totalpages: 65536
<7>  DMA zone: 512 pages used for memmap
<7>  DMA zone: 0 pages reserved
<7>  DMA zone: 65024 pages, LIFO batch:15
<7>  Normal zone: 0 pages used for memmap
CPU0: D VIVT undefined 5 cache
CPU0: I cache: 32768 bytes, associativity 32, 32 byte lines, 32 sets
CPU0: D cache: 32768 bytes, associativity 32, 32 byte lines, 32 sets
Built 1 zonelists.  Total pages: 65024
<5>Kernel command line: console=ttyS0,115200 root=/dev/sda1 mem=256M@0xa0000000
PID hash table entries: 1024 (order: 10, 4096 bytes)
Console: colour dummy device 80x30
Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
<6>Memory: 256MB = 256MB total
<5>Memory: 256896KB available (2568K code, 185K data, 104K init)
<7>Calibrating delay loop... 593.10 BogoMIPS (lpj=2965504)
Mount-cache hash table entries: 512
<6>CPU: Testing write buffer coherency: ok
<6>NET: Registered protocol family 16
<6>PCI: bus0: Fast back to back transfers disabled
<5>SCSI subsystem initialized
<7>libata version 2.00 loaded.
<6>usbcore: registered new interface driver usbfs
<6>usbcore: registered new interface driver hub
<6>usbcore: registered new device driver usb
<6>NET: Registered protocol family 2
IP route cache hash table entries: 2048 (order: 1, 8192 bytes)
TCP established hash table entries: 8192 (order: 3, 32768 bytes)
TCP bind hash table entries: 4096 (order: 2, 16384 bytes)
<6>TCP: Hash tables configured (established 8192 bind 4096)
<6>TCP reno registered
<4>NetWinder Floating Point Emulator V0.97 (extended precision)
<6>Installing knfsd (copyright (C) 1996 okir@monad.swb.de).
<6>JFFS2 version 2.2. (NAND) (C) 2001-2006 Red Hat, Inc.
<6>io scheduler noop registered
<6>io scheduler anticipatory registered
<6>io scheduler deadline registered
<6>io scheduler cfq registered (default)
<2>remap_area_pte: page already exists
<2>kernel BUG at arch/arm/mm/ioremap.c:61!
<1>Unable to handle kernel NULL pointer dereference at virtual address 00000000
<1>pgd = c0004000
<1>[00000000] *pgd=00000000
Internal error: Oops: 8f5 [#1]
Modules linked in:
CPU: 0
PC is at __bug+0x20/0x2c
LR is at 0x1
pc : [<c0026988>]    lr : [<00000001>]    Not tainted
sp : c0531ee0  ip : 60000093  fp : c0531eec
r10: d0850000  r9 : d0850000  r8 : 80250000
r7 : d0a00000  r6 : c0521000  r5 : d0850c00  r4 : d0850c00
r3 : 00000000  r2 : 00000000  r1 : 000008ac  r0 : 0000002b
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  Segment kernel
Control: 397F
Table: A0004000  DAC: 00000017
Process swapper (pid: 1, stack limit = 0xc0530250)
Stack: (0xc0531ee0 to 0xc0532000)
1ee0: c0531f38 c0531ef0 c0029ee8 c0026974 d0850c00 00000400 c0007420 000000a3
1f00: c0007420 d0850000 800a0000 800a0000 00000000 c0546400 c0291478 00000000
1f20: 00000000 00000000 00000000 c0531f4c c0531f3c c0029fe0 c0029bf4 c0291470
1f40: c0531f7c c0531f50 c01984dc c0029f8c c0157b54 c0291470 c0546400 c0291478
1f60: 00000000 00000000 00000000 00000000 c0531f98 c0531f80 c01361d0 c0198334
1f80: c0546400 c001e6f8 c0530000 c0531fac c0531f9c c0134f4c c0136140 c001ec84
1fa0: c0531ff4 c0531fb0 c00220c0 c0134f3c 00000000 00000000 c002202c c003f14c
1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
1fe0: 00000000 00000000 00000000 c0531ff8 c003f14c c0022038 00000000 00000000
Backtrace:
[<c0026968>] (__bug+0x0/0x2c) from [<c0029ee8>] (__ioremap_pfn+0x300/0x398)
[<c0029be8>] (__ioremap_pfn+0x0/0x398) from [<c0029fe0>] (__ioremap+0x60/0x6c)
[<c0029f80>] (__ioremap+0x0/0x6c) from [<c01984dc>] (quirk_usb_early_handoff+0x1b4/0x380)
 r4 = C0291470
[<c0198328>] (quirk_usb_early_handoff+0x0/0x380) from [<c01361d0>] (pci_fixup_device+0x9c/0xd4)
[<c0136134>] (pci_fixup_device+0x0/0xd4) from [<c0134f4c>] (pci_init+0x1c/0x3c)
 r6 = C0530000  r5 = C001E6F8  r4 = C0546400
[<c0134f30>] (pci_init+0x0/0x3c) from [<c00220c0>] (init+0x94/0x27c)
 r4 = C001EC84
[<c002202c>] (init+0x0/0x27c) from [<c003f14c>] (do_exit+0x0/0x7e4)
 r7 = 00000000  r6 = 00000000  r5 = 00000000  r4 = 00000000
Code: e1a01000 e59f000c eb00596d e3a03000 (e5833000)
 <0>Kernel panic - not syncing: Attempted to kill init!

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=da2c12a279ae225f3d4696f76cb3b32a5bec5bfb

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Lennert Buytenhek]

On Sun, Dec 17, 2006 at 11:02:10PM +0200, Riku Voipio wrote:

> > > bah. 2.6.20-git shows nothing (with or without Lennert's patch) after
> > > the following:
>  
> > > Uncompressing Linux..........................................................................................done, booting the kernel.
> > 
> > Try the printascii()-in-printk() hack in my svn tree.
> 
> Thanks, that was priceless advice. I reverted 
> da2c12a279ae225f3d4696f76cb3b32a5bec5bfb "[ARM] Clean up ioremap code"
> and n2100 booted fine.

Can you try with da2c12a279ae225f3d4696f76cb3b32a5bec5bfb with the
following patch applied?

	http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4030/1


> I can now confirm Lennerts patch makes r8169 work without any module
> parameter work on Thecus n2100.

Yay.  Aunt Tillie will be so happy.

Francois, if you submit the r8169.c half of that patch, I'll do my half.

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Riku Voipio]

On Sun, Dec 17, 2006 at 10:13:13PM +0100, Lennert Buytenhek wrote:
> On Sun, Dec 17, 2006 at 11:02:10PM +0200, Riku Voipio wrote:
> 
> > > > bah. 2.6.20-git shows nothing (with or without Lennert's patch) after
> > > > the following:
> >  
> > > > Uncompressing Linux..........................................................................................done, booting the kernel.
> > > 
> > > Try the printascii()-in-printk() hack in my svn tree.
> > 
> > Thanks, that was priceless advice. I reverted 
> > da2c12a279ae225f3d4696f76cb3b32a5bec5bfb "[ARM] Clean up ioremap code"
> > and n2100 booted fine.
 
> Can you try with da2c12a279ae225f3d4696f76cb3b32a5bec5bfb with the
> following patch applied?
 
> 	http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4030/1

Boots fine. 

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Francois Romieu]

Lennert Buytenhek <buytenh@wantstofly.org> :
[...]
> Francois, if you submit the r8169.c half of that patch, I'll do my half.

Ok, I'll do it now.

-- 
Ueimor

Re: r8169 on n2100 (was Re: r8169 mac address change (was Re: [0/3] 2.6.19-rc2: known regressions))

[Martin Michlmayr]

* Francois Romieu <romieu@fr.zoreil.com> [2006-12-17 22:48]:
> > Francois, if you submit the r8169.c half of that patch, I'll do my half.
> Ok, I'll do it now.

Thanks to everyone who was involved in coming up with this elegant
solution!
-- 
Martin Michlmayr
http://www.cyrius.com/