I follow you comments...

Am Mittwoch 18 Juli 2007 schrieb eczema:
> +       if ((set == 0) && priv->TIOSTATUS == TIOCM_RTS) {
> +               dbg("%s TIOCMSET RESET called !!!", __FUNCTION__);
> +               priv->TIOSTATUS = 0;
> +               if (iuu_reset(port, 0x0C))
> +                       return -EIO;
> +               current->state = TASK_INTERRUPTIBLE;
> +               schedule_timeout(1 + 500 * HZ / 1000);

If you are really interrupted, you'll break the time requirements.


FIXED


+static int iuu_tiocmget(struct usb_serial_port *port, struct file 
+*file) {
+       u8 *st;
+       st = kmalloc(sizeof(u8), GFP_KERNEL);
+       iuu_status(port, st);

kmalloc() can fail.

+       if (st[0] & IUU_FULLCARD_IN) {
+               dbg("%s  card present ! value returned %i ", __FUNCTION__,
+                   TIOCM_CD);
+               kfree(st);
+               return 0;
+       } else {
+               kfree(st);
+               return TIOCM_CD;
+       }

You might unify the cleanup


UNIFIED by usinf int status = ....
And kfree , return status at end


+iuu_ioctl(struct usb_serial_port *port, struct file *file, unsigned int cmd,
+         unsigned long arg)
+{
+
+       int mask;
+
+       dbg("%s (%d) cmd = 0x%04x", __FUNCTION__, port->number, cmd);
+
+       get_user(mask, (unsigned long *)arg);

This can fail

Error handling added if (get_user...
                                        return -EFAULT;


+       case TCFLSH:
+               return (-1);

Meaning what?

FIXED with using -ENOIOCTLCMD



+       if (priv->write_busy)
+               return -1;



What for if you don't ever read write_busy?
Priv->write_busy = 1 if the routine is already writing. If not, 
priv->write_busy is 0 and the function continue normally..





+       tty->low_latency = 1;

Should be done in probe()
Added in iuu_open ( where I can access for the first time the tty pointer )


+       dbg("%s -  port %d", __FUNCTION__, port->number);
+       usb_clear_halt(serial->dev, port->write_urb->pipe);
+       usb_clear_halt(serial->dev, port->read_urb->pipe);

Is this really needed?
I'm not sure... say me....



> > +struct iuu_buffers {
> > +       u8 buf[256];
> > +       u8 finalbuf[256];
> > +       u8 dbgbuf[512];
> > +       u8 len;
> > +};
> > +
> 
> Is that safe? Kmalloc will give out chunks of memory safe for DMA, but will
> they be aligned?


I'm not a guru in alignment... so what's the best ? How to do it better ?

Alain

Attachment: iuu_phoenix.patch
Description: Binary data

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
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