linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] 8139cp: set ring address before enabling receiver
@ 2012-06-01  4:19 Jason Wang
  2012-06-01  4:19 ` [PATCH 2/2] 8139cp/8139too: terminate the eeprom access with the right opmode Jason Wang
  2012-06-01 18:23 ` [PATCH 1/2] 8139cp: set ring address before enabling receiver David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Jason Wang @ 2012-06-01  4:19 UTC (permalink / raw)
  To: netdev, davem, linux-kernel; +Cc: mst

Currently, we enable the receiver before setting the ring address which could
lead the card DMA into unexpected areas. Solving this by set the ring address
before enabling the receiver.

btw. I find and test this in qemu as I didn't have a 8139cp card in hand. please
review it carefully.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/ethernet/realtek/8139cp.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 5eef290..7f08779 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -979,6 +979,17 @@ static void cp_init_hw (struct cp_private *cp)
 	cpw32_f (MAC0 + 0, le32_to_cpu (*(__le32 *) (dev->dev_addr + 0)));
 	cpw32_f (MAC0 + 4, le32_to_cpu (*(__le32 *) (dev->dev_addr + 4)));
 
+	cpw32_f(HiTxRingAddr, 0);
+	cpw32_f(HiTxRingAddr + 4, 0);
+
+	ring_dma = cp->ring_dma;
+	cpw32_f(RxRingAddr, ring_dma & 0xffffffff);
+	cpw32_f(RxRingAddr + 4, (ring_dma >> 16) >> 16);
+
+	ring_dma += sizeof(struct cp_desc) * CP_RX_RING_SIZE;
+	cpw32_f(TxRingAddr, ring_dma & 0xffffffff);
+	cpw32_f(TxRingAddr + 4, (ring_dma >> 16) >> 16);
+
 	cp_start_hw(cp);
 	cpw8(TxThresh, 0x06); /* XXX convert magic num to a constant */
 
@@ -992,17 +1003,6 @@ static void cp_init_hw (struct cp_private *cp)
 
 	cpw8(Config5, cpr8(Config5) & PMEStatus);
 
-	cpw32_f(HiTxRingAddr, 0);
-	cpw32_f(HiTxRingAddr + 4, 0);
-
-	ring_dma = cp->ring_dma;
-	cpw32_f(RxRingAddr, ring_dma & 0xffffffff);
-	cpw32_f(RxRingAddr + 4, (ring_dma >> 16) >> 16);
-
-	ring_dma += sizeof(struct cp_desc) * CP_RX_RING_SIZE;
-	cpw32_f(TxRingAddr, ring_dma & 0xffffffff);
-	cpw32_f(TxRingAddr + 4, (ring_dma >> 16) >> 16);
-
 	cpw16(MultiIntr, 0);
 
 	cpw8_f(Cfg9346, Cfg9346_Lock);


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

* [PATCH 2/2] 8139cp/8139too: terminate the eeprom access with the right opmode
  2012-06-01  4:19 [PATCH 1/2] 8139cp: set ring address before enabling receiver Jason Wang
@ 2012-06-01  4:19 ` Jason Wang
  2012-06-01 18:23   ` David Miller
  2012-06-01 18:23 ` [PATCH 1/2] 8139cp: set ring address before enabling receiver David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Jason Wang @ 2012-06-01  4:19 UTC (permalink / raw)
  To: netdev, davem, linux-kernel; +Cc: mst

Currently, we terminate the eeprom access through clearing the CS by:

RTL_W8 (Cfg9346, ~EE_CS); or writeb (~EE_CS, ee_addr);

This would left the eeprom into "Config. Register Write Enable:"
state which is not expcted as the highest two bits were set to
0x11 ( expected is the "Normal" mode (0x00)). Solving this by write
0x0 instead of ~EE_CS when terminating the eeprom access.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/ethernet/realtek/8139cp.c  |    2 +-
 drivers/net/ethernet/realtek/8139too.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 7f08779..995d0cf 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -1636,7 +1636,7 @@ static void eeprom_cmd(void __iomem *ee_addr, int cmd, int cmd_len)
 
 static void eeprom_cmd_end(void __iomem *ee_addr)
 {
-	writeb (~EE_CS, ee_addr);
+	writeb(0, ee_addr);
 	eeprom_delay ();
 }
 
diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 03df076..1d83565 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -1173,7 +1173,7 @@ static int __devinit read_eeprom (void __iomem *ioaddr, int location, int addr_l
 	}
 
 	/* Terminate the EEPROM access. */
-	RTL_W8 (Cfg9346, ~EE_CS);
+	RTL_W8(Cfg9346, 0);
 	eeprom_delay ();
 
 	return retval;


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

* Re: [PATCH 1/2] 8139cp: set ring address before enabling receiver
  2012-06-01  4:19 [PATCH 1/2] 8139cp: set ring address before enabling receiver Jason Wang
  2012-06-01  4:19 ` [PATCH 2/2] 8139cp/8139too: terminate the eeprom access with the right opmode Jason Wang
@ 2012-06-01 18:23 ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2012-06-01 18:23 UTC (permalink / raw)
  To: jasowang; +Cc: netdev, linux-kernel, mst

From: Jason Wang <jasowang@redhat.com>
Date: Fri, 01 Jun 2012 12:19:39 +0800

> Currently, we enable the receiver before setting the ring address which could
> lead the card DMA into unexpected areas. Solving this by set the ring address
> before enabling the receiver.
> 
> btw. I find and test this in qemu as I didn't have a 8139cp card in hand. please
> review it carefully.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I hate bugs like this, the other common issue is enabling interrupts
before the chip is fully programmed.

Applied, thanks.

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

* Re: [PATCH 2/2] 8139cp/8139too: terminate the eeprom access with the right opmode
  2012-06-01  4:19 ` [PATCH 2/2] 8139cp/8139too: terminate the eeprom access with the right opmode Jason Wang
@ 2012-06-01 18:23   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2012-06-01 18:23 UTC (permalink / raw)
  To: jasowang; +Cc: netdev, linux-kernel, mst

From: Jason Wang <jasowang@redhat.com>
Date: Fri, 01 Jun 2012 12:19:48 +0800

> Currently, we terminate the eeprom access through clearing the CS by:
> 
> RTL_W8 (Cfg9346, ~EE_CS); or writeb (~EE_CS, ee_addr);
> 
> This would left the eeprom into "Config. Register Write Enable:"
> state which is not expcted as the highest two bits were set to
> 0x11 ( expected is the "Normal" mode (0x00)). Solving this by write
> 0x0 instead of ~EE_CS when terminating the eeprom access.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied.

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

end of thread, other threads:[~2012-06-01 18:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-01  4:19 [PATCH 1/2] 8139cp: set ring address before enabling receiver Jason Wang
2012-06-01  4:19 ` [PATCH 2/2] 8139cp/8139too: terminate the eeprom access with the right opmode Jason Wang
2012-06-01 18:23   ` David Miller
2012-06-01 18:23 ` [PATCH 1/2] 8139cp: set ring address before enabling receiver David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).