Bug in new IBM 4xx on-chip Ethernet drivers?

Matt Porter mporter at kernel.crashing.org
Thu Mar 4 05:46:44 EST 2004


On Wed, Mar 03, 2004 at 12:59:36PM +0100, llandre wrote:
>
> Hi Matt,
>
> Benjamin Herrenschmidt suggested me (see at the end of the message) to get
> in touch
> with you about the following problem.
> I'm working with a custom 405EP-based board (PPChameleonEVB). So far I used
> the kernel 2.4.20
> and now I'm moving to the 2.4.23. I realized that the on-chip Ethernet MAC
> driver
> changed.
> I have a question about the function emac_phy_read in ibm_ocp_enet.c file.
> In my understanding the following snippet is not correct
>
>          /* Clear the speed bits and make a read request to the PHY */
>          stacr = ((EMAC_STACR_READ | (reg & 0x1f)) & ~EMAC_STACR_CLK_100MHZ);
>          stacr |= ((mii_id & 0x1F) << 5);
>
> because it assumes the OPB clock frequency is 50MHz. If it differs,
> the MII clock frequency generated by the Ethernet controller is erroneous.
> For example, with OPB frequency = 66MHz, the MII clock frequency is set
> to 3.3MHz (this should be 2.5MHz).
> I had already reported the problem to Armin Kuster (please see the message
> http://lists.linuxppc.org/linuxppc-embedded/200306/msg00134.html) and I
> proposed
> a solution but I got no response.
> What do you think?

OPB frequency can be configured at boottime so we should pass it in
to the driver.  Note that later EMACs (440G?) have the OPBC bits in
the STACR as reserved bits so they should always be zero.

The included patch adds some additions fields for opb bus freq as
well as a flag to indicate whether the opbc bits should be set
appropriately from the given opb bus spd.  Can you test this patch
on your board?  If you apply, then you only need to call the
ocp_get_one_device() routine from your board specific code to retrieve
the ocp_defs for each EMAC.  Set the opbc and opb_bus_spd fields with
your platform specific values and the driver should set the OPBC bits
appropriately.

Note: I'll be using the opb_bus_spd for setting MR1 eventually since
newer EMACs require OPB freq there as well. We may need to add
a generic data area to OCP since some values may be used by many
drivers (OPB freq used by IIC and EMAC) and it seems pointless to
copy the freq to each ocp_def additions field.

-Matt

===== drivers/net/ibm_emac/ibm_emac.h 1.3 vs edited =====
--- 1.3/drivers/net/ibm_emac/ibm_emac.h	Mon Jan 26 18:10:45 2004
+++ edited/drivers/net/ibm_emac/ibm_emac.h	Wed Mar  3 10:28:09 2004
@@ -36,6 +36,12 @@
 #define _IBM_OCP_EMAC_H_
 /* General defines needed for the driver */

+/* OPB */
+#define EMAC_OPB_SPD_50MHZ	50000000
+#define EMAC_OPB_SPD_66MHZ	66666666
+#define EMAC_OPB_SPD_83MHZ	83333333
+#define EMAC_OPB_SPD_100MHZ	100000000
+
 /* Emac */
 typedef struct emac_regs {
 	u32 em0mr0;
@@ -181,9 +187,11 @@
 #define EMAC_STACR_PHYE			0x00004000
 #define EMAC_STACR_WRITE		0x00002000
 #define EMAC_STACR_READ			0x00001000
-#define EMAC_STACR_CLK_83MHZ		0x00000800	/* 0's for 50Mhz */
+#define EMAC_STACR_CLK_50MHZ		0x00000000
 #define EMAC_STACR_CLK_66MHZ		0x00000400
+#define EMAC_STACR_CLK_83MHZ		0x00000800
 #define EMAC_STACR_CLK_100MHZ		0x00000C00
+#define EMAC_STACR_OPBC_MSK		0x00000C00

 /* Transmit Request Threshold Register */
 #define EMAC_TRTR_1600			0x18000000	/* 0's for 64 Bytes */
===== drivers/net/ibm_emac/ibm_ocp_enet.c 1.3 vs edited =====
--- 1.3/drivers/net/ibm_emac/ibm_ocp_enet.c	Mon Jan 26 17:45:52 2004
+++ edited/drivers/net/ibm_emac/ibm_ocp_enet.c	Wed Mar  3 10:32:00 2004
@@ -226,7 +226,7 @@
 emac_phy_read(struct net_device *dev, int mii_id, int reg)
 {
 	register int i;
-	uint32_t stacr;
+	uint32_t stacr, opbc = 0;
 	struct ocp_enet_private *fep = dev->priv;
 	volatile emac_t *emacp = fep->emacp;

@@ -252,8 +252,25 @@
 		return -1;
 	}

-	/* Clear the speed bits and make a read request to the PHY */
-	stacr = ((EMAC_STACR_READ | (reg & 0x1f)) & ~EMAC_STACR_CLK_100MHZ);
+	/*
+	 * STACR clock bits are reserved on some platforms so opbc
+	 * defaults to zero. On other platforms, they are used to
+	 * control MDIO frequency.
+	 */
+	if (fep->emacdata->opbc) {
+		if (fep->emacdata->opb_bus_spd <= EMAC_OPB_SPD_50MHZ)
+			opbc = EMAC_STACR_CLK_50MHZ;
+		else if (fep->emacdata->opb_bus_spd <= EMAC_OPB_SPD_66MHZ)
+			opbc = EMAC_STACR_CLK_66MHZ;
+		else if (fep->emacdata->opb_bus_spd <= EMAC_OPB_SPD_83MHZ)
+			opbc = EMAC_STACR_CLK_83MHZ;
+		else
+			opbc = EMAC_STACR_CLK_100MHZ;
+	}
+
+	/* Make a read request to the PHY */
+	stacr = (((EMAC_STACR_READ | (reg & 0x1f)) & ~EMAC_STACR_OPBC_MSK)
+			| opbc);
 	stacr |= ((mii_id & 0x1F) << 5);

 	out_be32(&emacp->em0stacr, stacr);
@@ -1268,6 +1285,7 @@
 	memset(ep, 0, sizeof(*ep));
 	ep->ndev = ndev;
 	ep->ocpdev = ocpdev;
+	ep->emacdata = (struct ocp_func_emac_data *)ocpdev->def->additions;
 	ndev->irq = ocpdev->def->irq;
 	ep->wol_irq = emacdata->wol_irq;
 	ep->mdio_dev = mdio_ndev;
===== drivers/net/ibm_emac/ibm_ocp_enet.h 1.1 vs edited =====
--- 1.1/drivers/net/ibm_emac/ibm_ocp_enet.h	Thu Jul 10 12:50:02 2003
+++ edited/drivers/net/ibm_emac/ibm_ocp_enet.h	Wed Mar  3 10:31:04 2004
@@ -152,6 +152,7 @@
         int wol_irq;
 	volatile emac_t *emacp;
 	struct ocp_device *ocpdev;
+	struct ocp_func_emac_data *emacdata;
         struct net_device *ndev;
         spinlock_t lock;
 };
===== include/asm-ppc/ocp_ids.h 1.8 vs edited =====
--- 1.8/include/asm-ppc/ocp_ids.h	Wed Jan  7 12:55:16 2004
+++ edited/include/asm-ppc/ocp_ids.h	Wed Mar  3 10:30:25 2004
@@ -101,6 +101,8 @@
 		int	mal_tx2_chan;	/* MAL tx channel 2 number */
 		int	wol_irq;	/* WOL interrupt */
 		int	mdio_idx;	/* EMAC idx of MDIO master or -1 */
+		int	opbc;		/* Indicates use of STACR clk bits */
+		int	opb_bus_spd;	/* OPB bus speed */
 	};

 /* Bridge devices 0xE00 - 0xEFF */


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/





More information about the Linuxppc-embedded mailing list