Luiz Fernando N. Capitulino wrote:
> On Tue, 11 Jul 2006 14:36:31 -0300
> "Naranjo Manuel Francisco" <[EMAIL PROTECTED]> wrote:
> 
> | > | +static int aircable_write(struct usb_serial_port *port,
> | > | +                                     const unsigned char *source, int 
> count)
> | > | +{
> | >
> | > [...]
> | >
> | > | +
> | > | +     port->write_urb_busy = 1;
> | >
> | >  Shouldn't you protect this with the spinlock?
> | You mean making an spinlock before changing the value, and
> | a spinunlock after changing the value?
> 
>  Yes, but I just checked and it seems that all the drivers which uses it
> does exactly what you did.
> 
>  And now I wonder whether they're buggy or not.
Don't have even an Idea.

> 
> | > | +static void aircable_write_bulk_callback (struct urb *urb, struct 
> pt_regs *regs)
> | > | +{
> | > | +     struct usb_serial_port *port = (struct usb_serial_port 
> *)urb->context;
> | > | +
> | > | +     /* free up the transfer buffer, as usb_free_urb() does not do 
> this */
> | > | +     kfree(urb->transfer_buffer);
> | > | +
> | > | +     dbg("%s - port %d", __FUNCTION__, port->number);
> | > | +
> | > | +     port->write_urb_busy = 1;
> | >
> | >  What about this one?
> | >
> | >  I think there are others..
> | >
> | > | +static void aircable_read_bulk_callback(struct urb *urb, struct 
> pt_regs *regs)
> | > | +{
> | > | +     struct usb_serial_port *port = (struct usb_serial_port 
> *)urb->context;
> | > | +     struct usb_serial *serial = port->serial;
> | > | +     struct tty_struct *tty;
> | > | +     unsigned char *data;
> | > | +     unsigned long no_packages;
> | > | +     unsigned long remaining, package_length;
> | > | +     unsigned long i;
> | > | +     int result;
> | > | +
> | > | +     dbg("%s - port %d", __FUNCTION__, port->number);
> | > | +
> | > | +     if (urb->status) {
> | > | +             dbg("%s - nonzero read bulk status received: %d", 
> __FUNCTION__,
> | > | +                                                             
> urb->status);
> | > | +             return;
> | > | +     }
> | >
> | >  Some other drivers does this, but personally I dislike it.
> | >
> | >  IMO, you can ignore (ie, report with dbg()) -ECONNRESET, -ENOENT
> | > and -ESHUTDOWN. But all the others should be reported with err().
> | >
> | I can't get your point, do you mean in other parts of my code, or for
> | other opportunities?
> 
>  Everytime you check urb->status.
Ok I will change that.
Thanks
> 
>  Take a look in my pl2303_read_int_callback():
> 
> http://distro2.conectiva.com.br/~lcapitulino/patches/usbserial/2.6.17-rc5/serialcore-port-V0/0011-usbserial-pl2303-Ports-urb-callback-functions.txt
> 



-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
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