Lonnie --

Lonnie Mendez wrote:
> If that is all you guys can see wrong, I will resubmit it.

Great to have a new USB serial driver.  Thanks.  Here are some
comments...

Cypress_write function allocates a new urb for each write.
Without any limit on how much memory your driver can use, it
could potentially use up all available memory.  Something like
"cat /dev/zero > /dev/ttyUSB0" might show this.

I think it is better to preallocate some urbs and keep a list
of available urbs, taking them off the list to write and putting
them back on the list in the callback.

But at least, you should limit the number of outstanding urbs
you have allocated.

When you don't have any urbs available for a write, write should
return 0.

You will have to fix write_room once you change write.  Write_room
is supposed to return the number of bytes that can be successfully
written, writing any number of bytes at a time, without write
returning 0.  Since the write could be only one byte, and each write
takes an urb, write_room can only return the number of free urbs you
have (or plan to allocate).  The tty layer depends on write_room for
things like NL-->CR/NL mapping and tab expansion. If write_room lies,
those features don't work correctly.

Cypress_write will double free the urb if usb_submit_urb() fails.

You should set the current state to TASK_INTERRUPTIBLE before calling
schedule_timeout.  And you probably want to calculate the timeout based
on HZ, rather than just using 50 jiffies.

Also, throttle can be called from interrupts, and a synchronous
usb_unlink_urb, like you are using, could sleep.  That would oops.

-- Al



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to