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

[Jon Ringle]

On Fri, Apr 25, 2014 at 10:24 AM, Charles Coldwell <coldwell@xxxxxxxxx> wrote:
> 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.

However, serial8250_interrupt() which is where port->handle_irq() is
called is not a threaded irq.

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

I have re-written this driver 4 times already to get to the version
that is being presented. I don't have the time or energy to undergo
yet another rewrite of this driver. If you'd like to take on this
task, be my guest. I'd be happy to test what you come up with on my
hardware.

Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/