2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver

2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver

[Eric Valette]

Andrew,

I tried your new test kernel and it broke my USB/Ethernet adapter. 
Adapter is detected, ifup works but no ping using IP adress on a point 
to point ethernet network. I saw the file change in the diff and 
probably something broke (either bogus endianness fixes or changed reset 
code data or ...). Bitkeeper being unreachable I can hardly follow what 
incremental broke it but, for sure, it is broken (FYI 2.6.9-rc1-mm2 works).

I also have oops trace when rebooting but cannot read as machines 
reboots. Will try network console when the rest is fixed :-)

-- 
    __
   /  `                   	Eric Valette
  /--   __  o _.          	6 rue Paul Le Flem
(___, / (_(_(__         	35740 Pace

Tel: +33 (0)2 99 85 26 76	Fax: +33 (0)2 99 85 26 76
E-mail: eric.valette@free.fr

Re: 2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver

[Eric Valette]

Eric Valette wrote:
> Andrew,
> 
> I tried your new test kernel and it broke my USB/Ethernet adapter. 
> Adapter is detected, ifup works but no ping using IP adress on a point 
> to point ethernet network. I saw the file change in the diff and 
> probably something broke (either bogus endianness fixes or changed reset 
> code data or ...). Bitkeeper being unreachable I can hardly follow what 
> incremental broke it but, for sure, it is broken (FYI 2.6.9-rc1-mm2 works).
> 
> I also have oops trace when rebooting but cannot read as machines 
> reboots. Will try network console when the rest is fixed :-)

Just to confirm : reverting the rtl8150.c diff makes the card 
functionnal again.

-- 
    __
   /  `                   	Eric Valette
  /--   __  o _.          	6 rue Paul Le Flem
(___, / (_(_(__         	35740 Pace

Tel: +33 (0)2 99 85 26 76	Fax: +33 (0)2 99 85 26 76
E-mail: eric.valette@free.fr

Re: 2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver : fixed

[Eric Valette]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

Eric Valette wrote:


> I tried your new test kernel and it broke my USB/Ethernet adapter. 
> Adapter is detected, ifup works but no ping using IP adress on a point 
> to point ethernet network. I saw the file change in the diff and 
> probably something broke (either bogus endianness fixes or changed reset 
> code data or ...). Bitkeeper being unreachable I can hardly follow what 
> incremental broke it but, for sure, it is broken (FYI 2.6.9-rc1-mm2 works).

Andrew,

Here is a small patch that makes the card functionnal again. I've 
forwarded the patch to driver author also.

Signed off by <eric.valette@free.fr>

Move value used to reset the card back to its previous definition.

-- 
    __
   /  `                   	Eric Valette
  /--   __  o _.          	6 rue Paul Le Flem
(___, / (_(_(__         	35740 Pace

Tel: +33 (0)2 99 85 26 76	Fax: +33 (0)2 99 85 26 76
E-mail: eric.valette@free.fr




[-- Attachment #2: patch_fix_rtl8150.c --]
[-- Type: text/x-csrc, Size: 319 bytes --]

--- linux/drivers/usb/net/rtl8150.c-2.6.9-rc1-mm4.orig	2004-09-09 11:15:11.000000000 +0200
+++ linux/drivers/usb/net/rtl8150.c	2004-09-09 11:15:46.000000000 +0200
@@ -341,7 +341,7 @@
 
 static int rtl8150_reset(rtl8150_t * dev)
 {
-	u8 data = 0x11;
+	u8 data = 0x10;
 	int i = HZ;
 
 	set_registers(dev, CR, 1, &data);

Re: 2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver : fixed

[Andrew Morton]

Eric Valette <eric.valette@free.fr> wrote:
>
> Here is a small patch that makes the card functionnal again. I've 
> forwarded the patch to driver author also.
> 
> --- linux/drivers/usb/net/rtl8150.c-2.6.9-rc1-mm4.orig	2004-09-09 11:15:11.000000000 +0200
> +++ linux/drivers/usb/net/rtl8150.c	2004-09-09 11:15:46.000000000 +0200
> @@ -341,7 +341,7 @@
>  
>  static int rtl8150_reset(rtl8150_t * dev)
>  {
> -	u8 data = 0x11;
> +	u8 data = 0x10;

hm, OK.  Presumably the change (which comes in via the bk-usb tree) was
made for a reason.  So I suspect both versions are wrong ;)

But it might be risky for Greg to merge this patch up at present.

Re: 2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver : fixed

[Greg KH]

On Thu, Sep 09, 2004 at 03:24:54PM -0700, Andrew Morton wrote:
> Eric Valette <eric.valette@free.fr> wrote:
> >
> > Here is a small patch that makes the card functionnal again. I've 
> > forwarded the patch to driver author also.
> > 
> > --- linux/drivers/usb/net/rtl8150.c-2.6.9-rc1-mm4.orig	2004-09-09 11:15:11.000000000 +0200
> > +++ linux/drivers/usb/net/rtl8150.c	2004-09-09 11:15:46.000000000 +0200
> > @@ -341,7 +341,7 @@
> >  
> >  static int rtl8150_reset(rtl8150_t * dev)
> >  {
> > -	u8 data = 0x11;
> > +	u8 data = 0x10;
> 
> hm, OK.  Presumably the change (which comes in via the bk-usb tree) was
> made for a reason.  So I suspect both versions are wrong ;)
> 
> But it might be risky for Greg to merge this patch up at present.

As all your patch does is revert the patch in my tree (it was a one line
change), mainline should work just fine for you, right?

I'll defer to Petkan as to what to do about this, as he sent me that
patch for a good reason I imagine :)

thanks,

greg k-h

Re: 2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver : fixed

[Eric Valette]

Greg KH wrote:
> On Thu, Sep 09, 2004 at 03:24:54PM -0700, Andrew Morton wrote:
> 
>>Eric Valette <eric.valette@free.fr> wrote:
>>
>>>Here is a small patch that makes the card functionnal again. I've 
>>>forwarded the patch to driver author also.
>>>
>>>--- linux/drivers/usb/net/rtl8150.c-2.6.9-rc1-mm4.orig	2004-09-09 11:15:11.000000000 +0200
>>>+++ linux/drivers/usb/net/rtl8150.c	2004-09-09 11:15:46.000000000 +0200
>>>@@ -341,7 +341,7 @@
>>> 
>>> static int rtl8150_reset(rtl8150_t * dev)
>>> {
>>>-	u8 data = 0x11;
>>>+	u8 data = 0x10;
>>
>>hm, OK.  Presumably the change (which comes in via the bk-usb tree) was
>>made for a reason.  So I suspect both versions are wrong ;)
>>
>>But it might be risky for Greg to merge this patch up at present.
> 
> 
> As all your patch does is revert the patch in my tree (it was a one line
> change), mainline should work just fine for you, right?
> 
> I'll defer to Petkan as to what to do about this, as he sent me that
> patch for a good reason I imagine :)

FYI : I already forwarded the patch to the author. Just copied LKML so 
that other can get the fix easily.

NB : I just reverted one small change, the rest of the changes (mainly 
access to driver private data, and IOCTL ops) are OK...

-- 
    __
   /  `                   	Eric Valette
  /--   __  o _.          	6 rue Paul Le Flem
(___, / (_(_(__         	35740 Pace

Tel: +33 (0)2 99 85 26 76	Fax: +33 (0)2 99 85 26 76
E-mail: eric.valette@free.fr

Re: 2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver : fixed

[Petko Manolov]

On Thu, 9 Sep 2004, Greg KH wrote:

> On Thu, Sep 09, 2004 at 03:24:54PM -0700, Andrew Morton wrote:
>> Eric Valette <eric.valette@free.fr> wrote:
>>>
>>> Here is a small patch that makes the card functionnal again. I've
>>> forwarded the patch to driver author also.
>>>
>>> --- linux/drivers/usb/net/rtl8150.c-2.6.9-rc1-mm4.orig	2004-09-09 11:15:11.000000000 +0200
>>> +++ linux/drivers/usb/net/rtl8150.c	2004-09-09 11:15:46.000000000 +0200
>>> @@ -341,7 +341,7 @@
>>>
>>>  static int rtl8150_reset(rtl8150_t * dev)
>>>  {
>>> -	u8 data = 0x11;
>>> +	u8 data = 0x10;
>>
>> hm, OK.  Presumably the change (which comes in via the bk-usb tree) was
>> made for a reason.  So I suspect both versions are wrong ;)
>>
>> But it might be risky for Greg to merge this patch up at present.
>
> As all your patch does is revert the patch in my tree (it was a one line
> change), mainline should work just fine for you, right?
>
> I'll defer to Petkan as to what to do about this, as he sent me that
> patch for a good reason I imagine :)

Steven Hein <ssh@sgi.com> sent me a patch that supposedly fix device 
registers misinitialization when it is being frequently reseted.

RTL8150 is quite flaky piece of HW so i first tested the new value and it 
did work for me.  That's why i decided to send it to Greg.

I would say lets wait for some time and see if we'll break someone else's
heart and then reverse the patch.  Another solution is to restore the 
original value and add new module parameter, so whoever thinks
anything != 0x10 work better for him will be free to change it.


 		Petko

Re: 2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver : fixed

[Eric Valette]

Petko Manolov wrote:

> Steven Hein <ssh@sgi.com> sent me a patch that supposedly fix device 
> registers misinitialization when it is being frequently reseted.

I never saw traces showing the card should be resetted. It hung 
occasionnaly in the past but did not know who to really blame (USB vs 
drivers). It is now  working well for quite a long time now allthough I 
periodically transfers more than 100mb via ftp...

> I would say lets wait for some time and see if we'll break someone else's
> heart and then reverse the patch.  Another solution is to restore the 
> original value and add new module parameter, so whoever thinks
> anything != 0x10 work better for him will be free to change it.

I'm not sure 2.6.9-rcx-mmx will provide valuable data, we will probably 
have to wait until distro incoporates this code to get some reasonnable 
hints... Do you have some well known driver users you could ping to get 
feedback more quickly?


Anyway, you are aware of the problem and I know how to fix my problem so 
all is fine.

Have a nice day,

-- 
    __
   /  `                   	Eric Valette
  /--   __  o _.          	6 rue Paul Le Flem
(___, / (_(_(__         	35740 Pace

Tel: +33 (0)2 99 85 26 76	Fax: +33 (0)2 99 85 26 76
E-mail: eric.valette@free.fr

Re: 2.6.9-rc1-mm4 badness in rtl8150.c ethernet driver : fixed

[Petko Manolov]

On Fri, 10 Sep 2004, Eric Valette wrote:

> Petko Manolov wrote:
>
> I never saw traces showing the card should be resetted. It hung occasionnaly 
> in the past but did not know who to really blame (USB vs drivers). It is now

Well, i haven't touched anything major in many many months. :-)

> working well for quite a long time now allthough I periodically transfers 
> more than 100mb via ftp...

That is good news, although many other people have made worse reports in 
the past.

> I'm not sure 2.6.9-rcx-mmx will provide valuable data, we will probably have 
> to wait until distro incoporates this code to get some reasonnable hints... 
> Do you have some well known driver users you could ping to get feedback more 
> quickly?

None that i am aware of.

> Anyway, you are aware of the problem and I know how to fix my problem so all 
> is fine.

All right - lets wait for a little while and see what sort info we'll get. 
Then we'll decide accordingly.


 		Petko

rtl8150.c ethernet driver : usb_unlink_urb ->usb_kill_urb

[Eric Valette]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]


> I'll defer to Petkan as to what to do about this, as he sent me that
> patch for a good reason I imagine :)

While we are looking at this driver, here is a way to avoid one full 
page of annoying messages at shutdown/module unload.

Signed-off-by: Eric Valette <Eric.Valette@free.fr>

-- 
    __
   /  `                   	Eric Valette
  /--   __  o _.          	6 rue Paul Le Flem
(___, / (_(_(__         	35740 Pace

Tel: +33 (0)2 99 85 26 76	Fax: +33 (0)2 99 85 26 76
E-mail: eric.valette@free.fr




[-- Attachment #2: patch_fix_rtl8150.c --]
[-- Type: text/x-csrc, Size: 319 bytes --]

--- linux/drivers/usb/net/rtl8150.c-2.6.9-rc1-mm4.orig	2004-09-09 11:15:11.000000000 +0200
+++ linux/drivers/usb/net/rtl8150.c	2004-09-09 11:15:46.000000000 +0200
@@ -341,7 +341,7 @@
 
 static int rtl8150_reset(rtl8150_t * dev)
 {
-	u8 data = 0x11;
+	u8 data = 0x10;
 	int i = HZ;
 
 	set_registers(dev, CR, 1, &data);

Re: rtl8150.c ethernet driver : usb_unlink_urb ->usb_kill_urb

[Eric Valette]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

Eric Valette wrote:
> 
>> I'll defer to Petkan as to what to do about this, as he sent me that
>> patch for a good reason I imagine :)
> 
> 
> While we are looking at this driver, here is a way to avoid one full 
> page of annoying messages at shutdown/module unload.
> 
> Signed-off-by: Eric Valette <Eric.Valette@free.fr>

Wrong patch, sorry...

-- 
    __
   /  `                   	Eric Valette
  /--   __  o _.          	6 rue Paul Le Flem
(___, / (_(_(__         	35740 Pace

Tel: +33 (0)2 99 85 26 76	Fax: +33 (0)2 99 85 26 76
E-mail: eric.valette@free.fr




[-- Attachment #2: patch_rtl8150.c.patch --]
[-- Type: text/x-patch, Size: 944 bytes --]

--- linux/drivers/usb/net/rtl8150.c-2.6.9-rc1-mm4.orig	2004-09-09 11:15:11.000000000 +0200
+++ linux/drivers/usb/net/rtl8150.c	2004-09-11 14:06:44.000000000 +0200
@@ -341,7 +341,7 @@
 
 static int rtl8150_reset(rtl8150_t * dev)
 {
-	u8 data = 0x11;
+	u8 data = 0x10;
 	int i = HZ;
 
 	set_registers(dev, CR, 1, &data);
@@ -389,10 +389,10 @@
 
 static void unlink_all_urbs(rtl8150_t * dev)
 {
-	usb_unlink_urb(dev->rx_urb);
-	usb_unlink_urb(dev->tx_urb);
-	usb_unlink_urb(dev->intr_urb);
-	usb_unlink_urb(dev->ctrl_urb);
+	usb_kill_urb(dev->rx_urb);
+	usb_kill_urb(dev->tx_urb);
+	usb_kill_urb(dev->intr_urb);
+	usb_kill_urb(dev->ctrl_urb);
 }
 
 static inline struct sk_buff *pull_skb(rtl8150_t *dev)
@@ -656,7 +656,7 @@
 	rtl8150_t *dev = netdev_priv(netdev);
 	warn("%s: Tx timeout.", netdev->name);
 	dev->tx_urb->transfer_flags |= URB_ASYNC_UNLINK;
-	usb_unlink_urb(dev->tx_urb);
+	usb_kill_urb(dev->tx_urb);
 	dev->stats.tx_errors++;
 }
 

Re: rtl8150.c ethernet driver : usb_unlink_urb ->usb_kill_urb

[Greg KH]

On Sat, Sep 11, 2004 at 07:33:17PM +0200, Eric Valette wrote:
> Eric Valette wrote:
> >
> >>I'll defer to Petkan as to what to do about this, as he sent me that
> >>patch for a good reason I imagine :)
> >
> >
> >While we are looking at this driver, here is a way to avoid one full 
> >page of annoying messages at shutdown/module unload.
> >
> >Signed-off-by: Eric Valette <Eric.Valette@free.fr>
> 
> Wrong patch, sorry...

Applied, thanks.

greg k-h