On Tuesday 27 March 2007 3:05am, Mike Isely wrote:
> On Mon, 26 Mar 2007, Marr wrote:
> > Greetings,
> >
> > *** Please CC: me on replies -- I'm not subscribed.
> >
> > A patch by Mike Isely that went into what became kernel 2.6.19 has
> > modified 'cypress_m8.c' in such a way that it makes the DeLorme Earthmate
> > LT-20 USB GPS receiver stop working entirely.
> >
> > After a little digging, I narrowed the problem down to a single line of
> > code which, if disabled, restores the full functionality of the
> > aforementioned LT-20 GPS.
> >
> > The offending line is in the 'cypress_serial_control()' function:
> >
> >    cypress_set_dead(port);
> >
> > This occurs at line number 407 in the vanilla 2.6.20 kernel's code.
> >
> > Basically, it's in this block of code (with my hack to disable the
> > problematic line):
> >
> >     if (retval != 5) {
> >             err("%s - failed to retrieve serial line settings - %d", 
> > __FUNCTION__,
> > retval);
> >             // cypress_set_dead(port);
> >             return retval;
> >     } else {
>
> This code change was an attempt to curtail the "damage" when something
> went wrong.  Without this change on my system at least things went into
> a spin and sucked down the CPU.  The idea here was that if the port is
> going to behave badly to at least cause it to fail gracefully.

Hi, Mike. Thanks for the info.

However, I'm still left wondering what your actual device is that uses the 
'cypress_m8' module.

As near as I can see, the Earthmate LT-20 GPS will never pass that check and, 
like the 'cypress_m8' driver did before your patch (and does now, after my 
"patch"), it should not call 'cypress_set_dead()' at all at that particular 
point. It should just report the problem (as it did then and does now 
[2.6.19]) and continue (as it did then).

Do you have some device which would _not_ work with that troublesome line 
disabled?

Or are you also using an LT-20 GPS which is working (with the new 2.6.19 
version of 'cypress_m8'), thereby implicating something different in our 2 
host systems (hardware and/or software)? BTW, Slackware 11.0 on a Toshiba 
Tecra A8-EZ8412 (Intel Core2 Duo laptop) here.

I'm a bit puzzled because, IIRC, Lonnie Mendez created the 'cypress_m8' driver 
specifically for the Earthmate LT-20 GPS. Am I the only one having trouble 
with that GPS and the newer (2.6.19 and later kernels) 'cypress_m8' module?

> If you are already getting to this point, then something bad has already
> happened.  Probably it would be a good idea to figure out how you are
> getting there in the first place.

If you're successfully running an LT-20 (i.e. with 2.6.19 or later), then I'll 
gladly investigate further. If not, however, then I've probably done pretty 
much all the experimenting I intend to, since I can fix the problem quite 
nicely by excising that one line.

I just wanted to report this to you and to the 'linux-usb-devel' list in case 
a fix which is more elegant than my quick hack could be made. Not knowing 
what device you're using, maybe just a specific device check at that point 
would suffice?

In general, I'm guessing (based on your patches) that your understanding of 
the 'cypress_m8' driver far exceeds mine. I'm willing to test things as 
needed, though.

> In my case, this change was one of (IIRC) 3 changes.  I first made some
> changes that cause the driver to behave better when it goes awry, then I
> attacked the underlying problem and fixed the reason why it was behaving
> badly in the first place.  For my situation (again, IIRC) it was because
> the driver was disregarding the polling rate reported by the hardware,
> causing the USB core to poll it too quickly, which then caused data
> lossage, the cypress hardware to hang, and then the driver went nuts
> retrying forever an operation that had no chance of ever succeeding.
> In fact, the misbehavior was so bad I think that it made the module
> unremovable - needing a reboot.

Before I discovered the "real" source of the problem, I did play around with 
the polling rate but still could not get the LT-20 to work at all by doing 
so.

> I'm fairly sure I commented this in the patches I had submitted.

Yes, there were comments in the 'ChangeLog-2.6.19' file, but none of which 
indicated the actual device you're using (not to imply that such information 
was required in a changelog). I did understand what you were aiming for, 
though.

> If you have a suggested additional fix, let me know and I can still try
> it on my hardware.

Any chance you could try disabling the line I did and running that new module 
on any/all of your 'cypress_m8'-using devices?

Also, just out of curiosity, how reliably could you reproduce (i.e. using the 
old, pre-2.6.19 code) the problem which prompted you to write those patches 
in the first place?

> > Also, it's not a huge issue, but when rather large changes go into a
> > kernel module, shouldn't the module version be incremented?  The
> > 'cypress_m8' module stayed at "v1.09" throughout these substantial (IMHO)
> > changes.
>
> I didn't consider these changes to be very large.  They were mostly
> surgical by nature.

Based on Greg K-H's reply, I'll accept "unchanged module version" as 
semi-standard practice in the (Linux) kernel module world. But, IMHO, such 
module authors might as well put "v.m." in that field -- "virtually 
meaningless". :^)  I mean, if the module version doesn't change whenever the 
code changes, it makes it useless to check '/var/log/messages' to see what 
version of a driver is running, since 2 very differently-behaving modules 
will report the same version. As I already said, "it's not a huge issue", 
just bad practice, IMHO. Not picking on you specifically, Mike -- it seems 
widespread.

Thanks again for your insight, Mike -- much appreciated!

Bill Marr

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to