Re: [PATCH v7 1/2] serial: sc16is7xx

From: Alexander Shiyan
Date: Fri Apr 25 2014 - 10:35:48 EST


Fri, 25 Apr 2014 10:24:30 -0400 (EDT) ÐÑ Charles Coldwell <coldwell@xxxxxxxxx>:
> On Fri, 25 Apr 2014, Jon Ringle wrote:
>
> > On Fri, Apr 25, 2014 at 9:44 AM, Charles Coldwell <coldwell@xxxxxxxxx> wrote:
> > > On Fri, 25 Apr 2014, Charles Coldwell wrote:
> > >
> > >> On Thu, 24 Apr 2014, jon@xxxxxxxxxx wrote:
> > >>
> > >> > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> > >>
> > >> Isn't this a lot of duplication?
> > >
> > > Actually, the whole thing seems like duplication to me.
> >
> > The fact that we need to reach over the SPI/I2C bus makes a big
> > difference in the way access is handled.
> >
> > To achieve acceptable throughput, it is necessary to use threaded irq
> > and also bulk i2c transfers for RX and TX using
> > regmap_raw_{read,write}() to optimize the use of the i2c bus.
>
> Fair enough, but the 8250 framework does allow you to insert your own
> irq service routine. "serial8250_default_handle_irq" is the default
> (unsurprisingly), but if the uart_port has a non-NULL "handle_irq"
> method it will be faithfully copied into the uart_8250_port
> "handle_irq" method in 8250_core.c:early_serial_setup.
>
> > This is not a good fit for 8250.
>
> If that's really true, then I would say it argues in favor of a
> revision of the 8250 code. Certainly, this is not the last time that
> a 16550-compatible UART will appear on a non-PCI, non-ISA bus.

At this stage, I would suggest just move the driver code into serial/8250
to indicate the compatibility and then apply this patch and look for ways
to optimize similar functions.

---