linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Fix for autonegotiation of ppc 40x ethernet driver
@ 2003-10-08  8:17 Ronald Wahl
  2003-10-08  8:41 ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 4+ messages in thread
From: Ronald Wahl @ 2003-10-08  8:17 UTC (permalink / raw)
  To: linuxppc-dev


Hello,

I have found a bug in the ethernet driver for the PPC 40x. The problem
is that the driver doesn't deal correctly if a user wants a limited
advertising range (via ethtool interface) - lets say 10Mbps/Halfduplex
and 10MBps/Fullduplex. The link always enters 100MBps/Fullduplex mode if
the link partner supports it. This is an incorrect behavior. I appended
a fix for this (against 2.4.22+ of the linuxppc 3.4 devel tree). It
would be nice if this change could be reviewed and incorporated into the
official code. If any questions arise - just ask...

- ron

Index: drivers/net/ibm_emac/ibm_ocp_enet.c
===================================================================
RCS file: /var/CVS/eric_firmware/linuxnew/drivers/net/ibm_emac/ibm_ocp_enet.c,v
retrieving revision 1.1.23.1
diff -u -r1.1.23.1 ibm_ocp_enet.c
--- drivers/net/ibm_emac/ibm_ocp_enet.c	4 Sep 2003 12:27:54 -0000	1.1.23.1
+++ drivers/net/ibm_emac/ibm_ocp_enet.c	8 Oct 2003 07:59:34 -0000
@@ -928,8 +928,7 @@
 	int forced_duplex;

 	/* Default advertise */
-	advertise = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
-		    ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full;
+	advertise = fep->advertising;
 	autoneg = fep->want_autoneg;
 	forced_speed = fep->phy_mii.speed;
 	forced_duplex = fep->phy_mii.duplex;
@@ -938,6 +937,7 @@
 	if (ep) {
 	    if (ep->autoneg == AUTONEG_ENABLE) {
 		advertise = ep->advertising;
+		fep->advertising = advertise;
 		autoneg = 1;
 	    } else {
 		autoneg = 0;
@@ -1114,7 +1114,7 @@
 emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct ocp_enet_private *fep = dev->priv;
-	uint *data = (uint *) & rq->ifr_data;
+	ushort *data = (ushort *) & rq->ifr_data;

 	switch (cmd) {
         case SIOCETHTOOL:
@@ -1367,8 +1367,12 @@
 	if (ep->phy_mii.def->ops->init)
 	        ep->phy_mii.def->ops->init(&ep->phy_mii);
 	netif_carrier_off(ndev);
-        if (ep->phy_mii.def->features & SUPPORTED_Autoneg)
+        if (ep->phy_mii.def->features & SUPPORTED_Autoneg) {
                 ep->want_autoneg = 1;
+		ep->advertising =
+			ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
+			ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full;
+	}
 	emac_start_link(ep, NULL);


Index: drivers/net/ibm_emac/ibm_ocp_enet.h
===================================================================
RCS file: /var/CVS/eric_firmware/linuxnew/drivers/net/ibm_emac/ibm_ocp_enet.h,v
retrieving revision 1.1.23.1
diff -u -r1.1.23.1 ibm_ocp_enet.h
--- drivers/net/ibm_emac/ibm_ocp_enet.h	4 Sep 2003 12:27:54 -0000	1.1.23.1
+++ drivers/net/ibm_emac/ibm_ocp_enet.h	8 Oct 2003 07:59:34 -0000
@@ -136,6 +136,7 @@
 	struct mii_phy phy_mii;
         int mii_phy_addr;
         int want_autoneg;
+	u32 advertising;
         int timer_ticks;
 	struct timer_list link_timer;
 	struct net_device *mdio_dev;
Index: drivers/net/ibm_emac/ibm_ocp_phy.c
===================================================================
RCS file: /var/CVS/eric_firmware/linuxnew/drivers/net/ibm_emac/ibm_ocp_phy.c,v
retrieving revision 1.1.23.1
diff -u -r1.1.23.1 ibm_ocp_phy.c
--- drivers/net/ibm_emac/ibm_ocp_phy.c	4 Sep 2003 12:27:54 -0000	1.1.23.1
+++ drivers/net/ibm_emac/ibm_ocp_phy.c	8 Oct 2003 07:59:34 -0000
@@ -151,16 +151,17 @@

 static int genmii_read_link(struct mii_phy *phy)
 {
-	u16 lpa;
+	u16 adv, lpa;

 	if (phy->autoneg) {
+		adv = phy_read(phy, MII_ADVERTISE);
 		lpa = phy_read(phy, MII_LPA);

-		if (lpa & (LPA_10FULL | LPA_100FULL))
+		if ((lpa & adv) & (LPA_10FULL | LPA_100FULL))
 			phy->duplex = DUPLEX_FULL;
 		else
 			phy->duplex = DUPLEX_HALF;
-		if (lpa & (LPA_100FULL | LPA_100HALF))
+		if ((lpa & adv) & (LPA_100FULL | LPA_100HALF))
 			phy->speed = SPEED_100;
 		else
 			phy->speed = SPEED_10;

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

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

* Re: Fix for autonegotiation of ppc 40x ethernet driver
  2003-10-08  8:17 Fix for autonegotiation of ppc 40x ethernet driver Ronald Wahl
@ 2003-10-08  8:41 ` Benjamin Herrenschmidt
  2003-10-08  8:51   ` Ronald Wahl
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2003-10-08  8:41 UTC (permalink / raw)
  To: Ronald Wahl; +Cc: linuxppc-dev list


On Wed, 2003-10-08 at 10:17, Ronald Wahl wrote:
> Hello,
>
> I have found a bug in the ethernet driver for the PPC 40x. The problem
> is that the driver doesn't deal correctly if a user wants a limited
> advertising range (via ethtool interface) - lets say 10Mbps/Halfduplex
> and 10MBps/Fullduplex. The link always enters 100MBps/Fullduplex mode if
> the link partner supports it. This is an incorrect behavior. I appended
> a fix for this (against 2.4.22+ of the linuxppc 3.4 devel tree). It
> would be nice if this change could be reviewed and incorporated into the
> official code. If any questions arise - just ask...

Hi !

There may well be a bug in the code, but I'm not sure this is
the proper fix. When setting up a forced mode, I'm not sure it's
worth playing with advertise at all in fact... Just having the
link up shall be enough if aneg is disabled, we could ignore
LPA/ADVERTISE completely. Or maybe just read back BMCR from
read_link...

Ben.

> - ron
>
> Index: drivers/net/ibm_emac/ibm_ocp_enet.c
> ===================================================================
> RCS file: /var/CVS/eric_firmware/linuxnew/drivers/net/ibm_emac/ibm_ocp_enet.c,v
> retrieving revision 1.1.23.1
> diff -u -r1.1.23.1 ibm_ocp_enet.c
> --- drivers/net/ibm_emac/ibm_ocp_enet.c	4 Sep 2003 12:27:54 -0000	1.1.23.1
> +++ drivers/net/ibm_emac/ibm_ocp_enet.c	8 Oct 2003 07:59:34 -0000
> @@ -928,8 +928,7 @@
>  	int forced_duplex;
>
>  	/* Default advertise */
> -	advertise = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
> -		    ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full;
> +	advertise = fep->advertising;
>  	autoneg = fep->want_autoneg;
>  	forced_speed = fep->phy_mii.speed;
>  	forced_duplex = fep->phy_mii.duplex;
> @@ -938,6 +937,7 @@
>  	if (ep) {
>  	    if (ep->autoneg == AUTONEG_ENABLE) {
>  		advertise = ep->advertising;
> +		fep->advertising = advertise;
>  		autoneg = 1;
>  	    } else {
>  		autoneg = 0;
> @@ -1114,7 +1114,7 @@
>  emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>  {
>  	struct ocp_enet_private *fep = dev->priv;
> -	uint *data = (uint *) & rq->ifr_data;
> +	ushort *data = (ushort *) & rq->ifr_data;
>
>  	switch (cmd) {
>          case SIOCETHTOOL:
> @@ -1367,8 +1367,12 @@
>  	if (ep->phy_mii.def->ops->init)
>  	        ep->phy_mii.def->ops->init(&ep->phy_mii);
>  	netif_carrier_off(ndev);
> -        if (ep->phy_mii.def->features & SUPPORTED_Autoneg)
> +        if (ep->phy_mii.def->features & SUPPORTED_Autoneg) {
>                  ep->want_autoneg = 1;
> +		ep->advertising =
> +			ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
> +			ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full;
> +	}
>  	emac_start_link(ep, NULL);
>
>
> Index: drivers/net/ibm_emac/ibm_ocp_enet.h
> ===================================================================
> RCS file: /var/CVS/eric_firmware/linuxnew/drivers/net/ibm_emac/ibm_ocp_enet.h,v
> retrieving revision 1.1.23.1
> diff -u -r1.1.23.1 ibm_ocp_enet.h
> --- drivers/net/ibm_emac/ibm_ocp_enet.h	4 Sep 2003 12:27:54 -0000	1.1.23.1
> +++ drivers/net/ibm_emac/ibm_ocp_enet.h	8 Oct 2003 07:59:34 -0000
> @@ -136,6 +136,7 @@
>  	struct mii_phy phy_mii;
>          int mii_phy_addr;
>          int want_autoneg;
> +	u32 advertising;
>          int timer_ticks;
>  	struct timer_list link_timer;
>  	struct net_device *mdio_dev;
> Index: drivers/net/ibm_emac/ibm_ocp_phy.c
> ===================================================================
> RCS file: /var/CVS/eric_firmware/linuxnew/drivers/net/ibm_emac/ibm_ocp_phy.c,v
> retrieving revision 1.1.23.1
> diff -u -r1.1.23.1 ibm_ocp_phy.c
> --- drivers/net/ibm_emac/ibm_ocp_phy.c	4 Sep 2003 12:27:54 -0000	1.1.23.1
> +++ drivers/net/ibm_emac/ibm_ocp_phy.c	8 Oct 2003 07:59:34 -0000
> @@ -151,16 +151,17 @@
>
>  static int genmii_read_link(struct mii_phy *phy)
>  {
> -	u16 lpa;
> +	u16 adv, lpa;
>
>  	if (phy->autoneg) {
> +		adv = phy_read(phy, MII_ADVERTISE);
>  		lpa = phy_read(phy, MII_LPA);
>
> -		if (lpa & (LPA_10FULL | LPA_100FULL))
> +		if ((lpa & adv) & (LPA_10FULL | LPA_100FULL))
>  			phy->duplex = DUPLEX_FULL;
>  		else
>  			phy->duplex = DUPLEX_HALF;
> -		if (lpa & (LPA_100FULL | LPA_100HALF))
> +		if ((lpa & adv) & (LPA_100FULL | LPA_100HALF))
>  			phy->speed = SPEED_100;
>  		else
>  			phy->speed = SPEED_10;
>
--
Benjamin Herrenschmidt <benh@kernel.crashing.org>


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

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

* Re: Fix for autonegotiation of ppc 40x ethernet driver
  2003-10-08  8:41 ` Benjamin Herrenschmidt
@ 2003-10-08  8:51   ` Ronald Wahl
  2003-10-08  8:56     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 4+ messages in thread
From: Ronald Wahl @ 2003-10-08  8:51 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list


On Wed, 08 Oct 2003 10:41:31 +0200, Benjamin Herrenschmidt wrote:

> On Wed, 2003-10-08 at 10:17, Ronald Wahl wrote:
>> Hello,
>>
>> I have found a bug in the ethernet driver for the PPC 40x. The problem
>> is that the driver doesn't deal correctly if a user wants a limited
>> advertising range (via ethtool interface) - lets say 10Mbps/Halfduplex
>> and 10MBps/Fullduplex. The link always enters 100MBps/Fullduplex mode if
>> the link partner supports it. This is an incorrect behavior. I appended
>> a fix for this (against 2.4.22+ of the linuxppc 3.4 devel tree). It
>> would be nice if this change could be reviewed and incorporated into the
>> official code. If any questions arise - just ask...

> Hi !

> There may well be a bug in the code, but I'm not sure this is
> the proper fix. When setting up a forced mode, I'm not sure it's

I wont force a single mode. I want to limit the advertising to a
limited _range_. So autonegotiation is still enabled - just with a
limited range. The forcing of a mode (i.e. disabling autonegotiation)
is working already without this patch.

> worth playing with advertise at all in fact... Just having the
> link up shall be enough if aneg is disabled, we could ignore
> LPA/ADVERTISE completely. Or maybe just read back BMCR from
> read_link...

- ron

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

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

* Re: Fix for autonegotiation of ppc 40x ethernet driver
  2003-10-08  8:51   ` Ronald Wahl
@ 2003-10-08  8:56     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2003-10-08  8:56 UTC (permalink / raw)
  To: Ronald Wahl; +Cc: linuxppc-dev list


> I wont force a single mode. I want to limit the advertising to a
> limited _range_. So autonegotiation is still enabled - just with a
> limited range. The forcing of a mode (i.e. disabling autonegotiation)
> is working already without this patch.

Ah ok. Well... we indeed need to and the two masks then, though it
should be possible to find a phy-specific way to get the exact
speed that was actually selected...

Ben.


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

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

end of thread, other threads:[~2003-10-08  8:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-10-08  8:17 Fix for autonegotiation of ppc 40x ethernet driver Ronald Wahl
2003-10-08  8:41 ` Benjamin Herrenschmidt
2003-10-08  8:51   ` Ronald Wahl
2003-10-08  8:56     ` Benjamin Herrenschmidt

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