Re: RFC: ti_usb-serial: userspace firmware, internal cfg. change, cleanup

== Wed, Nov 14, 2007 at 07:54:16AM -0800, Greg KH ==
> On Wed, Nov 14, 2007 at 04:47:55PM +0100, Oleg Verych wrote:
> > Al, Greg.
> > 
> > This is all-in-one patch for show.
> 
> Please run this through scripts/checkpatch.pl first and fix all of those
> warnings.

How nice, you have time to write this to me. I wonder how frequently you
did same for last, say, 9 years with your USB writings.

Hopefully i still have perl somewhere:

==
olecom@flower:/mnt/work/pc/kernel.org/linux-2.6.22/scripts$ perl checkpatch.pl /tmp/ti-usb/v94_ti-usbserial.patch
Must be run from the top-level dir. of a kernel tree
olecom@flower:/mnt/work/pc/kernel.org/linux-2.6.22/scripts$ perl checkpatch.pl  usage: checkpatch.pl [options] patchfile
version: 0.06
options: -q           => quiet
         --no-tree    => run without a kernel tree
olecom@flower:/mnt/work/pc/kernel.org/linux-2.6.22/scripts$ perl checkpatch.pl --no-tree /tmp/ti-usb/v94_ti-usbserial.patch
labels should not be indented
#369: FILE: drivers/usb/serial/ti_usb_3410_5052.c:1639:
+       ret_cleanup_firmware:

Missing Signed-off-by: line(s)
Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
olecom@flower:/mnt/work/pc/kernel.org/linux-2.6.22/scripts$
==

I use reasonably good editor for my stuff, and hopefully i need no
political morales. Please don't ask to use new version of that script.
It has its pros. and cons. Let's make drivers actually working, please.

> Also, please split up the whitespace changes from the real changes so
> that it is easier to review what is actually changed here.

They are not problem for testing or skipping, when looking onto important
changes. linux/scripts/cleanfile on usb sub-tree once is better, than
multiple do-as-i-say.

> > o  File 'tiusb3410.i51' or  'tiusb5052.i51' is being requested from the
> >    userspace, usually '/lib/firmware/'.
> 
> Where does this file come from?  Is it shipped by TI anywhere?

Usually, when you get a device, you get working drivers for number of
OSes. And, or miracle!, they have firmware in nice, up-to-date files, one
can symlink to /lib/firmware/. But no, Linux will have finally 5
firmwares in one, non working driver with dark and mysterious hotplug
script.

> > o  Reconfiguration is done internally.
> > 
> > o  kmalloc() isn't used for copying-copying of the binary. Optimistic
> >    allocation always requires 4 pages for maximum 16k of data.
> 
> What does this help with?

Speed. Speed to copy-copy-copy one blob over and over again from stupid
interfaces, like request_firmware().

> > o  git-nuke ti_fw_3410.h ti_fw_5052.h
> > 
> > o  Base Linux version is 2.6.22 with Oliver's 'usb-serial.c' fix. Tested
> >    with addition of pl2303-based cabel.
> 
> Can you please base this on 2.6.24-rc2, there's not much we can do with
> patches for 2.6.22.

I am pretty sure it will apply nicely to dusty usb-serial stuff.

> What does pl2303 have to do with this driver?

usb-serial.c has problems with multiple devices, when
usb_driver_set_configuration() is used. Yesterday's e-mail explained
this. I did my test, and all seems to be OK.

BTW, while reading data from pl2303-cabel, kernel oopses when device is
unplugged. But i'm sure it's OK.

> Please do not #define usb, that's ripe for confusion.

OK.

Thanks!
____

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel