> As a RFC, the driver doesn't have a valid UART port type yet, but borrow
> PORT_PXA. I would apply one if there is no main objection for it.

Please just add a new uart type for it.

> +config MAX3110_DESIGNWARE
> +     boolean "Enable Max3110 to work with Designware controller"
> +     default y
> +     depends on SERIAL_MAX3110
> +
> +config MAX3110_IRQ
> +     boolean "Enable IRQ for Max3110"
> +     default n
> +     depends on SERIAL_MAX3110
> +

These should be runtime


> +int max3110_write_then_read(struct uart_max3110 *max,
> +             const u8 *txbuf, u8 *rxbuf, unsigned len, int always_fast)
> +{
> +     struct spi_device *spi = max->spi;
> +     struct spi_message      message;
> +     struct spi_transfer     x;
> +     int ret;
> +
> +     if (!txbuf || !rxbuf)
> +             return -EINVAL;

How can the above occur - it seems no user triggered event can cause it
and almost all the paths then lose the error before userspace gets it
(and silently).


> +     if (len > MAX_READ_LEN) {
> +             pr_err(PR_FMT "read len %d is too large\n", len);
> +             return 0;
> +     }

Ditto


> +     while (len) {
> +             usable = tty_buffer_request_room(tty, len);
> +             if (usable) {
> +                     tty_insert_flip_string(tty, str, usable);
> +                     str += usable;
> +                     port->icount.rx += usable;
> +                     tty_flip_buffer_push(tty);

You really only want to push once. tty_insert_flip_string also knows
about multiple blocks so all of this can be a single

        tty_insert_flip_string(tty, str, len);
        tty_flip_buffer_push(tty);


>
> +static void
> +serial_m3110_set_termios(struct uart_port *port, struct ktermios *termios,
> +                    struct ktermios *old)
> +{
> +     struct uart_max3110 *max =
> +             container_of(port, struct uart_max3110, port);
> +     unsigned char cval;
> +     unsigned int baud, parity = 0;
> +     int clk_div = -1;
> +     u16 new_conf = max->cur_conf;
> +
> +     switch (termios->c_cflag & CSIZE) {
> +     case CS7:
> +             cval = UART_LCR_WLEN7;
> +             new_conf |= WC_7BIT_WORD;
> +             break;
> +     default:
> +     case CS8:
> +             cval = UART_LCR_WLEN8;
> +             new_conf |= WC_8BIT_WORD;

If you can only do CS8 when asked for other values you need to update the
termios struct to report that

                termios->c_cflag &= ~CSIZE;
                termios->c_flag |= CS8;

> +     if (!(termios->c_cflag & PARODD))
> +             parity |= UART_LCR_EPAR;
> +     max->parity = parity;
> +
> +     uart_update_timeout(port, termios->c_cflag, baud);
> +
> +     new_conf |= WC_TAG;
> +     if (new_conf != max->cur_conf) {
> +             max3110_out(max, new_conf);
> +             max->cur_conf = new_conf;
> +             max->baud = baud;
> +     }

And as you don't support it

        termios->c_cflag &= ~CMSPAR;


I'll take a more serious look over it in the new year when I'm back at
work.


------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Reply via email to