lkml.org 
[lkml]   [2016]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x
    From
    Date
    On Tue, 2016-03-01 at 18:46 +0200, Andy Shevchenko wrote:
    > On Tue, 2016-03-01 at 19:25 +0300, Sergei Ianovich wrote:
    > > On Tue, 2016-03-01 at 13:06 +0200, Andy Shevchenko wrote:
    > > > On Tue, 2016-03-01 at 00:26 +0300, Sergei Ianovich wrote:
    >
    > > > > + len &= 3;
    >
    > Mask as well to be defined.

    Sure.

    > So, but if you support only fixed rates, why do you care about BOTHER
    > at all?

    If BOTHER is defined, tty_termios_baud_rate()
    and tty_termios_encode_baud_rate() allow non-standard baud rates. I
    should clear it from c_cflag to indicate I don't support it.

    > > > >  
    > > > I think you can call this unconditionally together with case >
    > > > 115200.
    > >
    > > The calls are orthogonal. This one deals with the case when BOTHER
    > > is
    > > defined and set, and we have non-zero rate with BOTHER, but we have
    > > zero rate after BOTHER is cleared. So we set 9600 as a sane default
    > > speed.

    > >
    > > This one deals with the case when the rate is over 115200. If the
    > > previous case has been triggered, this one won't be.
    >
    > Yeah, but I meant to unconditionally call it just once here every
    > time.

    I see. It saves a few lines.

    > > ---
    > > drivers/tty/serial/8250/8250_lp8841.c: In function
    > > 'lp8841_serial_probe':
    > > drivers/tty/serial/8250/8250_lp8841.c:124:32: warning: excess
    > > elements in struct initializer
    > >   struct uart_8250_port uart = {0};
    > >                                 ^
    > > drivers/tty/serial/8250/8250_lp8841.c:124:32: note: (near
    > > initialization for 'uart.port.lock.<anonymous>.rlock.raw_lock')
    >
    > Do you have any warning verbosity enabled? I see a lot of stuff like
    > this in the code

    Plain `make`.

    The warning seems to be the result of initializing a spinlock with
    zero. Spinlocks are intentionally obfuscated, but I didn't investigate
    further.

    > $ git grep -n 'struct .* = {0};' | wc -l
    > 338
    >
    > $ git grep -n 'struct .* = { \?0 \?};' | wc -l
    > 550
    >
    > ( '… = { 0 };' included)

    The first structure member is most likely not a spinlock in those
    cases.

    > > ---
    > >
    > > Zero triggers a warning. I'll use memset().
    >
    > Either will work.

    OK

    The only remaining open point is BOTHER handling.

    \
     
     \ /
      Last update: 2016-03-01 19:01    [W:4.205 / U:0.324 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site