[PATCH 0/3] sis190 gigabit fixes(bug 9735,11149) and new phy

[PATCH 0/3] sis190 gigabit fixes(bug 9735,11149) and new phy

[Riccardo Ghetta]

The first patch add a new phy found on my Asus F5Sr laptop. I suppose 
it's an Atheros chip, but really don't know.
The second patch should enable sis191 adapters to connect at Gb speeds
(at least, it does on my laptop). Also bumps the version number.
The third is just cosmetic, to make easier to identify newer phys.

Riccardo Ghetta (3):
  sis190: add new phy found on asus F5 series laptops
  sis190: fix gigabit negotiation
  sis190: better message on unknown PHY

 drivers/net/sis190.c |   53 +++++++++++++++++++++++++++++++++-----------------
 1 files changed, 35 insertions(+), 18 deletions(-)

[PATCH 1/3] sis190: add new phy found on asus F5 series laptops

[Riccardo Ghetta]

Signed-off-by: Riccardo Ghetta <birrachiara@tin.it>
---
 drivers/net/sis190.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
index 55ccd51..3d4542e 100644
--- a/drivers/net/sis190.c
+++ b/drivers/net/sis190.c
@@ -317,6 +317,7 @@ static struct mii_chip_info {
         unsigned int type;
 	u32 feature;
 } mii_chip_table[] = {
+	{ "Atheros PHY ?",        { 0x004d, 0xd010 }, LAN, 0 },
 	{ "Atheros PHY AR8012",   { 0x004d, 0xd020 }, LAN, 0 },
 	{ "Broadcom PHY BCM5461", { 0x0020, 0x60c0 }, LAN, F_PHY_BCM5461 },
 	{ "Broadcom PHY AC131",   { 0x0143, 0xbc70 }, LAN, 0 },
-- 
1.6.2.4

Re: [PATCH 1/3] sis190: add new phy found on asus F5 series laptops

[Francois Romieu]

Riccardo Ghetta <birrachiara@tin.it> :
> Signed-off-by: Riccardo Ghetta <birrachiara@tin.it>
> ---
>  drivers/net/sis190.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
> index 55ccd51..3d4542e 100644
> --- a/drivers/net/sis190.c
> +++ b/drivers/net/sis190.c
> @@ -317,6 +317,7 @@ static struct mii_chip_info {
>          unsigned int type;
>  	u32 feature;
>  } mii_chip_table[] = {
> +	{ "Atheros PHY ?",        { 0x004d, 0xd010 }, LAN, 0 },
>  	{ "Atheros PHY AR8012",   { 0x004d, 0xd020 }, LAN, 0 },
>  	{ "Broadcom PHY BCM5461", { 0x0020, 0x60c0 }, LAN, F_PHY_BCM5461 },
>  	{ "Broadcom PHY AC131",   { 0x0143, 0xbc70 }, LAN, 0 },

Please be more specific with the identifier of your laptop. Afaiu some
laptops in the F5 series include network components from Realtek. It could
be confusing.

You can remove the question mark. If we can only tell that the phy comes
from Atheros, so be it.

-- 
Ueimor

Re: [PATCH 1/3] sis190: add new phy found on asus F5 series laptops

[rgh]

Francois Romieu wrote:
> Please be more specific with the identifier of your laptop. Afaiu some
> laptops in the F5 series include network components from Realtek. It could
> be confusing.
> 
> You can remove the question mark. If we can only tell that the phy comes
> from Atheros, so be it.
Ok, I'll try to have a better description.
Thanks,
R

Re: [PATCH 1/3] sis190: add new phy found on asus F5 series laptops

[David Miller]

From: Riccardo Ghetta <birrachiara@tin.it>
Date: Wed, 3 Jun 2009 11:52:36 +0200

> Signed-off-by: Riccardo Ghetta <birrachiara@tin.it>

Please resubmit this whole patch series once you've
addressed all of the feedback.

Also, please fix the date on your computer :-)

[PATCH 2/3] sis190: fix gigabit negotiation

[Riccardo Ghetta]

Fixes an initialization error; the chip negotiates gigabit, but
the driver mistakenly handled it as 100Mb.
Hopefully should fix bugs 9735 and 11149

Signed-off-by: Riccardo Ghetta <birrachiara@tin.it>
---
 drivers/net/sis190.c |   37 +++++++++++++++++++++++++------------
 1 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
index 3d4542e..8322e28 100644
--- a/drivers/net/sis190.c
+++ b/drivers/net/sis190.c
@@ -47,7 +47,7 @@
 #define PHY_ID_ANY		0x1f
 #define MII_REG_ANY		0x1f
 
-#define DRV_VERSION		"1.2"
+#define DRV_VERSION		"1.3"
 #define DRV_NAME		"sis190"
 #define SIS190_DRIVER_NAME	DRV_NAME " Gigabit Ethernet driver " DRV_VERSION
 #define PFX DRV_NAME ": "
@@ -943,9 +943,9 @@ static void sis190_phy_task(struct work_struct *work)
 			u32 ctl;
 			const char *msg;
 		} reg31[] = {
-			{ LPA_1000XFULL | LPA_SLCT, 0x07000c00 | 0x00001000,
+			{ LPA_1000FULL, 0x07000c00 | 0x00001000,
 				"1000 Mbps Full Duplex" },
-			{ LPA_1000XHALF | LPA_SLCT, 0x07000c00,
+			{ LPA_1000HALF, 0x07000c00,
 				"1000 Mbps Half Duplex" },
 			{ LPA_100FULL, 0x04000800 | 0x00001000,
 				"100 Mbps Full Duplex" },
@@ -956,22 +956,35 @@ static void sis190_phy_task(struct work_struct *work)
 			{ LPA_10HALF, 0x04000400,
 				"10 Mbps Half Duplex" },
 			{ 0, 0x04000400, "unknown" }
- 		}, *p;
-		u16 adv;
+		}, *p = NULL;
+		u16 adv, autoexp, gigadv, gigrec;
 
 		val = mdio_read(ioaddr, phy_id, 0x1f);
 		net_link(tp, KERN_INFO "%s: mii ext = %04x.\n", dev->name, val);
 
 		val = mdio_read(ioaddr, phy_id, MII_LPA);
 		adv = mdio_read(ioaddr, phy_id, MII_ADVERTISE);
-		net_link(tp, KERN_INFO "%s: mii lpa = %04x adv = %04x.\n",
-			 dev->name, val, adv);
-
-		val &= adv;
+		autoexp = mdio_read(ioaddr, phy_id, MII_EXPANSION);
+		net_link(tp, KERN_INFO "%s: mii lpa=%04x adv=%04x exp=%04x.\n",
+			 dev->name, val, adv, autoexp);
+
+		if (val & ADVERTISE_NPAGE && autoexp & EXPANSION_NWAY) {
+			/* check for gigabit speed */
+			gigadv = mdio_read(ioaddr, phy_id, MII_CTRL1000);
+			gigrec = mdio_read(ioaddr, phy_id, MII_STAT1000);
+			val = (gigadv & (gigrec >> 2));
+			if (val & ADVERTISE_1000FULL)
+				p = reg31;
+			else if (val & ADVERTISE_1000HALF)
+				p = reg31+1;
+		}
+		if (!p) {
+			val &= adv;
 
-		for (p = reg31; p->val; p++) {
-			if ((val & p->val) == p->val)
-				break;
+			for (p = reg31; p->val; p++) {
+				if ((val & p->val) == p->val)
+					break;
+			}
 		}
 
 		p->ctl |= SIS_R32(StationControl) & ~0x0f001c00;
-- 
1.6.2.4

Re: [PATCH 2/3] sis190: fix gigabit negotiation

[Francois Romieu]

Riccardo Ghetta <birrachiara@tin.it> :
> Fixes an initialization error; the chip negotiates gigabit, but
> the driver mistakenly handled it as 100Mb.

Did you draw some inspiration from SiS own driver ? If so it would
deserve to be quoted.

> Hopefully should fix bugs 9735 and 11149

http://bugzilla.kernel.org/show_bug.cgi?id=9735
http://bugzilla.kernel.org/show_bug.cgi?id=11149

> 
> Signed-off-by: Riccardo Ghetta <birrachiara@tin.it>
> ---
>  drivers/net/sis190.c |   37 +++++++++++++++++++++++++------------
>  1 files changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
> index 3d4542e..8322e28 100644
> --- a/drivers/net/sis190.c
> +++ b/drivers/net/sis190.c
[...]
> @@ -956,22 +956,35 @@ static void sis190_phy_task(struct work_struct *work)
>  			{ LPA_10HALF, 0x04000400,
>  				"10 Mbps Half Duplex" },
>  			{ 0, 0x04000400, "unknown" }
> - 		}, *p;
> -		u16 adv;
> +		}, *p = NULL;
> +		u16 adv, autoexp, gigadv, gigrec;
>  
>  		val = mdio_read(ioaddr, phy_id, 0x1f);
>  		net_link(tp, KERN_INFO "%s: mii ext = %04x.\n", dev->name, val);
>  
>  		val = mdio_read(ioaddr, phy_id, MII_LPA);
>  		adv = mdio_read(ioaddr, phy_id, MII_ADVERTISE);
> -		net_link(tp, KERN_INFO "%s: mii lpa = %04x adv = %04x.\n",
> -			 dev->name, val, adv);
> -
> -		val &= adv;
> +		autoexp = mdio_read(ioaddr, phy_id, MII_EXPANSION);
> +		net_link(tp, KERN_INFO "%s: mii lpa=%04x adv=%04x exp=%04x.\n",
> +			 dev->name, val, adv, autoexp);
> +
> +		if (val & ADVERTISE_NPAGE && autoexp & EXPANSION_NWAY) {

val has been read from the LPA register.

s/ADVERTISE_NPAGE/LPA_NPAGE/ ?

> +			/* check for gigabit speed */
> +			gigadv = mdio_read(ioaddr, phy_id, MII_CTRL1000);
> +			gigrec = mdio_read(ioaddr, phy_id, MII_STAT1000);
> +			val = (gigadv & (gigrec >> 2));
> +			if (val & ADVERTISE_1000FULL)
> +				p = reg31;
> +			else if (val & ADVERTISE_1000HALF)
> +				p = reg31+1;
                                    ^^^^^^^ reg31 + 1

Otherwise ok.

-- 
Ueimor

Re: [PATCH 2/3] sis190: fix gigabit negotiation

[rgh]

Francois Romieu wrote:
> Riccardo Ghetta <birrachiara@tin.it> :
>> Fixes an initialization error; the chip negotiates gigabit, but
>> the driver mistakenly handled it as 100Mb.
> 
> Did you draw some inspiration from SiS own driver ? If so it would
> deserve to be quoted.
> 
Yes, I looked at both sis and forcedeth drivers. I'll quote both.

> val has been read from the LPA register.
> 
> s/ADVERTISE_NPAGE/LPA_NPAGE/ ?
Uhu, yes, sorry.

[PATCH 3/3] sis190: better message on unknown PHY

[Riccardo Ghetta]

Signed-off-by: Riccardo Ghetta <birrachiara@tin.it>
---
 drivers/net/sis190.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
index 8322e28..a69f3d2 100644
--- a/drivers/net/sis190.c
+++ b/drivers/net/sis190.c
@@ -348,7 +348,7 @@ static struct {
 	u32 msg_enable;
 } debug = { -1 };
 
-MODULE_DESCRIPTION("SiS sis190 Gigabit Ethernet driver");
+MODULE_DESCRIPTION("SiS sis190/191 Gigabit Ethernet driver");
 module_param(rx_copybreak, int, 0);
 MODULE_PARM_DESC(rx_copybreak, "Copy breakpoint for copy-only-tiny-frames");
 module_param_named(debug, debug.msg_enable, int, 0);
@@ -1329,12 +1329,15 @@ static void sis190_init_phy(struct net_device *dev, struct sis190_private *tp,
 			((mii_status & (BMSR_100FULL | BMSR_100HALF)) ?
 				LAN : HOME) : p->type;
 		tp->features |= p->feature;
-	} else
+		net_probe(tp, KERN_INFO "%s: %s transceiver at address %d.\n",
+			pci_name(tp->pci_dev), p->name, phy_id);
+	} else {
 		phy->type = UNKNOWN;
-
-	net_probe(tp, KERN_INFO "%s: %s transceiver at address %d.\n",
-		  pci_name(tp->pci_dev),
-		  (phy->type == UNKNOWN) ? "Unknown PHY" : p->name, phy_id);
+		net_probe(tp, KERN_INFO
+			"%s: unknown PHY 0x%x:0x%x transceiver at address %d\n",
+			pci_name(tp->pci_dev),
+			phy->id[0], (phy->id[1] & 0xfff0), phy_id);
+	}
 }
 
 static void sis190_mii_probe_88e1111_fixup(struct sis190_private *tp)
-- 
1.6.2.4

Re: [PATCH 3/3] sis190: better message on unknown PHY

[Francois Romieu]

Riccardo Ghetta <birrachiara@tin.it> :
[...]
> diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
> index 8322e28..a69f3d2 100644
> --- a/drivers/net/sis190.c
> +++ b/drivers/net/sis190.c
[...]
> @@ -1329,12 +1329,15 @@ static void sis190_init_phy(struct net_device *dev, struct sis190_private *tp,
>  			((mii_status & (BMSR_100FULL | BMSR_100HALF)) ?
>  				LAN : HOME) : p->type;
>  		tp->features |= p->feature;
> -	} else
> +		net_probe(tp, KERN_INFO "%s: %s transceiver at address %d.\n",
> +			pci_name(tp->pci_dev), p->name, phy_id);
> +	} else {
>  		phy->type = UNKNOWN;
> -
> -	net_probe(tp, KERN_INFO "%s: %s transceiver at address %d.\n",
> -		  pci_name(tp->pci_dev),
> -		  (phy->type == UNKNOWN) ? "Unknown PHY" : p->name, phy_id);
> +		net_probe(tp, KERN_INFO
> +			"%s: unknown PHY 0x%x:0x%x transceiver at address %d\n",
                                         ^^^^ ^^^^ 0x%04x:0x%04x ?
> +			pci_name(tp->pci_dev),
> +			phy->id[0], (phy->id[1] & 0xfff0), phy_id);
> +	}
>  }

Please make it the first in the series.

-- 
Ueimor