Skip to contentSkip to conversation

PowerPC 4xx: ibm_newemac 440GX phy clock workaround.

RFC PATCH In review Valentine Barshak 6 participants 39 messages Feb 21, 2008 – Apr 12, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 PowerPC 4xx: ibm_newemac 440GX phy clock workaround.

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) and because of that it can't find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips.

---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)
drivers/net/ibm_newemac/core.c +15 -1
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c
@@ -43,6 +43,8 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/uaccess.h>
+#include <asm/dcr.h>
+#include <asm/dcr-regs.h>
 
 #include "core.h"
 
@@ -2480,8 +2482,11 @@ static int __devinit emac_init_config(st
 	}
 
 	/* Check EMAC version */
-	if (of_device_is_compatible(np, "ibm,emac4"))
+	if (of_device_is_compatible(np, "ibm,emac4")) {
 		dev->features |= EMAC_FTR_EMAC4;
+		if (of_device_is_compatible(np, "ibm,emac-440gx"))
+			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	}
 
 	/* Fixup some feature bits based on the device tree */
 	if (of_get_property(np, "has-inverted-stacr-oc", NULL))
@@ -2673,8 +2678,17 @@ static int __devinit emac_probe(struct o
 	dev->stop_timeout = STOP_TIMEOUT_100;
 	INIT_DELAYED_WORK(&dev->link_work, emac_link_timer);
 
+	/* Enable internal phy clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) | SDR0_MFR_ECS);
+
 	/* Find PHY if any */
 	err = emac_init_phy(dev);
+
+	/* Enable external phy clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) & ~SDR0_MFR_ECS);
+
 	if (err != 0)
 		goto err_detach_tah;
 
drivers/net/ibm_newemac/core.h +6 -2
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h
@@ -301,6 +301,10 @@ struct emac_instance {
  * Set if we have new type STACR with STAOPC
  */
 #define EMAC_FTR_HAS_NEW_STACR		0x00000040
+/*
+ * Set if we need phy clock workaround for 440gx
+ */
+#define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -312,8 +316,8 @@ enum {
 
 	EMAC_FTRS_POSSIBLE	=
 #ifdef CONFIG_IBM_NEW_EMAC_EMAC4
-	    EMAC_FTR_EMAC4	| EMAC_FTR_HAS_NEW_STACR	|
-	    EMAC_FTR_STACR_OC_INVERT	|
+	    EMAC_FTR_EMAC4 | EMAC_FTR_HAS_NEW_STACR |
+	    EMAC_FTR_STACR_OC_INVERT | EMAC_FTR_440GX_PHY_CLK_FIX |
 #endif
 #ifdef CONFIG_IBM_NEW_EMAC_TAH
 	    EMAC_FTR_HAS_TAH	|
#1 lore raw
Quoted 5 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) and because of that it can't find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips.

The main problem is that will force clock on -all- EMACs ... which can be a problem as they can be in probe at the same time. Might be worth also adding a global mutex around that block.

Also, would you mind having a look at the other workaround for the similar bug ?

Cheers,
Ben.

Quoted 74 lines
---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c

drivers/net/ibm_newemac/core.c +15 -1
@@ -43,6 +43,8 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/uaccess.h>
+#include <asm/dcr.h>
+#include <asm/dcr-regs.h>
 
 #include "core.h"
 
@@ -2480,8 +2482,11 @@ static int __devinit emac_init_config(st
 	}
 
 	/* Check EMAC version */
-	if (of_device_is_compatible(np, "ibm,emac4"))
+	if (of_device_is_compatible(np, "ibm,emac4")) {
 		dev->features |= EMAC_FTR_EMAC4;
+		if (of_device_is_compatible(np, "ibm,emac-440gx"))
+			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	}
 
 	/* Fixup some feature bits based on the device tree */
 	if (of_get_property(np, "has-inverted-stacr-oc", NULL))
@@ -2673,8 +2678,17 @@ static int __devinit emac_probe(struct o
 	dev->stop_timeout = STOP_TIMEOUT_100;
 	INIT_DELAYED_WORK(&dev->link_work, emac_link_timer);
 
+	/* Enable internal phy clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) | SDR0_MFR_ECS);
+
 	/* Find PHY if any */
 	err = emac_init_phy(dev);
+
+	/* Enable external phy clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) & ~SDR0_MFR_ECS);
+
 	if (err != 0)
 		goto err_detach_tah;
 

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h

drivers/net/ibm_newemac/core.h +6 -2
@@ -301,6 +301,10 @@ struct emac_instance {
  * Set if we have new type STACR with STAOPC
  */
 #define EMAC_FTR_HAS_NEW_STACR		0x00000040
+/*
+ * Set if we need phy clock workaround for 440gx
+ */
+#define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -312,8 +316,8 @@ enum {
 
 	EMAC_FTRS_POSSIBLE	=
 #ifdef CONFIG_IBM_NEW_EMAC_EMAC4
-	    EMAC_FTR_EMAC4	| EMAC_FTR_HAS_NEW_STACR	|
-	    EMAC_FTR_STACR_OC_INVERT	|
+	    EMAC_FTR_EMAC4 | EMAC_FTR_HAS_NEW_STACR |
+	    EMAC_FTR_STACR_OC_INVERT | EMAC_FTR_440GX_PHY_CLK_FIX |
 #endif
 #ifdef CONFIG_IBM_NEW_EMAC_TAH
 	    EMAC_FTR_HAS_TAH	|
#2 lore raw
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 5 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) and because of that it can't find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips.

The main problem is that will force clock on -all- EMACs ... which can be a problem as they can be in probe at the same time. Might be worth also adding a global mutex around that block.

I've tried to move clock selection inside the global emac_phy_map_lock block. That works fine for 440GX.

Quoted 3 lines

Also, would you mind having a look at the other workaround for the similar bug?

OK, I'll add 440EP/440GR workaround, but I'm not sure if we need it for
the currently supported boards. The older ibm_emac driver also has a workaround for it on 405ep. This part uses CPC0 dcr registers. So, looks like we'll have to search device tree for the CPC0 entry in ibm_newemac driver, map dcr registers and select clock with dcr_read/write calls. I've omitted the 405ep part, since there's currently no board supported.

Thanks,
Valentine.

Quoted 79 lines

Cheers,
Ben.

Quoted 74 lines
---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c

drivers/net/ibm_newemac/core.c +15 -1
@@ -43,6 +43,8 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/uaccess.h>
+#include <asm/dcr.h>
+#include <asm/dcr-regs.h>
 
 #include "core.h"
 
@@ -2480,8 +2482,11 @@ static int __devinit emac_init_config(st
 	}
 
 	/* Check EMAC version */
-	if (of_device_is_compatible(np, "ibm,emac4"))
+	if (of_device_is_compatible(np, "ibm,emac4")) {
 		dev->features |= EMAC_FTR_EMAC4;
+		if (of_device_is_compatible(np, "ibm,emac-440gx"))
+			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	}
 
 	/* Fixup some feature bits based on the device tree */
 	if (of_get_property(np, "has-inverted-stacr-oc", NULL))
@@ -2673,8 +2678,17 @@ static int __devinit emac_probe(struct o
 	dev->stop_timeout = STOP_TIMEOUT_100;
 	INIT_DELAYED_WORK(&dev->link_work, emac_link_timer);
 
+	/* Enable internal phy clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) | SDR0_MFR_ECS);
+
 	/* Find PHY if any */
 	err = emac_init_phy(dev);
+
+	/* Enable external phy clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) & ~SDR0_MFR_ECS);
+
 	if (err != 0)
 		goto err_detach_tah;
 

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h

drivers/net/ibm_newemac/core.h +6 -2
@@ -301,6 +301,10 @@ struct emac_instance {
  * Set if we have new type STACR with STAOPC
  */
 #define EMAC_FTR_HAS_NEW_STACR		0x00000040
+/*
+ * Set if we need phy clock workaround for 440gx
+ */
+#define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -312,8 +316,8 @@ enum {
 
 	EMAC_FTRS_POSSIBLE	=
 #ifdef CONFIG_IBM_NEW_EMAC_EMAC4
-	    EMAC_FTR_EMAC4	| EMAC_FTR_HAS_NEW_STACR	|
-	    EMAC_FTR_STACR_OC_INVERT	|
+	    EMAC_FTR_EMAC4 | EMAC_FTR_HAS_NEW_STACR |
+	    EMAC_FTR_STACR_OC_INVERT | EMAC_FTR_440GX_PHY_CLK_FIX |
 #endif
 #ifdef CONFIG_IBM_NEW_EMAC_TAH
 	    EMAC_FTR_HAS_TAH	|
#3 lore raw
Quoted 19 lines · On Fri, 22 Feb 2008 22:15:58 +0300 Valentine Barshak <[email protected]>…
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 5 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) and because of that it can't find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips.

The main problem is that will force clock on -all- EMACs ... which can be a problem as they can be in probe at the same time. Might be worth also adding a global mutex around that block.

I've tried to move clock selection inside the global emac_phy_map_lock block. That works fine for 440GX.

Quoted 3 lines

Also, would you mind having a look at the other workaround for the similar bug?

Quoted 6 lines

OK, I'll add 440EP/440GR workaround, but I'm not sure if we need it for the currently supported boards. The older ibm_emac driver also has a workaround for it on 405ep. This part uses CPC0 dcr registers. So, looks like we'll have to search device tree for the CPC0 entry in ibm_newemac driver, map dcr registers and select clock with dcr_read/write calls. I've omitted the 405ep part, since there's currently no board supported.

I'll be doing a 405EP port for .26.

josh

#4 lore raw

RFC PATCH 1/1 ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

In reply to Benjamin Herrenschmidt

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) and because of that it can't find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)
drivers/net/ibm_newemac/core.c +15 -1
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c
@@ -43,6 +43,8 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/uaccess.h>
+#include <asm/dcr.h>
+#include <asm/dcr-regs.h>
 
 #include "core.h"
 
@@ -2323,6 +2325,10 @@ static int __devinit emac_init_phy(struc
 	dev->phy.mdio_read = emac_mdio_read;
 	dev->phy.mdio_write = emac_mdio_write;
 
+	/* Enable internal clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) | SDR0_MFR_ECS);
+
 	/* Configure EMAC with defaults so we can at least use MDIO
 	 * This is needed mostly for 440GX
 	 */
@@ -2355,6 +2361,11 @@ static int __devinit emac_init_phy(struc
 			if (!emac_mii_phy_probe(&dev->phy, i))
 				break;
 		}
+
+	/* Enable external clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) & ~SDR0_MFR_ECS);
+
 	mutex_unlock(&emac_phy_map_lock);
 	if (i == 0x20) {
 		printk(KERN_WARNING "%s: can't find PHY!\n", np->full_name);
@@ -2480,8 +2491,11 @@ static int __devinit emac_init_config(st
 	}
 
 	/* Check EMAC version */
-	if (of_device_is_compatible(np, "ibm,emac4"))
+	if (of_device_is_compatible(np, "ibm,emac4")) {
 		dev->features |= EMAC_FTR_EMAC4;
+		if (of_device_is_compatible(np, "ibm,emac-440gx"))
+			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	}
 
 	/* Fixup some feature bits based on the device tree */
 	if (of_get_property(np, "has-inverted-stacr-oc", NULL))
drivers/net/ibm_newemac/core.h +6 -2
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h
@@ -301,6 +301,10 @@ struct emac_instance {
  * Set if we have new type STACR with STAOPC
  */
 #define EMAC_FTR_HAS_NEW_STACR		0x00000040
+/*
+ * Set if we need phy clock workaround for 440gx
+ */
+#define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -312,8 +316,8 @@ enum {
 
 	EMAC_FTRS_POSSIBLE	=
 #ifdef CONFIG_IBM_NEW_EMAC_EMAC4
-	    EMAC_FTR_EMAC4	| EMAC_FTR_HAS_NEW_STACR	|
-	    EMAC_FTR_STACR_OC_INVERT	|
+	    EMAC_FTR_EMAC4 | EMAC_FTR_HAS_NEW_STACR |
+	    EMAC_FTR_STACR_OC_INVERT | EMAC_FTR_440GX_PHY_CLK_FIX |
 #endif
 #ifdef CONFIG_IBM_NEW_EMAC_TAH
 	    EMAC_FTR_HAS_TAH	|
#5 lore raw

RFC PATCH 1/1 ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

In reply to Benjamin Herrenschmidt

This patch adds ibm_newemac phy clock workaround for 440EP/440GR emacs. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock spearately as opposed to global clock selection for 440GX.

---
 drivers/net/ibm_newemac/core.c |   46 +++++++++++++++++++++++++++++++++++++++--
 drivers/net/ibm_newemac/core.h |    6 ++++-
 2 files changed, 49 insertions(+), 3 deletions(-)
drivers/net/ibm_newemac/core.c +44 -2
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c
@@ -129,10 +129,41 @@ static struct device_node *emac_boot_lis
 static inline void emac_report_timeout_error(struct emac_instance *dev,
 					     const char *error)
 {
-	if (net_ratelimit())
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX |
+				  EMAC_FTR_440EP_PHY_CLK_FIX))
+		DBG(dev, "%s" NL, error);
+	else if (net_ratelimit())
 		printk(KERN_ERR "%s: %s\n", dev->ndev->name, error);
 }
 
+/* EMAC PHY clock workaround:
+ * 440EP/440GR has more sane SDR0_MFR register implementation than 440GX,
+ * which allows controlling each EMAC clock
+ */
+static inline void emac_rx_clk_tx(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) |
+					(SDR0_MFR_ECS >> dev->cell_index));
+		local_irq_restore(flags);
+	}
+}
+
+static inline void emac_rx_clk_default(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) &
+					~(SDR0_MFR_ECS >> dev->cell_index));
+		local_irq_restore(flags);
+	}
+}
+
 /* PHY polling intervals */
 #define PHY_POLL_LINK_ON	HZ
 #define PHY_POLL_LINK_OFF	(HZ / 5)
@@ -1089,9 +1120,11 @@ static int emac_open(struct net_device *
 		int link_poll_interval;
 		if (dev->phy.def->ops->poll_link(&dev->phy)) {
 			dev->phy.def->ops->read_link(&dev->phy);
+			emac_rx_clk_default(dev);
 			netif_carrier_on(dev->ndev);
 			link_poll_interval = PHY_POLL_LINK_ON;
 		} else {
+			emac_rx_clk_tx(dev);
 			netif_carrier_off(dev->ndev);
 			link_poll_interval = PHY_POLL_LINK_OFF;
 		}
@@ -1169,6 +1202,7 @@ static void emac_link_timer(struct work_
 
 	if (dev->phy.def->ops->poll_link(&dev->phy)) {
 		if (!netif_carrier_ok(dev->ndev)) {
+			emac_rx_clk_default(dev);
 			/* Get new link parameters */
 			dev->phy.def->ops->read_link(&dev->phy);
 
@@ -1181,6 +1215,7 @@ static void emac_link_timer(struct work_
 		link_poll_interval = PHY_POLL_LINK_ON;
 	} else {
 		if (netif_carrier_ok(dev->ndev)) {
+			emac_rx_clk_tx(dev);
 			netif_carrier_off(dev->ndev);
 			netif_tx_disable(dev->ndev);
 			emac_reinitialize(dev);
@@ -2325,9 +2360,12 @@ static int __devinit emac_init_phy(struc
 	dev->phy.mdio_read = emac_mdio_read;
 	dev->phy.mdio_write = emac_mdio_write;
 
-	/* Enable internal clock source */
+	/* EMAC PHY clock workaround */
 	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		/* Enable internal clock source */
 		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) | SDR0_MFR_ECS);
+	else
+		emac_rx_clk_tx(dev);
 
 	/* Configure EMAC with defaults so we can at least use MDIO
 	 * This is needed mostly for 440GX
@@ -2495,6 +2533,10 @@ static int __devinit emac_init_config(st
 		dev->features |= EMAC_FTR_EMAC4;
 		if (of_device_is_compatible(np, "ibm,emac-440gx"))
 			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	} else {
+		if (of_device_is_compatible(np, "ibm,emac-440ep") ||
+		    of_device_is_compatible(np, "ibm,emac-440gr"))
+			dev->features |= EMAC_FTR_440EP_PHY_CLK_FIX;
 	}
 
 	/* Fixup some feature bits based on the device tree */
drivers/net/ibm_newemac/core.h +5 -1
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h
@@ -305,6 +305,10 @@ struct emac_instance {
  * Set if we need phy clock workaround for 440gx
  */
 #define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
+/*
+ * Set if we need phy clock workaround for 440ep or 440gr
+ */
+#define EMAC_FTR_440EP_PHY_CLK_FIX	0x00000100
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -328,7 +332,7 @@ enum {
 #ifdef CONFIG_IBM_NEW_EMAC_RGMII
 	    EMAC_FTR_HAS_RGMII	|
 #endif
-	    0,
+	EMAC_FTR_440EP_PHY_CLK_FIX,
 };
 
 static inline int emac_has_feature(struct emac_instance *dev,
#6 lore raw

Re: [RFC][PATCH] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 7 lines · On Fri, 22 Feb 2008 22:28:17 +0300 Valentine Barshak <[email protected]>…

This patch adds ibm_newemac phy clock workaround for 440EP/440GR emacs. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock spearately as opposed to global clock selection for 440GX.

---

[snip]

Quoted 17 lines
drivers/net/ibm_newemac/core.c +17 -0
+/* EMAC PHY clock workaround:
+ * 440EP/440GR has more sane SDR0_MFR register implementation than 440GX,
+ * which allows controlling each EMAC clock
+ */
+static inline void emac_rx_clk_tx(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) |
+					(SDR0_MFR_ECS >> dev->cell_index));
+		local_irq_restore(flags);
+	}
+}
+
+static inline void emac_rx_clk_default(struct emac_instance *dev)

drivers/net/ibm_newemac/core.c static struct device_node *emac_boot_lis · line 165

Quoted 10 lines
drivers/net/ibm_newemac/core.c +10 -0
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) &
+					~(SDR0_MFR_ECS >> dev->cell_index));
+		local_irq_restore(flags);
+	}
+}

Why did you do local_irq_save in these two functions? mtdcri already does spin_lock_irqsave...

josh

#7 lore raw

Re: [RFC][PATCH] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 44 lines · Josh Boyer
Quoted 7 lines · On Fri, 22 Feb 2008 22:28:17 +0300 Valentine Barshak <[email protected]>…

This patch adds ibm_newemac phy clock workaround for 440EP/440GR emacs. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock spearately as opposed to global clock selection for 440GX.

---

[snip]

Quoted 17 lines
drivers/net/ibm_newemac/core.c +17 -0
+/* EMAC PHY clock workaround:
+ * 440EP/440GR has more sane SDR0_MFR register implementation than 440GX,
+ * which allows controlling each EMAC clock
+ */
+static inline void emac_rx_clk_tx(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) |
+					(SDR0_MFR_ECS >> dev->cell_index));
+		local_irq_restore(flags);
+	}
+}
+
+static inline void emac_rx_clk_default(struct emac_instance *dev)

drivers/net/ibm_newemac/core.c static struct device_node *emac_boot_lis · line 165

Quoted 10 lines
drivers/net/ibm_newemac/core.c +10 -0
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) &
+					~(SDR0_MFR_ECS >> dev->cell_index));
+		local_irq_restore(flags);
+	}
+}

Why did you do local_irq_save in these two functions? mtdcri already does spin_lock_irqsave...

Quoted 1 line

josh

Oops, this got copy-pasted from the older ibm_emac. Thanks,
Valentine.

#8 lore raw

Re: [RFC][PATCH] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 16 lines
Quoted 16 lines

drivers/net/ibm_newemac/core.c static struct device_node *emac_boot_lis · line 165

Quoted 13 lines
drivers/net/ibm_newemac/core.c +11 -0
+static inline void emac_rx_clk_default(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) &
+					~(SDR0_MFR_ECS >> dev->cell_index));
+		local_irq_restore(flags);
+	}
+}

Why did you do local_irq_save in these two functions? mtdcri already does spin_lock_irqsave...

Quoted 7 lines
Quoted 2 lines

josh

Oops, this got copy-pasted from the older ibm_emac. Thanks,
Valentine.
_______________________________________________

When I saw Josh's question, I thought the irq save/restore was there to make the read-modify-write atomic; i.e. read SDR0_MFR, "AND" out some bits, then write it back without the possibility of anything else touching SDR0_MFR. I'm just starting to get familiar with the kernel, so if you have a chance, please help educate me. Does the irq lock in mtdcri protect the read-modify-write? Or maybe this R-M-W doesn't need protecting?

Steve

#9 lore raw

Re: [RFC][PATCH] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 4 lines · On Tue, 26 Feb 2008 09:10:06 -0500 "Steven A. Falco" <[email protected]> wrote:

When I saw Josh's question, I thought the irq save/restore was there to make the read-modify-write atomic; i.e. read SDR0_MFR, "AND" out some bits, then write it back without the possibility of anything else touching SDR0_MFR. I'm just starting to get familiar with the kernel,

That was probably the intention, yes.

Quoted 3 lines

so if you have a chance, please help educate me. Does the irq lock in mtdcri protect the read-modify-write? Or maybe this R-M-W doesn't need protecting?

Nope, it doesn't. It is possible for something to come in between the first call to mtdcri and the second call to mtdcri and modify the value. The window would be quite small, but I suppose it's theoretically possible.

Seems the code will do the right thing since everything is using flags. I suppose my question can be withdrawn. It is slightly confusing to do it that way though. Perhaps a function to do read-modify-writes on DCRs would be welcome. dcr_modify anyone?

josh

#10 lore raw

Re: [RFC][PATCH] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 4 lines · Josh Boyer

Seems the code will do the right thing since everything is using flags. I suppose my question can be withdrawn. It is slightly confusing to do it that way though. Perhaps a function to do read-modify-writes on DCRs would be welcome. dcr_modify anyone?

Yup, we probably want to expose a dcri_clrset(), though I would also expose then __mtdcri/__mfdcri (non locked version) and the spinlock in case somebody wants to do something fancy.

Cheers,
Ben.

#11 lore raw

Re: [RFC][PATCH] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 9 lines · Benjamin Herrenschmidt
Quoted 4 lines · Josh Boyer

Seems the code will do the right thing since everything is using flags. I suppose my question can be withdrawn. It is slightly confusing to do it that way though. Perhaps a function to do read-modify-writes on DCRs would be welcome. dcr_modify anyone?

Yup, we probably want to expose a dcri_clrset(), though I would also expose then __mtdcri/__mfdcri (non locked version) and the spinlock in case somebody wants to do something fancy.

I've made a patch which adds dcri_clrset function, but I haven't exposed the unlocked versions though. I think that dcri_clrset is sufficient for working with indirect dcr (at least for now, since the most fancy stuff done with it atm is read/modify/write) and exposing the unlocked version may lead to its misuse. I'll submit the patch shortly.

Thanks,
Valentine.

Quoted 9 lines

Cheers,
Ben.

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev

#12 lore raw

PATCH 1/1 PowerPC 4xx: Add dcri_clrset() for locked read/modify/write functionality

This adds dcri_clrset() macro which does read/modify/write on indirect dcr registers while holding indirect dcr lock.

---
 include/asm-powerpc/dcr-native.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+)
include/asm-powerpc/dcr-native.h +17 -0
@@ -82,6 +82,19 @@ static inline void __mtdcri(int base_add
 	spin_unlock_irqrestore(&dcr_ind_lock, flags);
 }
 
+static inline void __dcri_clrset(int base_addr, int base_data, int reg,
+				 unsigned clr, unsigned set)
+{
+	unsigned long flags;
+	unsigned int val;
+
+	spin_lock_irqsave(&dcr_ind_lock, flags);
+	__mtdcr(base_addr, reg);
+	val = (__mfdcr(base_data) & ~clr) | set;
+	__mtdcr(base_data, val);
+	spin_unlock_irqrestore(&dcr_ind_lock, flags);
+}
+
 #define mfdcri(base, reg)	__mfdcri(DCRN_ ## base ## _CONFIG_ADDR,	\
 					 DCRN_ ## base ## _CONFIG_DATA,	\
 					 reg)
@@ -90,6 +103,10 @@ static inline void __mtdcri(int base_add
 					 DCRN_ ## base ## _CONFIG_DATA,	\
 					 reg, data)
 
+#define dcri_clrset(base, reg, clr, set)	__dcri_clrset(DCRN_ ## base ## _CONFIG_ADDR,	\
+							      DCRN_ ## base ## _CONFIG_DATA,	\
+							      reg, clr, set)
+
 #endif /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_NATIVE_H */
Acked-by on v1 1/1
#13 lore raw

Re: [PATCH] PowerPC 4xx: Add dcri_clrset() for locked read/modify/write functionality

Quoted 4 lines · Valentine Barshak

This adds dcri_clrset() macro which does read/modify/write on indirect dcr registers while holding indirect dcr lock.

---

Quoted 37 lines
---
 include/asm-powerpc/dcr-native.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+)
include/asm-powerpc/dcr-native.h +17 -0
@@ -82,6 +82,19 @@ static inline void __mtdcri(int base_add
 	spin_unlock_irqrestore(&dcr_ind_lock, flags);
 }
 
+static inline void __dcri_clrset(int base_addr, int base_data, int reg,
+				 unsigned clr, unsigned set)
+{
+	unsigned long flags;
+	unsigned int val;
+
+	spin_lock_irqsave(&dcr_ind_lock, flags);
+	__mtdcr(base_addr, reg);
+	val = (__mfdcr(base_data) & ~clr) | set;
+	__mtdcr(base_data, val);
+	spin_unlock_irqrestore(&dcr_ind_lock, flags);
+}
+
 #define mfdcri(base, reg)	__mfdcri(DCRN_ ## base ## _CONFIG_ADDR,	\
 					 DCRN_ ## base ## _CONFIG_DATA,	\
 					 reg)
@@ -90,6 +103,10 @@ static inline void __mtdcri(int base_add
 					 DCRN_ ## base ## _CONFIG_DATA,	\
 					 reg, data)
 
+#define dcri_clrset(base, reg, clr, set)	__dcri_clrset(DCRN_ ## base ## _CONFIG_ADDR,	\
+							      DCRN_ ## base ## _CONFIG_DATA,	\
+							      reg, clr, set)
+
 #endif /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_NATIVE_H */
#14 lore raw

Re: [PATCH] PowerPC 4xx: Add dcri_clrset() for locked read/modify/write functionality

Quoted 8 lines · On Thu, 06 Mar 2008 11:06:18 +1100 Benjamin Herrenschmidt…
Quoted 4 lines · Valentine Barshak

This adds dcri_clrset() macro which does read/modify/write on indirect dcr registers while holding indirect dcr lock.

Indeed, looks good. Valentine, are you going to rework your EMAC patch to use this?

josh

#15 lore raw

Re: [PATCH] PowerPC 4xx: Add dcri_clrset() for locked read/modify/write functionality

Quoted 7 lines · Josh Boyer
Quoted 4 lines · On Thu, 06 Mar 2008 11:06:18 +1100 Benjamin Herrenschmidt…
Quoted 3 lines · Valentine Barshak

This adds dcri_clrset() macro which does read/modify/write on indirect dcr registers while holding indirect dcr lock.

Quoted 7 lines
Quoted 2 lines
Quoted 1 line

Indeed, looks good. Valentine, are you going to rework your EMAC patch to use this?

josh

Yes, I was going to update the pcie stuff and rework the emac patches.

Thanks,
Valentine.

#16 lore raw

Re: [PATCH] PowerPC 4xx: Add dcri_clrset() for locked read/modify/write functionality

Quoted 9 lines · Valentine Barshak
Quoted 8 lines · Josh Boyer
Quoted 5 lines · On Thu, 06 Mar 2008 11:06:18 +1100 Benjamin Herrenschmidt…
Quoted 4 lines · Valentine Barshak

This adds dcri_clrset() macro which does read/modify/write on indirect dcr registers while holding indirect dcr lock.

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

Indeed, looks good. Valentine, are you going to rework your EMAC patch to use this?

josh

Yes, I was going to update the pcie stuff and rework the emac patches.

The PCIe stuff is less of an issue as it happens early during boot, there should be no race there, but it's still a good idea in the long run. The EMAC bits look more like that kind of stuff we should have ready asap for linux-next.

Cheers,
Ben.

#17 lore raw

PATCH 1/2 ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)
drivers/net/ibm_newemac/core.c +15 -1
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c
@@ -43,6 +43,8 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/uaccess.h>
+#include <asm/dcr.h>
+#include <asm/dcr-regs.h>
 
 #include "core.h"
 
@@ -2323,6 +2325,10 @@ static int __devinit emac_init_phy(struc
 	dev->phy.mdio_read = emac_mdio_read;
 	dev->phy.mdio_write = emac_mdio_write;
 
+	/* Enable internal clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS);
+
 	/* Configure EMAC with defaults so we can at least use MDIO
 	 * This is needed mostly for 440GX
 	 */
@@ -2355,6 +2361,11 @@ static int __devinit emac_init_phy(struc
 			if (!emac_mii_phy_probe(&dev->phy, i))
 				break;
 		}
+
+	/* Enable external clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, SDR0_MFR_ECS, 0);
+
 	mutex_unlock(&emac_phy_map_lock);
 	if (i == 0x20) {
 		printk(KERN_WARNING "%s: can't find PHY!\n", np->full_name);
@@ -2480,8 +2491,11 @@ static int __devinit emac_init_config(st
 	}
 
 	/* Check EMAC version */
-	if (of_device_is_compatible(np, "ibm,emac4"))
+	if (of_device_is_compatible(np, "ibm,emac4")) {
 		dev->features |= EMAC_FTR_EMAC4;
+		if (of_device_is_compatible(np, "ibm,emac-440gx"))
+			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	}
 
 	/* Fixup some feature bits based on the device tree */
 	if (of_get_property(np, "has-inverted-stacr-oc", NULL))
drivers/net/ibm_newemac/core.h +6 -2
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h
@@ -301,6 +301,10 @@ struct emac_instance {
  * Set if we have new type STACR with STAOPC
  */
 #define EMAC_FTR_HAS_NEW_STACR		0x00000040
+/*
+ * Set if we need phy clock workaround for 440gx
+ */
+#define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -312,8 +316,8 @@ enum {
 
 	EMAC_FTRS_POSSIBLE	=
 #ifdef CONFIG_IBM_NEW_EMAC_EMAC4
-	    EMAC_FTR_EMAC4	| EMAC_FTR_HAS_NEW_STACR	|
-	    EMAC_FTR_STACR_OC_INVERT	|
+	    EMAC_FTR_EMAC4 | EMAC_FTR_HAS_NEW_STACR |
+	    EMAC_FTR_STACR_OC_INVERT | EMAC_FTR_440GX_PHY_CLK_FIX |
 #endif
 #ifdef CONFIG_IBM_NEW_EMAC_TAH
 	    EMAC_FTR_HAS_TAH	|
Acked-by on v1 1/2
#18 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

Quoted 8 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

Just get them acked by Jeff now and they can go in via Josh tree.

Cheers,
Ben.

Quoted 78 lines
---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c

drivers/net/ibm_newemac/core.c +15 -1
@@ -43,6 +43,8 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/uaccess.h>
+#include <asm/dcr.h>
+#include <asm/dcr-regs.h>
 
 #include "core.h"
 
@@ -2323,6 +2325,10 @@ static int __devinit emac_init_phy(struc
 	dev->phy.mdio_read = emac_mdio_read;
 	dev->phy.mdio_write = emac_mdio_write;
 
+	/* Enable internal clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS);
+
 	/* Configure EMAC with defaults so we can at least use MDIO
 	 * This is needed mostly for 440GX
 	 */
@@ -2355,6 +2361,11 @@ static int __devinit emac_init_phy(struc
 			if (!emac_mii_phy_probe(&dev->phy, i))
 				break;
 		}
+
+	/* Enable external clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, SDR0_MFR_ECS, 0);
+
 	mutex_unlock(&emac_phy_map_lock);
 	if (i == 0x20) {
 		printk(KERN_WARNING "%s: can't find PHY!\n", np->full_name);
@@ -2480,8 +2491,11 @@ static int __devinit emac_init_config(st
 	}
 
 	/* Check EMAC version */
-	if (of_device_is_compatible(np, "ibm,emac4"))
+	if (of_device_is_compatible(np, "ibm,emac4")) {
 		dev->features |= EMAC_FTR_EMAC4;
+		if (of_device_is_compatible(np, "ibm,emac-440gx"))
+			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	}
 
 	/* Fixup some feature bits based on the device tree */
 	if (of_get_property(np, "has-inverted-stacr-oc", NULL))

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h

drivers/net/ibm_newemac/core.h +6 -2
@@ -301,6 +301,10 @@ struct emac_instance {
  * Set if we have new type STACR with STAOPC
  */
 #define EMAC_FTR_HAS_NEW_STACR		0x00000040
+/*
+ * Set if we need phy clock workaround for 440gx
+ */
+#define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -312,8 +316,8 @@ enum {
 
 	EMAC_FTRS_POSSIBLE	=
 #ifdef CONFIG_IBM_NEW_EMAC_EMAC4
-	    EMAC_FTR_EMAC4	| EMAC_FTR_HAS_NEW_STACR	|
-	    EMAC_FTR_STACR_OC_INVERT	|
+	    EMAC_FTR_EMAC4 | EMAC_FTR_HAS_NEW_STACR |
+	    EMAC_FTR_STACR_OC_INVERT | EMAC_FTR_440GX_PHY_CLK_FIX |
 #endif
 #ifdef CONFIG_IBM_NEW_EMAC_TAH
 	    EMAC_FTR_HAS_TAH	|
#19 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

Quoted 9 lines · On Tue, 25 Mar 2008 10:21:53 +1100 Benjamin Herrenschmidt…
Quoted 7 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

Quoted 5 lines
Quoted 1 line

Just get them acked by Jeff now and they can go in via Josh tree.

This never went to netdev or Jeff from what I can see.

josh

#20 lore raw

PATCH 2/2 ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

In reply to Benjamin Herrenschmidt

This patch adds ibm_newemac PHY clock workaround for 440EP/440GR EMAC attached to a PHY which doesn't generate RX clock if there is no link. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock separately as opposed to global clock selection for 440GX.

---
 drivers/net/ibm_newemac/core.c |   34 ++++++++++++++++++++++++++++++++--
 drivers/net/ibm_newemac/core.h |    6 +++++-
 2 files changed, 37 insertions(+), 3 deletions(-)
drivers/net/ibm_newemac/core.c +32 -2
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c
@@ -129,10 +129,29 @@ static struct device_node *emac_boot_lis
 static inline void emac_report_timeout_error(struct emac_instance *dev,
 					     const char *error)
 {
-	if (net_ratelimit())
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX |
+				  EMAC_FTR_440EP_PHY_CLK_FIX))
+		DBG(dev, "%s" NL, error);
+	else if (net_ratelimit())
 		printk(KERN_ERR "%s: %s\n", dev->ndev->name, error);
 }
 
+/* EMAC PHY clock workaround:
+ * 440EP/440GR has more sane SDR0_MFR register implementation than 440GX,
+ * which allows controlling each EMAC clock
+ */
+static inline void emac_rx_clk_tx(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS >> dev->cell_index);
+}
+
+static inline void emac_rx_clk_default(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, SDR0_MFR_ECS >> dev->cell_index, 0);
+}
+
 /* PHY polling intervals */
 #define PHY_POLL_LINK_ON	HZ
 #define PHY_POLL_LINK_OFF	(HZ / 5)
@@ -1089,9 +1112,11 @@ static int emac_open(struct net_device *
 		int link_poll_interval;
 		if (dev->phy.def->ops->poll_link(&dev->phy)) {
 			dev->phy.def->ops->read_link(&dev->phy);
+			emac_rx_clk_default(dev);
 			netif_carrier_on(dev->ndev);
 			link_poll_interval = PHY_POLL_LINK_ON;
 		} else {
+			emac_rx_clk_tx(dev);
 			netif_carrier_off(dev->ndev);
 			link_poll_interval = PHY_POLL_LINK_OFF;
 		}
@@ -1169,6 +1194,7 @@ static void emac_link_timer(struct work_
 
 	if (dev->phy.def->ops->poll_link(&dev->phy)) {
 		if (!netif_carrier_ok(dev->ndev)) {
+			emac_rx_clk_default(dev);
 			/* Get new link parameters */
 			dev->phy.def->ops->read_link(&dev->phy);
 
@@ -1181,6 +1207,7 @@ static void emac_link_timer(struct work_
 		link_poll_interval = PHY_POLL_LINK_ON;
 	} else {
 		if (netif_carrier_ok(dev->ndev)) {
+			emac_rx_clk_tx(dev);
 			netif_carrier_off(dev->ndev);
 			netif_tx_disable(dev->ndev);
 			emac_reinitialize(dev);
@@ -2325,9 +2352,12 @@ static int __devinit emac_init_phy(struc
 	dev->phy.mdio_read = emac_mdio_read;
 	dev->phy.mdio_write = emac_mdio_write;
 
-	/* Enable internal clock source */
+	/* EMAC PHY clock workaround */
 	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		/* Enable internal clock source */
 		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS);
+	else
+		emac_rx_clk_tx(dev);
 
 	/* Configure EMAC with defaults so we can at least use MDIO
 	 * This is needed mostly for 440GX
@@ -2495,6 +2525,10 @@ static int __devinit emac_init_config(st
 		dev->features |= EMAC_FTR_EMAC4;
 		if (of_device_is_compatible(np, "ibm,emac-440gx"))
 			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	} else {
+		if (of_device_is_compatible(np, "ibm,emac-440ep") ||
+		    of_device_is_compatible(np, "ibm,emac-440gr"))
+			dev->features |= EMAC_FTR_440EP_PHY_CLK_FIX;
 	}
 
 	/* Fixup some feature bits based on the device tree */
drivers/net/ibm_newemac/core.h +5 -1
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h
@@ -305,6 +305,10 @@ struct emac_instance {
  * Set if we need phy clock workaround for 440gx
  */
 #define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
+/*
+ * Set if we need phy clock workaround for 440ep or 440gr
+ */
+#define EMAC_FTR_440EP_PHY_CLK_FIX	0x00000100
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -328,7 +332,7 @@ enum {
 #ifdef CONFIG_IBM_NEW_EMAC_RGMII
 	    EMAC_FTR_HAS_RGMII	|
 #endif
-	    0,
+	EMAC_FTR_440EP_PHY_CLK_FIX,
 };
 
 static inline int emac_has_feature(struct emac_instance *dev,
Acked-by on v1 2/2
#21 lore raw

Re: [PATCH 2/2] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 7 lines · Valentine Barshak

This patch adds ibm_newemac PHY clock workaround for 440EP/440GR EMAC attached to a PHY which doesn't generate RX clock if there is no link. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock separately as opposed to global clock selection for 440GX.

Quoted 115 lines
---
 drivers/net/ibm_newemac/core.c |   34 ++++++++++++++++++++++++++++++++--
 drivers/net/ibm_newemac/core.h |    6 +++++-
 2 files changed, 37 insertions(+), 3 deletions(-)

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c

drivers/net/ibm_newemac/core.c +32 -2
@@ -129,10 +129,29 @@ static struct device_node *emac_boot_lis
 static inline void emac_report_timeout_error(struct emac_instance *dev,
 					     const char *error)
 {
-	if (net_ratelimit())
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX |
+				  EMAC_FTR_440EP_PHY_CLK_FIX))
+		DBG(dev, "%s" NL, error);
+	else if (net_ratelimit())
 		printk(KERN_ERR "%s: %s\n", dev->ndev->name, error);
 }
 
+/* EMAC PHY clock workaround:
+ * 440EP/440GR has more sane SDR0_MFR register implementation than 440GX,
+ * which allows controlling each EMAC clock
+ */
+static inline void emac_rx_clk_tx(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS >> dev->cell_index);
+}
+
+static inline void emac_rx_clk_default(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, SDR0_MFR_ECS >> dev->cell_index, 0);
+}
+
 /* PHY polling intervals */
 #define PHY_POLL_LINK_ON	HZ
 #define PHY_POLL_LINK_OFF	(HZ / 5)
@@ -1089,9 +1112,11 @@ static int emac_open(struct net_device *
 		int link_poll_interval;
 		if (dev->phy.def->ops->poll_link(&dev->phy)) {
 			dev->phy.def->ops->read_link(&dev->phy);
+			emac_rx_clk_default(dev);
 			netif_carrier_on(dev->ndev);
 			link_poll_interval = PHY_POLL_LINK_ON;
 		} else {
+			emac_rx_clk_tx(dev);
 			netif_carrier_off(dev->ndev);
 			link_poll_interval = PHY_POLL_LINK_OFF;
 		}
@@ -1169,6 +1194,7 @@ static void emac_link_timer(struct work_
 
 	if (dev->phy.def->ops->poll_link(&dev->phy)) {
 		if (!netif_carrier_ok(dev->ndev)) {
+			emac_rx_clk_default(dev);
 			/* Get new link parameters */
 			dev->phy.def->ops->read_link(&dev->phy);
 
@@ -1181,6 +1207,7 @@ static void emac_link_timer(struct work_
 		link_poll_interval = PHY_POLL_LINK_ON;
 	} else {
 		if (netif_carrier_ok(dev->ndev)) {
+			emac_rx_clk_tx(dev);
 			netif_carrier_off(dev->ndev);
 			netif_tx_disable(dev->ndev);
 			emac_reinitialize(dev);
@@ -2325,9 +2352,12 @@ static int __devinit emac_init_phy(struc
 	dev->phy.mdio_read = emac_mdio_read;
 	dev->phy.mdio_write = emac_mdio_write;
 
-	/* Enable internal clock source */
+	/* EMAC PHY clock workaround */
 	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		/* Enable internal clock source */
 		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS);
+	else
+		emac_rx_clk_tx(dev);
 
 	/* Configure EMAC with defaults so we can at least use MDIO
 	 * This is needed mostly for 440GX
@@ -2495,6 +2525,10 @@ static int __devinit emac_init_config(st
 		dev->features |= EMAC_FTR_EMAC4;
 		if (of_device_is_compatible(np, "ibm,emac-440gx"))
 			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	} else {
+		if (of_device_is_compatible(np, "ibm,emac-440ep") ||
+		    of_device_is_compatible(np, "ibm,emac-440gr"))
+			dev->features |= EMAC_FTR_440EP_PHY_CLK_FIX;
 	}
 
 	/* Fixup some feature bits based on the device tree */

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h

drivers/net/ibm_newemac/core.h +5 -1
@@ -305,6 +305,10 @@ struct emac_instance {
  * Set if we need phy clock workaround for 440gx
  */
 #define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
+/*
+ * Set if we need phy clock workaround for 440ep or 440gr
+ */
+#define EMAC_FTR_440EP_PHY_CLK_FIX	0x00000100
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -328,7 +332,7 @@ enum {
 #ifdef CONFIG_IBM_NEW_EMAC_RGMII
 	    EMAC_FTR_HAS_RGMII	|
 #endif
-	    0,
+	EMAC_FTR_440EP_PHY_CLK_FIX,
 };
 
 static inline int emac_has_feature(struct emac_instance *dev,
#22 lore raw

Re: [PATCH 2/2] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 11 lines · On Tue, 25 Mar 2008 10:22:15 +1100 Benjamin Herrenschmidt…
Quoted 7 lines · Valentine Barshak

This patch adds ibm_newemac PHY clock workaround for 440EP/440GR EMAC attached to a PHY which doesn't generate RX clock if there is no link. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock separately as opposed to global clock selection for 440GX.

This never went to netdev or Jeff from what I can see.

josh

#23 lore raw

Re: [PATCH 2/2] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 10 lines · Josh Boyer
Quoted 7 lines · On Tue, 25 Mar 2008 10:22:15 +1100 Benjamin Herrenschmidt…
Quoted 6 lines · Valentine Barshak

This patch adds ibm_newemac PHY clock workaround for 440EP/440GR EMAC attached to a PHY which doesn't generate RX clock if there is no link. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock separately as opposed to global clock selection for 440GX.

Quoted 6 lines
Quoted 2 lines
Quoted 1 line

This never went to netdev or Jeff from what I can see.

josh

Do I need to resend these and the other one from http://ozlabs.org/pipermail/linuxppc-dev/2008-February/052114.html cc'ing Jeff?

I thought it was decided that Ben collects and sends all emac patches to netdev.

Thanks,
Valentine.

#24 lore raw

Re: [PATCH 2/2] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

Quoted 14 lines · On Thu, 27 Mar 2008 16:50:14 +0300 Valentine Barshak <[email protected]>…
Quoted 13 lines · Josh Boyer
Quoted 9 lines · On Tue, 25 Mar 2008 10:22:15 +1100 Benjamin Herrenschmidt…
Quoted 7 lines · Valentine Barshak

This patch adds ibm_newemac PHY clock workaround for 440EP/440GR EMAC attached to a PHY which doesn't generate RX clock if there is no link. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock separately as opposed to global clock selection for 440GX.

Quoted 7 lines
Quoted 3 lines

This never went to netdev or Jeff from what I can see.

josh

Do I need to resend these and the other one from
http://ozlabs.org/pipermail/linuxppc-dev/2008-February/052114.html
cc'ing Jeff?

Can't hurt.

Quoted 2 lines

I thought it was decided that Ben collects and sends all emac patches to netdev.

So did I.

josh

#25 lore raw

PATCH 1/1 ibm_newemac: emac_tx_csum typo fix

Move the "&& skb->ip_summed == CHECKSUM_PARTIAL" part out of emac_has_feature parameters.

---
 drivers/net/ibm_newemac/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/ibm_newemac/core.c +2 -2
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c
@@ -1235,8 +1235,8 @@ static int emac_close(struct net_device 
 static inline u16 emac_tx_csum(struct emac_instance *dev,
 			       struct sk_buff *skb)
 {
-	if (emac_has_feature(dev, EMAC_FTR_HAS_TAH &&
-			     skb->ip_summed == CHECKSUM_PARTIAL)) {
+	if (emac_has_feature(dev, EMAC_FTR_HAS_TAH) &&
+		(skb->ip_summed == CHECKSUM_PARTIAL)) {
 		++dev->stats.tx_packets_csum;
 		return EMAC_TX_CTRL_TAH_CSUM;
 	}
Applied
#26 lore raw

Re: [PATCH] ibm_newemac: emac_tx_csum typo fix

Quoted 12 lines · Valentine Barshak

Move the "&& skb->ip_summed == CHECKSUM_PARTIAL" part out of emac_has_feature parameters.

---
 drivers/net/ibm_newemac/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c

drivers/net/ibm_newemac/core.c static int emac_close(struct net_device · line 1241

Quoted 9 lines
drivers/net/ibm_newemac/core.c +2 -2
@@ -1235,8 +1235,8 @@ static int emac_close(struct net_device 
 static inline u16 emac_tx_csum(struct emac_instance *dev,
 			       struct sk_buff *skb)
 {
-	if (emac_has_feature(dev, EMAC_FTR_HAS_TAH &&
-			     skb->ip_summed == CHECKSUM_PARTIAL)) {
+	if (emac_has_feature(dev, EMAC_FTR_HAS_TAH) &&
+		(skb->ip_summed == CHECKSUM_PARTIAL)) {
 		++dev->stats.tx_packets_csum;
 		return EMAC_TX_CTRL_TAH_CSUM;

applied

#27 lore raw

PATCH 1/2 ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

In reply to Josh Boyer

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)
drivers/net/ibm_newemac/core.c +15 -1
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c
@@ -43,6 +43,8 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/uaccess.h>
+#include <asm/dcr.h>
+#include <asm/dcr-regs.h>
 
 #include "core.h"
 
@@ -2323,6 +2325,10 @@ static int __devinit emac_init_phy(struc
 	dev->phy.mdio_read = emac_mdio_read;
 	dev->phy.mdio_write = emac_mdio_write;
 
+	/* Enable internal clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS);
+
 	/* Configure EMAC with defaults so we can at least use MDIO
 	 * This is needed mostly for 440GX
 	 */
@@ -2355,6 +2361,11 @@ static int __devinit emac_init_phy(struc
 			if (!emac_mii_phy_probe(&dev->phy, i))
 				break;
 		}
+
+	/* Enable external clock source */
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, SDR0_MFR_ECS, 0);
+
 	mutex_unlock(&emac_phy_map_lock);
 	if (i == 0x20) {
 		printk(KERN_WARNING "%s: can't find PHY!\n", np->full_name);
@@ -2480,8 +2491,11 @@ static int __devinit emac_init_config(st
 	}
 
 	/* Check EMAC version */
-	if (of_device_is_compatible(np, "ibm,emac4"))
+	if (of_device_is_compatible(np, "ibm,emac4")) {
 		dev->features |= EMAC_FTR_EMAC4;
+		if (of_device_is_compatible(np, "ibm,emac-440gx"))
+			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	}
 
 	/* Fixup some feature bits based on the device tree */
 	if (of_get_property(np, "has-inverted-stacr-oc", NULL))
drivers/net/ibm_newemac/core.h +6 -2
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h
@@ -301,6 +301,10 @@ struct emac_instance {
  * Set if we have new type STACR with STAOPC
  */
 #define EMAC_FTR_HAS_NEW_STACR		0x00000040
+/*
+ * Set if we need phy clock workaround for 440gx
+ */
+#define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -312,8 +316,8 @@ enum {
 
 	EMAC_FTRS_POSSIBLE	=
 #ifdef CONFIG_IBM_NEW_EMAC_EMAC4
-	    EMAC_FTR_EMAC4	| EMAC_FTR_HAS_NEW_STACR	|
-	    EMAC_FTR_STACR_OC_INVERT	|
+	    EMAC_FTR_EMAC4 | EMAC_FTR_HAS_NEW_STACR |
+	    EMAC_FTR_STACR_OC_INVERT | EMAC_FTR_440GX_PHY_CLK_FIX |
 #endif
 #ifdef CONFIG_IBM_NEW_EMAC_TAH
 	    EMAC_FTR_HAS_TAH	|
#28 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

Quoted 7 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

Quoted 5 lines
---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

is this for 2.6.25-rc?

#29 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

Quoted 8 lines · Jeff Garzik
Quoted 7 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

Quoted 7 lines
Quoted 5 lines
---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

is this for 2.6.25-rc?

Nah, too late imho.

Ben.

#30 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

In reply to Jeff Garzik
Quoted 8 lines · On Fri, 28 Mar 2008 22:18:25 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 7 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

Quoted 7 lines
Quoted 5 lines
---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

is this for 2.6.25-rc?

No. This, and patch 2/2, are for 2.6.26 and depend on a patch in my tree. These are the two Ben asked about going through the powerpc tree but naturally we wanted an Ack from you first.

thx,
josh

#31 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

In reply to Jeff Garzik
Quoted 8 lines · On Fri, 28 Mar 2008 22:18:25 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 7 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

Quoted 7 lines
Quoted 5 lines
---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

is this for 2.6.25-rc?

Jeff, can I get an ack from you on this patch, and patch 2 in this set? They depend on a patch in my tree and I'd like to include them in my next push to Paul for 2.6.26.

josh

#32 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

Quoted 16 lines · Josh Boyer
Quoted 13 lines · On Fri, 28 Mar 2008 22:18:25 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 12 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)
Quoted 5 lines
Quoted 1 line

is this for 2.6.25-rc?

Jeff, can I get an ack from you on this patch, and patch 2 in this set? They depend on a patch in my tree and I'd like to include them in my next push to Paul for 2.6.26.

ACK

I had queried the status of these patches, and didn't receive any reply initially from my query...

#33 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

Quoted 17 lines · Jeff Garzik
Quoted 16 lines · Josh Boyer
Quoted 13 lines · On Fri, 28 Mar 2008 22:18:25 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 12 lines · Valentine Barshak

The PowerPC 440GX Taishan board fails to reset EMAC3 (reset timeout error) if there's no link. Because of that it fails to find PHY chip. The older ibm_emac driver had a workaround for that: the EMAC_CLK_INTERNAL/EMAC_CLK_EXTERNAL macros, which toggle the Ethernet Clock Select bit in the SDR0_MFR register. This patch does the same for "ibm,emac-440gx" compatible chips. The workaround forces clock on -all- EMACs, so we select clock under global emac_phy_map_lock.

---
 drivers/net/ibm_newemac/core.c |   16 +++++++++++++++-
 drivers/net/ibm_newemac/core.h |    8 ++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)
Quoted 7 lines
Quoted 5 lines
Quoted 1 line

is this for 2.6.25-rc?

Jeff, can I get an ack from you on this patch, and patch 2 in this set? They depend on a patch in my tree and I'd like to include them in my next push to Paul for 2.6.26.

ACK

Many thanks.

Quoted 2 lines

I had queried the status of these patches, and didn't receive any reply initially from my query...

Erm... you did.

http://ozlabs.org/pipermail/linuxppc-dev/2008-March/053737.html

No worries though. I lose email all the time.

josh

#34 lore raw

Re: [PATCH 1/2] ibm_newemac: PowerPC 440GX EMAC PHY clock workaround

Quoted 8 lines · Josh Boyer
Quoted 1 line · Jeff Garzik

I had queried the status of these patches, and didn't receive any reply initially from my query...

Erm... you did.

http://ozlabs.org/pipermail/linuxppc-dev/2008-March/053737.html

No worries though. I lose email all the time.

Whoops, sorry about that!

Jeff

#35 lore raw

PATCH 2/2 ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

In reply to Josh Boyer

This patch adds ibm_newemac PHY clock workaround for 440EP/440GR EMAC attached to a PHY which doesn't generate RX clock if there is no link. The code is based on the previous ibm_emac driver stuff. The 440EP/440GR allows controlling each EMAC clock separately as opposed to global clock selection for 440GX.

---
 drivers/net/ibm_newemac/core.c |   34 ++++++++++++++++++++++++++++++++--
 drivers/net/ibm_newemac/core.h |    6 +++++-
 2 files changed, 37 insertions(+), 3 deletions(-)
drivers/net/ibm_newemac/core.c +32 -2
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.c linux-2.6/drivers/net/ibm_newemac/core.c
@@ -129,10 +129,29 @@ static struct device_node *emac_boot_lis
 static inline void emac_report_timeout_error(struct emac_instance *dev,
 					     const char *error)
 {
-	if (net_ratelimit())
+	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX |
+				  EMAC_FTR_440EP_PHY_CLK_FIX))
+		DBG(dev, "%s" NL, error);
+	else if (net_ratelimit())
 		printk(KERN_ERR "%s: %s\n", dev->ndev->name, error);
 }
 
+/* EMAC PHY clock workaround:
+ * 440EP/440GR has more sane SDR0_MFR register implementation than 440GX,
+ * which allows controlling each EMAC clock
+ */
+static inline void emac_rx_clk_tx(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS >> dev->cell_index);
+}
+
+static inline void emac_rx_clk_default(struct emac_instance *dev)
+{
+	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX))
+		dcri_clrset(SDR0, SDR0_MFR, SDR0_MFR_ECS >> dev->cell_index, 0);
+}
+
 /* PHY polling intervals */
 #define PHY_POLL_LINK_ON	HZ
 #define PHY_POLL_LINK_OFF	(HZ / 5)
@@ -1089,9 +1112,11 @@ static int emac_open(struct net_device *
 		int link_poll_interval;
 		if (dev->phy.def->ops->poll_link(&dev->phy)) {
 			dev->phy.def->ops->read_link(&dev->phy);
+			emac_rx_clk_default(dev);
 			netif_carrier_on(dev->ndev);
 			link_poll_interval = PHY_POLL_LINK_ON;
 		} else {
+			emac_rx_clk_tx(dev);
 			netif_carrier_off(dev->ndev);
 			link_poll_interval = PHY_POLL_LINK_OFF;
 		}
@@ -1169,6 +1194,7 @@ static void emac_link_timer(struct work_
 
 	if (dev->phy.def->ops->poll_link(&dev->phy)) {
 		if (!netif_carrier_ok(dev->ndev)) {
+			emac_rx_clk_default(dev);
 			/* Get new link parameters */
 			dev->phy.def->ops->read_link(&dev->phy);
 
@@ -1181,6 +1207,7 @@ static void emac_link_timer(struct work_
 		link_poll_interval = PHY_POLL_LINK_ON;
 	} else {
 		if (netif_carrier_ok(dev->ndev)) {
+			emac_rx_clk_tx(dev);
 			netif_carrier_off(dev->ndev);
 			netif_tx_disable(dev->ndev);
 			emac_reinitialize(dev);
@@ -2325,9 +2352,12 @@ static int __devinit emac_init_phy(struc
 	dev->phy.mdio_read = emac_mdio_read;
 	dev->phy.mdio_write = emac_mdio_write;
 
-	/* Enable internal clock source */
+	/* EMAC PHY clock workaround */
 	if (emac_has_feature(dev, EMAC_FTR_440GX_PHY_CLK_FIX))
+		/* Enable internal clock source */
 		dcri_clrset(SDR0, SDR0_MFR, 0, SDR0_MFR_ECS);
+	else
+		emac_rx_clk_tx(dev);
 
 	/* Configure EMAC with defaults so we can at least use MDIO
 	 * This is needed mostly for 440GX
@@ -2495,6 +2525,10 @@ static int __devinit emac_init_config(st
 		dev->features |= EMAC_FTR_EMAC4;
 		if (of_device_is_compatible(np, "ibm,emac-440gx"))
 			dev->features |= EMAC_FTR_440GX_PHY_CLK_FIX;
+	} else {
+		if (of_device_is_compatible(np, "ibm,emac-440ep") ||
+		    of_device_is_compatible(np, "ibm,emac-440gr"))
+			dev->features |= EMAC_FTR_440EP_PHY_CLK_FIX;
 	}
 
 	/* Fixup some feature bits based on the device tree */
drivers/net/ibm_newemac/core.h +5 -1
diff -pruN linux-2.6.orig/drivers/net/ibm_newemac/core.h linux-2.6/drivers/net/ibm_newemac/core.h
@@ -305,6 +305,10 @@ struct emac_instance {
  * Set if we need phy clock workaround for 440gx
  */
 #define EMAC_FTR_440GX_PHY_CLK_FIX	0x00000080
+/*
+ * Set if we need phy clock workaround for 440ep or 440gr
+ */
+#define EMAC_FTR_440EP_PHY_CLK_FIX	0x00000100
 
 
 /* Right now, we don't quite handle the always/possible masks on the
@@ -328,7 +332,7 @@ enum {
 #ifdef CONFIG_IBM_NEW_EMAC_RGMII
 	    EMAC_FTR_HAS_RGMII	|
 #endif
-	    0,
+	EMAC_FTR_440EP_PHY_CLK_FIX,
 };
 
 static inline int emac_has_feature(struct emac_instance *dev,
#36 lore raw

PATCH 1/1 PowerPC 4xx: Use dcri_clrset() for PCIe indirect dcr read/modify/write access

In reply to Benjamin Herrenschmidt

Use dcri_clrset() for PCIe SDR0 read/modify/write access.

---
 arch/powerpc/sysdev/ppc4xx_pci.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)
arch/powerpc/sysdev/ppc4xx_pci.c +4 -7
diff -pruN linux-2.6.orig/arch/powerpc/sysdev/ppc4xx_pci.c linux-2.6/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -645,7 +645,7 @@ static int __init ppc440spe_pciex_core_i
 	int time_out = 20;
 
 	/* Set PLL clock receiver to LVPECL */
-	mtdcri(SDR0, PESDR0_PLLLCT1, mfdcri(SDR0, PESDR0_PLLLCT1) | 1 << 28);
+	dcri_clrset(SDR0, PESDR0_PLLLCT1, 0, 1 << 28);
 
 	/* Shouldn't we do all the calibration stuff etc... here ? */
 	if (ppc440spe_pciex_check_reset(np))
@@ -659,8 +659,7 @@ static int __init ppc440spe_pciex_core_i
 	}
 
 	/* De-assert reset of PCIe PLL, wait for lock */
-	mtdcri(SDR0, PESDR0_PLLLCT1,
-	       mfdcri(SDR0, PESDR0_PLLLCT1) & ~(1 << 24));
+	dcri_clrset(SDR0, PESDR0_PLLLCT1, 1 << 24, 0);
 	udelay(3);
 
 	while (time_out) {
@@ -712,9 +711,8 @@ static int ppc440spe_pciex_init_port_hw(
 		mtdcri(SDR0, port->sdr_base + PESDRn_440SPE_HSSL7SET1,
 		       0x35000000);
 	}
-	val = mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET);
 	mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
-	       (val & ~(1 << 24 | 1 << 16)) | 1 << 12);
+		(1 << 24) | (1 << 16), 1 << 12);
 
 	return 0;
 }
@@ -1042,8 +1040,7 @@ static int __init ppc4xx_pciex_port_init
 		port->link = 0;
 	}
 
-	mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
-	       mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) | 1 << 20);
+	dcri_clrset(SDR0, port->sdr_base + PESDRn_RCSSET, 0, 1 << 20);
 	msleep(100);
 
 	return 0;
#37 lore raw

Re: [PATCH] PowerPC 4xx: Use dcri_clrset() for PCIe indirect dcr read/modify/write access

Oops, sorry, please discard this one.
There's a typo in it. The fixed patch is coming right away. Thanks.
Valentine.

Quoted 54 lines · Valentine Barshak

Use dcri_clrset() for PCIe SDR0 read/modify/write access.

---
 arch/powerpc/sysdev/ppc4xx_pci.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff -pruN linux-2.6.orig/arch/powerpc/sysdev/ppc4xx_pci.c linux-2.6/arch/powerpc/sysdev/ppc4xx_pci.c

arch/powerpc/sysdev/ppc4xx_pci.c +4 -7
@@ -645,7 +645,7 @@ static int __init ppc440spe_pciex_core_i
 	int time_out = 20;
 
 	/* Set PLL clock receiver to LVPECL */
-	mtdcri(SDR0, PESDR0_PLLLCT1, mfdcri(SDR0, PESDR0_PLLLCT1) | 1 << 28);
+	dcri_clrset(SDR0, PESDR0_PLLLCT1, 0, 1 << 28);
 
 	/* Shouldn't we do all the calibration stuff etc... here ? */
 	if (ppc440spe_pciex_check_reset(np))
@@ -659,8 +659,7 @@ static int __init ppc440spe_pciex_core_i
 	}
 
 	/* De-assert reset of PCIe PLL, wait for lock */
-	mtdcri(SDR0, PESDR0_PLLLCT1,
-	       mfdcri(SDR0, PESDR0_PLLLCT1) & ~(1 << 24));
+	dcri_clrset(SDR0, PESDR0_PLLLCT1, 1 << 24, 0);
 	udelay(3);
 
 	while (time_out) {
@@ -712,9 +711,8 @@ static int ppc440spe_pciex_init_port_hw(
 		mtdcri(SDR0, port->sdr_base + PESDRn_440SPE_HSSL7SET1,
 		       0x35000000);
 	}
-	val = mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET);
 	mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
-	       (val & ~(1 << 24 | 1 << 16)) | 1 << 12);
+		(1 << 24) | (1 << 16), 1 << 12);
 
 	return 0;
 }
@@ -1042,8 +1040,7 @@ static int __init ppc4xx_pciex_port_init
 		port->link = 0;
 	}
 
-	mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
-	       mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) | 1 << 20);
+	dcri_clrset(SDR0, port->sdr_base + PESDRn_RCSSET, 0, 1 << 20);
 	msleep(100);
 
 	return 0;

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev

#38 lore raw

PATCH 1/1 PowerPC 4xx: Use dcri_clrset() for PCIe indirect dcr read/modify/write access

Use dcri_clrset() for PCIe SDR0 read/modify/write access.

---
 arch/powerpc/sysdev/ppc4xx_pci.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
arch/powerpc/sysdev/ppc4xx_pci.c +5 -8
diff -pruN linux-2.6.orig/arch/powerpc/sysdev/ppc4xx_pci.c linux-2.6/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -645,7 +645,7 @@ static int __init ppc440spe_pciex_core_i
 	int time_out = 20;
 
 	/* Set PLL clock receiver to LVPECL */
-	mtdcri(SDR0, PESDR0_PLLLCT1, mfdcri(SDR0, PESDR0_PLLLCT1) | 1 << 28);
+	dcri_clrset(SDR0, PESDR0_PLLLCT1, 0, 1 << 28);
 
 	/* Shouldn't we do all the calibration stuff etc... here ? */
 	if (ppc440spe_pciex_check_reset(np))
@@ -659,8 +659,7 @@ static int __init ppc440spe_pciex_core_i
 	}
 
 	/* De-assert reset of PCIe PLL, wait for lock */
-	mtdcri(SDR0, PESDR0_PLLLCT1,
-	       mfdcri(SDR0, PESDR0_PLLLCT1) & ~(1 << 24));
+	dcri_clrset(SDR0, PESDR0_PLLLCT1, 1 << 24, 0);
 	udelay(3);
 
 	while (time_out) {
@@ -712,9 +711,8 @@ static int ppc440spe_pciex_init_port_hw(
 		mtdcri(SDR0, port->sdr_base + PESDRn_440SPE_HSSL7SET1,
 		       0x35000000);
 	}
-	val = mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET);
-	mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
-	       (val & ~(1 << 24 | 1 << 16)) | 1 << 12);
+	dcri_clrset(SDR0, port->sdr_base + PESDRn_RCSSET,
+			(1 << 24) | (1 << 16), 1 << 12);
 
 	return 0;
 }
@@ -1042,8 +1040,7 @@ static int __init ppc4xx_pciex_port_init
 		port->link = 0;
 	}
 
-	mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
-	       mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) | 1 << 20);
+	dcri_clrset(SDR0, port->sdr_base + PESDRn_RCSSET, 0, 1 << 20);
 	msleep(100);
 
 	return 0;