DM9000B: Fix PHY power for network down/up

PATCH In review Henry Nestler 3 participants 8 messages Feb 20, 2011 – Feb 23, 2011

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Henry Nestler

author

2011-02-20 21:45 UTC #0 lore raw

PATCH 2/2 DM9000B: Fix PHY power for network down/up

DM9000 revision B needs 1 ms delay after PHY power on (see spec), and PHY power must on in register DM9000_GPR before all other settings will change. Remember, that register DM9000_GPR was not changed by reset sequence.

Without these fix the FIFO goes out of sync and sends wrong data after sequence of "ifconfig ethX down ; sleep 3 ; ifconfig ethX up".
---
Kernel version 2.6.38-rc5

 drivers/net/dm9000.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

drivers/net/dm9000.c +4 -3

index 2d4c4fc..5925569 100644
@@ -802,10 +802,7 @@ dm9000_init_dm9000(struct net_device *dev)
 	/* Checksum mode */
 	dm9000_set_rx_csum_unlocked(dev, db->rx_csum);
 
-	/* GPIO0 on pre-activate PHY */
-	iow(db, DM9000_GPR, 0);	/* REG_1F bit0 activate phyxcer */
 	iow(db, DM9000_GPCR, GPCR_GEP_CNTL);	/* Let GPIO0 output */
-	iow(db, DM9000_GPR, 0);	/* Enable PHY */
 
 	ncr = (db->flags & DM9000_PLATF_EXT_PHY) ? NCR_EXT_PHY : 0;

@@ -1194,6 +1191,10 @@ dm9000_open(struct net_device *dev)
 	if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
 		return -EAGAIN;
 
+	/* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */
+	iow(db, DM9000_GPR, 0);	/* REG_1F bit0 activate phyxcer */
+	udelay(1000); /* delay needs by DM9000B */
+
 	/* Initialize DM9000 board */
 	dm9000_reset(db);
 	dm9000_init_dm9000(dev);

Sergei Shtylyov

2011-02-21 11:14 UTC #1 lore raw

Hello.

Quoted 2 lines · Henry Nestler

DM9000 revision B needs 1 ms delay after PHY power on (see spec), and PHY power must on in register

Couldn't parse that.

Quoted 4 lines

DM9000_GPR before all other settings will change. Remember, that register DM9000_GPR was not changed by reset sequence.
Without these fix the FIFO goes out of sync and sends wrong data after

s/these/this/

Quoted 1 line

sequence of "ifconfig ethX down ; sleep 3 ; ifconfig ethX up".

[...]

Quoted 4 lines

drivers/net/dm9000.c +0 -0
index 2d4c4fc..5925569 100644

[...]

drivers/net/dm9000.c dm9000_open(struct net_device *dev) · line 1196

Quoted 7 lines

drivers/net/dm9000.c +3 -0

@@ -1194,6 +1191,10 @@ dm9000_open(struct net_device *dev)
  	if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
  		return -EAGAIN;
 
+	/* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */
+	iow(db, DM9000_GPR, 0);	/* REG_1F bit0 activate phyxcer */
+	udelay(1000); /* delay needs by DM9000B */

Why not mdelay(1)?

WBR, Sergei

Henry Nestler

author

2011-02-21 21:03 UTC #2 lore raw

Quoted 6 lines · Sergei Shtylyov

Quoted 2 lines · Henry Nestler
DM9000 revision B needs 1 ms delay after PHY power on (see spec), and PHY power must on in register
Couldn't parse that.

This can read in manual DM900B-12-DS-F02 from September 2 2010, Page 14:
"If this Register 1FH bit 0 is updated from '1' to '0', the all Registers can not be accessed within 1ms."

The example driver code waits 2 ms.

Quoted 9 lines

Quoted 4 lines

drivers/net/dm9000.c +0 -0
index 2d4c4fc..5925569 100644

[...]

Quoted 4 lines

drivers/net/dm9000.c +0 -0

@@ -1194,6 +1191,10 @@ dm9000_open(struct net_device *dev)
  	if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
  		return -EAGAIN;

Quoted 5 lines

drivers/net/dm9000.c dm9000_open(struct net_device *dev) · line 1196
Quoted 3 lines
drivers/net/dm9000.c +3 -0
+	/* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */
+	iow(db, DM9000_GPR, 0);	/* REG_1F bit0 activate phyxcer */
+	udelay(1000); /* delay needs by DM9000B */
Why not mdelay(1)?

Because udelay is the base of mdelay.
See include/linux/delay.h:31

#define mdelay(n) ... udelay((n)*1000)

Signature · 1 line

Henry N.

Sergei Shtylyov

2011-02-22 12:36 UTC #3 lore raw

Hello.

Quoted 4 lines · Henry Nestler

Quoted 4 lines
Quoted 2 lines
DM9000 revision B needs 1 ms delay after PHY power on (see spec), and PHY power must on in register
Couldn't parse that.

You seem to have missed a word in your patch description.

Quoted 3 lines

This can read in manual DM900B-12-DS-F02 from September 2 2010, Page 14:
"If this Register 1FH bit 0 is updated from '1' to '0', the all Registers can not be accessed within 1ms."

That I've understood.

Quoted 17 lines

The example driver code waits 2 ms.
Quoted 9 lines
Quoted 4 lines
drivers/net/dm9000.c +0 -0
index 2d4c4fc..5925569 100644
[...]
Quoted 4 lines
drivers/net/dm9000.c +0 -0
@@ -1194,6 +1191,10 @@ dm9000_open(struct net_device *dev)
   	if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
   		return -EAGAIN;
 
Quoted 5 lines
drivers/net/dm9000.c dm9000_open(struct net_device *dev) · line 1196
Quoted 3 lines
drivers/net/dm9000.c +3 -0
+	/* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */
+	iow(db, DM9000_GPR, 0);	/* REG_1F bit0 activate phyxcer */
+	udelay(1000); /* delay needs by DM9000B */
Why not mdelay(1)?

Quoted 4 lines

Because udelay is the base of mdelay.
See include/linux/delay.h:31
#define mdelay(n) ... udelay((n)*1000)

And?

WBR, Sergei

David Miller

maintainer

2011-02-22 18:23 UTC #4 lore raw

In reply to Henry Nestler

From: Henry Nestler <[email protected]>
Date: Mon, 21 Feb 2011 22:03:31 +0100

Quoted 29 lines · Sergei Shtylyov

Quoted 6 lines · Henry Nestler
Quoted 3 lines
DM9000 revision B needs 1 ms delay after PHY power on (see spec), and PHY power must on in register
Couldn't parse that.
This can read in manual DM900B-12-DS-F02 from September 2 2010, Page 14:
"If this Register 1FH bit 0 is updated from '1' to '0', the all Registers can not be accessed within 1ms."
The example driver code waits 2 ms.
Quoted 9 lines
Quoted 4 lines
drivers/net/dm9000.c +0 -0
index 2d4c4fc..5925569 100644
[...]
Quoted 4 lines
drivers/net/dm9000.c +0 -0
@@ -1194,6 +1191,10 @@ dm9000_open(struct net_device *dev)
  	if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
  		return -EAGAIN;
 
Quoted 5 lines
drivers/net/dm9000.c dm9000_open(struct net_device *dev) · line 1196
Quoted 3 lines
drivers/net/dm9000.c +3 -0
+	/* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */
+	iow(db, DM9000_GPR, 0);	/* REG_1F bit0 activate phyxcer */
+	udelay(1000); /* delay needs by DM9000B */
Why not mdelay(1)?

Quoted 4 lines

Because udelay is the base of mdelay.
See include/linux/delay.h:31
#define mdelay(n) ... udelay((n)*1000)

He is telling you to use mdelay(1) because it's clearer. Please do so.

Henry Nestler

author

2011-02-22 19:58 UTC #5 lore raw

Hello David,

Quoted 9 lines · David Miller

From: Henry Nestler <[email protected]>
Date: Mon, 21 Feb 2011 22:03:31 +0100
Quoted 6 lines · Sergei Shtylyov
Quoted 4 lines · Henry Nestler
drivers/net/dm9000.c dm9000_open(struct net_device *dev) · line 1196
Quoted 1 line
drivers/net/dm9000.c +1 -0
+	udelay(1000); /* delay needs by DM9000B */
Why not mdelay(1)?

Quoted 7 lines

Quoted 4 lines
Because udelay is the base of mdelay.
See include/linux/delay.h:31
#define mdelay(n) ... udelay((n)*1000)
He is telling you to use mdelay(1) because it's clearer. Please do so.

Thanks, that text explains it better. No problem. I will resend this patch.

Signature · 1 line

Henry N.

Henry Nestler

author

2011-02-22 21:29 UTC #6 lore raw

PATCH v2 2/2 DM9000B: Fix PHY power for network down/up

In reply to David Miller

DM9000 revision B needs 1 ms delay after PHY power-on. PHY must be powered on by writing 0 into register DM9000_GPR before all other settings will change (see Davicom spec and example code).

Remember, that register DM9000_GPR was not changed by reset sequence.

Without this fix the FIFO is out of sync and sends wrong data after sequence of "ifconfig ethX down ; ifconfig ethX up".
---
Kernel version 2.6.38-rc5

 drivers/net/dm9000.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

drivers/net/dm9000.c +4 -3

index 2d4c4fc..5925569 100644
@@ -802,10 +802,7 @@ dm9000_init_dm9000(struct net_device *dev)
 	/* Checksum mode */
 	dm9000_set_rx_csum_unlocked(dev, db->rx_csum);
 
-	/* GPIO0 on pre-activate PHY */
-	iow(db, DM9000_GPR, 0);	/* REG_1F bit0 activate phyxcer */
 	iow(db, DM9000_GPCR, GPCR_GEP_CNTL);	/* Let GPIO0 output */
-	iow(db, DM9000_GPR, 0);	/* Enable PHY */
 
 	ncr = (db->flags & DM9000_PLATF_EXT_PHY) ? NCR_EXT_PHY : 0;

@@ -1194,6 +1191,10 @@ dm9000_open(struct net_device *dev)
 	if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
 		return -EAGAIN;
 
+	/* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */
+	iow(db, DM9000_GPR, 0);	/* REG_1F bit0 activate phyxcer */
+	mdelay(1); /* delay needs by DM9000B */
+
 	/* Initialize DM9000 board */
 	dm9000_reset(db);
 	dm9000_init_dm9000(dev);

David Miller

maintainer Applied

2011-02-23 22:29 UTC #7 lore raw

From: Henry Nestler <[email protected]>
Date: Tue, 22 Feb 2011 22:29:42 +0100

Quoted 8 lines

DM9000 revision B needs 1 ms delay after PHY power-on. PHY must be powered on by writing 0 into register DM9000_GPR before all other settings will change (see Davicom spec and example code).
Remember, that register DM9000_GPR was not changed by reset sequence.
Without this fix the FIFO is out of sync and sends wrong data after sequence of "ifconfig ethX down ; ifconfig ethX up".

I've applied both of your patches, thanks.