Re: sis190 driver

Re: sis190 driver

[Francois Romieu]

[bouncing @sis.com address removed from the Cc:]

Lars Vahlenberg <lars.vahlenberg@mandator.com> :
[...]
> I can get mii-tool to work with this patch, but if I have
> a ping command running and changing to another speed I
> stop receiving or get 1 - 3 sek pings. ei x000ms.

The current SiS driver is way more readable than the previous version
but I still have not finished to revamp its mii/phy init sequence:
- it's a bit ad hoc;
- it duplicates code here and there (see link timer and mii/phy init);
- imnsho, it tries to achieve too much work in the pci probe phase;
- it mostly ignores what is available in include/linux/mii.h and
  drivers/net/mii.c.

Expect something to test before the end of the week.

--
Ueimor

Re: sis190 driver

[Erior]

Hi

Added PHY identifier for the Asus K8S-MX motherboard.

--- sis190.old  2005-07-29 23:16:07.000000000 +0000
+++ sis190.c    2005-07-29 23:15:37.000000000 +0000
@@ -325,6 +325,7 @@ static struct mii_chip_info {
        { "Broadcom PHY BCM5461", { 0x0020, 0x60c0 }, LAN },
        { "Agere PHY ET1101B",    { 0x0282, 0xf010 }, LAN },
        { "Marvell PHY 88E1111",  { 0x0141, 0x0cc0 }, LAN },
+       { "Realtek PHY RTL8201CL",{ 0x0000, 0x8200 }, LAN },
        { NULL, }
 };

/Lars

On 7/29/05, Lars Vahlenberg <lars.vahlenberg@mandator.com> wrote:
> 
> 
> 
> -----Original Message-----
> From: Francois Romieu [mailto:romieu@fr.zoreil.com]
> Sent: Fri 2005-07-29 00:11
> To: linux-kernel@vger.kernel.org
> Cc: pascal.chapperon@wanadoo.fr; Lars Vahlenberg; Alexey Dobriyan;
> jgarzik@pobox.com
> Subject: [patch 2.6.13-rc3] sis190 driver
>  
> Single file patch:
> http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3-sis190-test.patch
> 
> Patch-kit:
> http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3/patches
> 
> Tarball:
> http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3.tar.bz2
> 
> Changes from previous version (20050722)
> o Add endian annotations (Alexey Dobriyan).
> 
> o Hopefully fixed the build of the patch.
> 
> o Minor round of mii/phy related changes. May crash.
> 
> Testing reports/review/patches are always appreciated.
> 
> Ok, now back to washing.
> 
> --
> Ueimor
> 
> 
> 
> 


-- 
There is nothing that cannot be solved through sufficient application
of brute force and ignorance.

Re: sis190 driver

[Erior]

This will get the driver to aknowledge mii-tool reported mode,
without this the drivers always says "100BaseTx-FD" while my switch
shows 10BaseT-HD as the selected mode.

/Lars
--- sis190.new  2005-07-29 23:26:39.000000000 +0000
+++ sis190.c    2005-07-29 23:38:39.000000000 +0000
@@ -956,7 +956,8 @@ static void sis190_phy_task(void * data)
                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);
+               val = mdio_read(ioaddr, phy_id, MII_LPA)
+                   & mdio_read(ioaddr, phy_id, MII_ADVERTISE );
                net_link(tp, KERN_INFO "%s: mii lpa = %04x.\n", dev->name, val);

                for (p = reg31; p->ctl; p++) {


On 7/29/05, Erior <lars.vahlenberg@gmail.com> wrote:
> Hi
> 
> Added PHY identifier for the Asus K8S-MX motherboard.
> 
> --- sis190.old  2005-07-29 23:16:07.000000000 +0000
> +++ sis190.c    2005-07-29 23:15:37.000000000 +0000
> @@ -325,6 +325,7 @@ static struct mii_chip_info {
>         { "Broadcom PHY BCM5461", { 0x0020, 0x60c0 }, LAN },
>         { "Agere PHY ET1101B",    { 0x0282, 0xf010 }, LAN },
>         { "Marvell PHY 88E1111",  { 0x0141, 0x0cc0 }, LAN },
> +       { "Realtek PHY RTL8201CL",{ 0x0000, 0x8200 }, LAN },
>         { NULL, }
>  };
> 
> /Lars
> 
> On 7/29/05, Lars Vahlenberg <lars.vahlenberg@mandator.com> wrote:
> > 
> > 
> > 
> > -----Original Message-----
> > From: Francois Romieu [mailto:romieu@fr.zoreil.com]
> > Sent: Fri 2005-07-29 00:11
> > To: linux-kernel@vger.kernel.org
> > Cc: pascal.chapperon@wanadoo.fr; Lars Vahlenberg; Alexey Dobriyan;
> > jgarzik@pobox.com
> > Subject: [patch 2.6.13-rc3] sis190 driver
> >  
> > Single file patch:
> >
> http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3-sis190-test.patch
> > 
> > Patch-kit:
> > http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3/patches
> > 
> > Tarball:
> > http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3.tar.bz2
> > 
> > Changes from previous version (20050722)
> > o Add endian annotations (Alexey Dobriyan).
> > 
> > o Hopefully fixed the build of the patch.
> > 
> > o Minor round of mii/phy related changes. May crash.
> > 
> > Testing reports/review/patches are always appreciated.
> > 
> > Ok, now back to washing.
> > 
> > --
> > Ueimor
> > 
> > 
> > 
> > 
> 
> 
> -- 
> There is nothing that cannot be solved through sufficient application
> of brute force and ignorance.
> 


-- 
There is nothing that cannot be solved through sufficient application
of brute force and ignorance.

Re: sis190 driver

[Erior]

Hi

I can change the speed with my changes however there is something wrong with
taking care of failures.

If we are sending something for example have a ping command running and change
the speed with mii-tool or disconnect the cable and connect it again, the count
on ifconfig says that we are sending packages though I don't know what because
nothing arrives to the destination machine..hmm.. I can try to dump
the the network traffic
on my 10Mbit hub if needed but I don't think we are sending anything
that makes sense.

Is there any kind of test or information I can provid to help you fixing this ?

/Lars

On 7/29/05, Erior <lars.vahlenberg@gmail.com> wrote:
> This will get the driver to aknowledge mii-tool reported mode,
> without this the drivers always says "100BaseTx-FD" while my switch
> shows 10BaseT-HD as the selected mode.
> 
> /Lars
> --- sis190.new  2005-07-29 23:26:39.000000000 +0000
> +++ sis190.c    2005-07-29 23:38:39.000000000 +0000
> @@ -956,7 +956,8 @@ static void sis190_phy_task(void * data)
>                 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);
> +               val = mdio_read(ioaddr, phy_id, MII_LPA)
> +                   & mdio_read(ioaddr, phy_id, MII_ADVERTISE );
>                 net_link(tp, KERN_INFO "%s: mii lpa = %04x.\n", dev->name,
> val);
> 
>                 for (p = reg31; p->ctl; p++) {
> 
> 
> On 7/29/05, Erior <lars.vahlenberg@gmail.com> wrote:
> > Hi
> > 
> > Added PHY identifier for the Asus K8S-MX motherboard.
> > 
> > --- sis190.old  2005-07-29 23:16:07.000000000 +0000
> > +++ sis190.c    2005-07-29 23:15:37.000000000 +0000
> > @@ -325,6 +325,7 @@ static struct mii_chip_info {
> >         { "Broadcom PHY BCM5461", { 0x0020, 0x60c0 }, LAN },
> >         { "Agere PHY ET1101B",    { 0x0282, 0xf010 }, LAN },
> >         { "Marvell PHY 88E1111",  { 0x0141, 0x0cc0 }, LAN },
> > +       { "Realtek PHY RTL8201CL",{ 0x0000, 0x8200 }, LAN },
> >         { NULL, }
> >  };
> > 
> > /Lars
> > 
> > On 7/29/05, Lars Vahlenberg <lars.vahlenberg@mandator.com> wrote:
> > > 
> > > 
> > > 
> > > -----Original Message-----
> > > From: Francois Romieu [mailto:romieu@fr.zoreil.com]
> > > Sent: Fri 2005-07-29 00:11
> > > To: linux-kernel@vger.kernel.org
> > > Cc: pascal.chapperon@wanadoo.fr; Lars Vahlenberg; Alexey Dobriyan;
> > > jgarzik@pobox.com
> > > Subject: [patch 2.6.13-rc3] sis190 driver
> > >  
> > > Single file patch:
> > >
> >
> http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3-sis190-test.patch
> > > 
> > > Patch-kit:
> > > http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3/patches
> > > 
> > > Tarball:
> > > http://www.zoreil.com/~romieu/sis190/20050728-2.6.13-rc3.tar.bz2
> > > 
> > > Changes from previous version (20050722)
> > > o Add endian annotations (Alexey Dobriyan).
> > > 
> > > o Hopefully fixed the build of the patch.
> > > 
> > > o Minor round of mii/phy related changes. May crash.
> > > 
> > > Testing reports/review/patches are always appreciated.
> > > 
> > > Ok, now back to washing.
> > > 
> > > --
> > > Ueimor
> > > 
> > > 
> > > 
> > > 
> > 
> > 
> > -- 
> > There is nothing that cannot be solved through sufficient application
> > of brute force and ignorance.
> > 
> 
> 
> -- 
> There is nothing that cannot be solved through sufficient application
> of brute force and ignorance.
> 


-- 
There is nothing that cannot be solved through sufficient application
of brute force and ignorance.

Re: sis190 driver

[Francois Romieu]

Erior <lars.vahlenberg@gmail.com> :
[...]
> Is there any kind of test or information I can provid to help you fixing
> this ?

It could help to know if the device reports a link event interrupt or such.
(ethtool allow to modify the log level of the driver if required). Don't
hesitate to publish a complete dmesg somewhere.

A generous amount of mii/phy related code remains to be merged from SiS
driver and the 8201 is labelled as requiring extra quirks in sis900.c.
It is not hopeless :o)

--
Ueimor

Re: sis190 driver

[Francois Romieu]

Erior <lars.vahlenberg@gmail.com> :
[...]
> Added PHY identifier for the Asus K8S-MX motherboard.

- is it ok to add a Signed-off-by: Lars Vahlenberg <lars.vahlenberg@gmail.com> ?

- how the whole driver work now ?

--
Ueimor

sis190 driver

[Francois Romieu]

No major change from previous version. I'm quietly merging bits from
the SiS driver that Lars kindly pointed out. The detection of the
mac address is done differently.

I'll welcome feedback related to regressions and/or netconsole testing.

Single file patch:
http://www.zoreil.com/~romieu/sis190/20050722-2.6.13-rc2-sis190-test.patch

Patch-kit:
http://www.zoreil.com/~romieu/sis190/20050722-2.6.13-rc2/patches

Tarball:
http://www.zoreil.com/~romieu/sis190/20050722-2.6.13-rc2.tar.bz2

--
Ueimor

Re: sis190 driver

[Alexey Dobriyan]

On Fri, Jul 22, 2005 at 01:09:50AM +0200, Francois Romieu wrote:
> No major change from previous version. I'm quietly merging bits from
> the SiS driver that Lars kindly pointed out. The detection of the
> mac address is done differently.
> 
> I'll welcome feedback related to regressions and/or netconsole testing.
> 
> Single file patch:
> http://www.zoreil.com/~romieu/sis190/20050722-2.6.13-rc2-sis190-test.patch

MAINTAINERS chunk isn't -p1 applicable. ;-)

sparse asks whether you have endianness bugs here:
----------------------------------------------------------------------------
   450	static inline void sis190_make_unusable_by_asic(struct RxDesc *desc)
   451	{
   452		desc->PSize = 0x0;
   453	===>	desc->addr = 0xdeadbeef;	<===
   454		desc->size &= cpu_to_le32(RingEnd);
   455		wmb();
   456		desc->status = 0x0;
   457	}

drivers/net/sis190.c:453:13: warning: incorrect type in assignment (different base types)
drivers/net/sis190.c:453:13:    expected restricted unsigned int [assigned] [usertype] addr
drivers/net/sis190.c:453:13:    got unsigned int
----------------------------------------------------------------------------
   544	static int sis190_rx_interrupt(struct net_device *dev,
   545				       struct sis190_private *tp, void __iomem *ioaddr)
   546	{

   554		for (; rx_left > 0; rx_left--, cur_rx++) {
   555			unsigned int entry = cur_rx % NUM_RX_DESC;
   556			struct RxDesc *desc = tp->RxDescRing + entry;
   557			u32 status;
   558	
   559		===>	if (desc->status & OWNbit)	<===
   560				break;

drivers/net/sis190.c:559:20: warning: incompatible types for operation (&)
drivers/net/sis190.c:559:20:    left side has type restricted unsigned int [assigned] [usertype] status
drivers/net/sis190.c:559:20:    right side has type unsigned int [unsigned] enum _DescStatusBit [unsigned] [toplevel] OWNbit
----------------------------------------------------------------------------
Add endian annotations.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

Index: linux-sis190/drivers/net/sis190.c
===================================================================
--- linux-sis190.orig/drivers/net/sis190.c	2005-07-22 07:56:37.000000000 +0400
+++ linux-sis190/drivers/net/sis190.c	2005-07-22 08:03:47.000000000 +0400
@@ -191,17 +191,17 @@ enum sis190_register_content {
 };
 
 struct TxDesc {
-	u32 PSize;
-	u32 status;
-	u32 addr;
-	u32 size;
+	__le32 PSize;
+	__le32 status;
+	__le32 addr;
+	__le32 size;
 };
 
 struct RxDesc {
-	u32 PSize;
-	u32 status;
-	u32 addr;
-	u32 size;
+	__le32 PSize;
+	__le32 status;
+	__le32 addr;
+	__le32 size;
 };
 
 enum _DescStatusBit {
@@ -1322,7 +1322,7 @@ static int __devinit sis190_get_mac_addr
 
 	/* Get MAC address from EEPROM */
 	for (i = 0; i < MAC_ADDR_LEN / 2; i++) {
-		u16 w = sis190_read_eeprom(ioaddr, EEPROMMACAddr + i);
+		__le16 w = sis190_read_eeprom(ioaddr, EEPROMMACAddr + i);
 
 		((u16 *)dev->dev_addr)[0] = le16_to_cpu(w);
 	}