linux-kernel - Re: [PATCH v7 1/2] serial: sc16is7xx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Apr 2014 10:41:39 -0400
From:	Jon Ringle <jon@...gle.org>
To:	Charles Coldwell <coldwell@...il.com>
Cc:	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Greg KH <gregkh@...uxfoundation.org>,
	Alexander Shiyan <shc_work@...l.ru>,
	Jon Ringle <jringle@...dpoint.com>
Subject: Re: [PATCH v7 1/2] serial: sc16is7xx

On Fri, Apr 25, 2014 at 10:24 AM, Charles Coldwell <coldwell@...il.com> wrote:
> On Fri, 25 Apr 2014, Jon Ringle wrote:
>
>> On Fri, Apr 25, 2014 at 9:44 AM, Charles Coldwell <coldwell@...il.com> wrote:
>> > On Fri, 25 Apr 2014, Charles Coldwell wrote:
>> >
>> >> On Thu, 24 Apr 2014, jon@...gle.org 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/