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

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

[llandre]

Hi Benjamin,

I'm working with a custom 405EP-based board. 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?




TIA and best regards,

llandre


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

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

[Benjamin Herrenschmidt]

> 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?

Hi ! You may well be right on this one. I don't have any 4xx HW
at hand any more, I suggest you contact Matt Porter
<mporter@kernel.crashing.org> on this one
(Sorry Matt, I know I'm not helping much lately...)

Ben.


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

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

[llandre]

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?


Thanks and best regards.



At 11.57 03/03/2004, Benjamin Herrenschmidt wrote:

> > 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?
>
>Hi ! You may well be right on this one. I don't have any 4xx HW
>at hand any more, I suggest you contact Matt Porter
><mporter@kernel.crashing.org> on this one
>(Sorry Matt, I know I'm not helping much lately...)
>
>Ben.




llandre


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

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

[Eugene Surovegin]

On Wed, Mar 03, 2004 at 10:48:00AM +0100, llandre wrote:
> I'm working with a custom 405EP-based board. 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).

Although you are rigth that this code needs fixing, my experience with
different 4xx hardware running with different OPB clock speed showed
that even incorrect setting doesn't affect MII operation.

And to be fair, all previous EMAC drivers had the same bug :).

Eugene.

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

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

[Matt Porter]

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/

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

[llandre]

Hi Eugenene,

I can confirm your experience. As phy we use the STE100P from ST. I've
never seen it to
fail with MII clock frequency set to 3.3MHz. Anyway I think that, for
production,
it should be set to 2.5MHz as requested by the datasheet.


>On Wed, Mar 03, 2004 at 10:48:00AM +0100, llandre wrote:
> > I'm working with a custom 405EP-based board. 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).
>
>Although you are rigth that this code needs fixing, my experience with
>different 4xx hardware running with different OPB clock speed showed
>that even incorrect setting doesn't affect MII operation.
>
>And to be fair, all previous EMAC drivers had the same bug :).
>
>Eugene.




llandre


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