Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1346751

Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x

Show all headers | View raw

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.

> > > +	len <<= 3;
> > 
> > Perhaps to define magic number (e.g. LP8841_DATA_LEN_SHIFT).
> 
> OK


> +	baud = tty_termios_baud_rate(termios);
> > > +
> > > +#ifdef BOTHER
> > > +	/* We only support fixed rates */

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

> > > 
> > 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.

> +
> > > +	/* We only support up to 115200 */
> > > +	if (baud > 115200) {
> > > +		baud = 115200;
> > > +		tty_termios_encode_baud_rate(termios, baud,
> > > baud);
> > > +	}
> 
> 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.
 tty_termios_encode_baud_rate(termios, baud, baud);

> +static int lp8841_serial_probe(struct platform_device *pdev)
> > > +{
> > > +     struct uart_8250_port uart = {};
> > 
> > {0}
> 
> ---
> 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

$ git grep -n 'struct .* = {0};' | wc -l
338

$ git grep -n 'struct .* = { \?0 \?};' | wc -l
550

( '… = { 0 };' included)

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

Either will work.

> Thanks for lightning fast reviews. I'll resubmit v8 if there is no
> objections to the points above.

See above.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v6] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-02-27 17:20 +0100
  Re: [PATCH v6] serial: support for 16550A serial ports on LP-8x4x Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-02-29 11:30 +0100
    Re: [PATCH v6] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-02-29 14:10 +0100
      Re: [PATCH v6] serial: support for 16550A serial ports on LP-8x4x One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-02-29 15:50 +0100
  [PATCH v7] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-02-29 22:30 +0100
    Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-03-01 12:10 +0100
      Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-03-01 17:30 +0100
        Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-03-01 17:50 +0100
          Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-03-01 18:20 +0100
            Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-03-01 18:50 +0100
              Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-03-01 19:50 +0100
              Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-03-01 20:30 +0100
                Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-03-01 21:00 +0100
    [PATCH v8] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-03-01 21:00 +0100
      [PATCH v9] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-03-01 21:10 +0100
        Re: [PATCH v9] serial: support for 16550A serial ports on LP-8x4x Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-03-01 21:30 +0100
        [PATCH v10] serial: support for 16550A serial ports on LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2016-03-01 22:30 +0100

csiph-web