lkml.org 
[lkml]   [2008]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] HSO: add option hso driver
    On Tue, May 13, 2008 at 11:27:27PM +0100, Alan Cox wrote:
    > > Jeff, please queue this up for 2.6.27 if there are no problems that you
    > > can see. It has been in the linux-next tree for a while, and many users
    > > are reporting that it is working for them.
    > >
    > > Or if you feel it's ok for 2.6.26, I will not object to that either :)
    >
    > Can we get it into 2.6.26 - its a driver for new hardware and the driver
    > is going to get a clean up for 2.6.27 anyway.
    >
    > Comments below are minor and I don't think blockers
    >
    > So for the serial side
    >
    > Acked-by: Alan Cox <alan@redhat.com>
    >
    >
    > > + termios = serial->tty->termios;
    >
    > tty is passed why keep using serial->tty (hint: think about parallel
    > hangup versus close/open)

    Ick, now fixed.

    > > +/* close the requested serial port */
    > > +static void hso_serial_close(struct tty_struct *tty, struct file *filp)
    > > +{
    > > + struct hso_serial *serial = tty->driver_data;
    > > + u8 usb_gone;
    > > +
    > > + D1("Closing serial port");
    > > +
    > > + /* sanity check */
    > > + if (tty == NULL || serial == NULL) {
    > > + D1("(tty == NULL || tty->driver_data == NULL)");
    > > + return;
    > > + }
    >
    > tty cannot be NULL here

    removed.

    > > +static int hso_serial_write(struct tty_struct *tty, const unsigned char *buf,
    > > + int count)
    > > +{
    > > + struct hso_serial *serial = get_serial_by_tty(tty);
    > > + int space, tx_bytes;
    > > + unsigned long flags;
    > > +
    > > + /* sanity check */
    > > + if (serial == NULL) {
    > > + printk(KERN_ERR "%s: tty or tty->driver_data is NULL\n",
    >
    > Checks one thing printks another 8)

    Fixed.

    > > + /* the actual setup */
    > > + spin_lock_irqsave(&serial->serial_lock, flags);
    > > + if (serial->open_count)
    > > + _hso_serial_set_termios(tty, old);
    >
    > (else *tty->termios = *old_termios)

    Added.

    Thanks for the review, I really appreciate it.

    greg k-h


    \
     
     \ /
      Last update: 2008-05-14 07:25    [W:0.022 / U:32.492 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site