Reg: Serial port driver for microchip's new PCIe UART device

Reg: Serial port driver for microchip's new PCIe UART device

[Kumaravel.Thiagarajan]

Dear Greg KH,

I am Kumaravel Thiagarajan from Microchip India.

We are working on a PCIe based multi-instance UART device.
Based on the Linux community feedback few months back, we had written it as a custom driver inside drivers/tty/serial/8250.
Now this custom driver is requiring a DWORD FIFO access for both Tx and Rx, and I am in the process of changing my driver code.

Can I model my custom driver on serial drivers present in drivers/tty/serial/ directory?
I am implementing my functions for uart_ops structure and the necessary ISR in a separate file mchp_pci1xxxx_uart.c inside the drivers/tty/serial/ directory.

I wish this custom UART driver to get through Linux community submission.
Do you see any risk in this approach? Do you have any suggestions?

Thank You.

Regards,
Kumaravel Thiagarajan
Technical staff Engineer - Software
Microchip India Private Limited,
Chennai, India

Re: Reg: Serial port driver for microchip's new PCIe UART device

[Greg KH]

On Wed, Feb 09, 2022 at 10:38:34AM +0000, Kumaravel.Thiagarajan@microchip.com wrote:
> Dear Greg KH,
> 
> I am Kumaravel Thiagarajan from Microchip India.
> 
> We are working on a PCIe based multi-instance UART device.
> Based on the Linux community feedback few months back, we had written it as a custom driver inside drivers/tty/serial/8250.
> Now this custom driver is requiring a DWORD FIFO access for both Tx and Rx, and I am in the process of changing my driver code.

Why does the hardware not follow the normal standard here?

And are you sure it will still not fit into the 8250 format?

> Can I model my custom driver on serial drivers present in drivers/tty/serial/ directory?

You could, but it would be much smaller and easier to use the 8250
framework given that you probably do have an 8250-like device, right?

> I am implementing my functions for uart_ops structure and the necessary ISR in a separate file mchp_pci1xxxx_uart.c inside the drivers/tty/serial/ directory.
> 
> I wish this custom UART driver to get through Linux community submission.
> Do you see any risk in this approach? Do you have any suggestions?

Let's see the code before we can give you any firm answers.

good luck!

greg k-h

RE: Reg: Serial port driver for microchip's new PCIe UART device

[Kumaravel.Thiagarajan]

Dear Greg KH,

I discussed with our silicon architect @Richard Petrie - M18281 and our answers are inline below.
Please let us know if you need any further information.

Thank You.

Regards,
Kumar

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Wednesday, February 9, 2022 5:01 PM
> To: Kumaravel Thiagarajan - I21417 <Kumaravel.Thiagarajan@microchip.com>
> Cc: linux-serial@vger.kernel.org; Sundararaman Hariharaputran - I21286
> <Sundararaman.H@microchip.com>; Ronnie Kunin - C21729
> <Ronnie.Kunin@microchip.com>; Tharunkumar Pasumarthi - I67821
> <Tharunkumar.Pasumarthi@microchip.com>; Annirudh D - I64147
> <Annirudh.D@microchip.com>; Pragash Mangalapandian - I21326
> <Pragash.Mangalapandian@microchip.com>
> Subject: Re: Reg: Serial port driver for microchip's new PCIe UART device
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Wed, Feb 09, 2022 at 10:38:34AM +0000,
> Kumaravel.Thiagarajan@microchip.com wrote:
> > Dear Greg KH,
> >
> > I am Kumaravel Thiagarajan from Microchip India.
> >
> > We are working on a PCIe based multi-instance UART device.
> > Based on the Linux community feedback few months back, we had written
> it as a custom driver inside drivers/tty/serial/8250.
> > Now this custom driver is requiring a DWORD FIFO access for both Tx and
> Rx, and I am in the process of changing my driver code.
> 
> Why does the hardware not follow the normal standard here?
We are building a PCIe 8250 based UART.  We can absolutely support the normal 8250 framework and standard drivers. 
However, the challenges we see are the round-trip delays introduced by PCIe reads and writes having an impact on throughput, particularly if you are downstream of a PCIe tree with multiple hops.
The sizes of the payloads are limited to 32-bit by the processor PIO, however, even going from 8-bit payloads to 32-bit payloads improves throughput dramatically.

> 
> And are you sure it will still not fit into the 8250 format?
As mentioned our hardware can support this, however, we see that it is less efficient due to the requirement for single byte reads and writes.

> 
> > Can I model my custom driver on serial drivers present in drivers/tty/serial/
> directory?
> 
> You could, but it would be much smaller and easier to use the 8250
> framework given that you probably do have an 8250-like device, right?
Adding DWORD reads/writes to the hardware is a necessary enhancement for improved performance over PCIe.
But 8250 framework looks very closely tied with reading character by character from the FIFO and I was not able to find a place in that framework where I could hook my own DWORD based Rx and Tx logic.
Is there any DWORD based UART FIFO driver example with 8250 framework available?

> 
> > I am implementing my functions for uart_ops structure and the necessary
> ISR in a separate file mchp_pci1xxxx_uart.c inside the drivers/tty/serial/
> directory.
> >
> > I wish this custom UART driver to get through Linux community submission.
> > Do you see any risk in this approach? Do you have any suggestions?
> 
> Let's see the code before we can give you any firm answers.
> 
> good luck!
> 
> greg k-h

Re: Reg: Serial port driver for microchip's new PCIe UART device

[Greg KH]

On Thu, Feb 10, 2022 at 10:38:39AM +0000, Kumaravel.Thiagarajan@microchip.com wrote:
> > > We are working on a PCIe based multi-instance UART device.
> > > Based on the Linux community feedback few months back, we had written
> > it as a custom driver inside drivers/tty/serial/8250.
> > > Now this custom driver is requiring a DWORD FIFO access for both Tx and
> > Rx, and I am in the process of changing my driver code.
> > 
> > Why does the hardware not follow the normal standard here?
> We are building a PCIe 8250 based UART.  We can absolutely support the normal 8250 framework and standard drivers. 
> However, the challenges we see are the round-trip delays introduced by PCIe reads and writes having an impact on throughput, particularly if you are downstream of a PCIe tree with multiple hops.
> The sizes of the payloads are limited to 32-bit by the processor PIO, however, even going from 8-bit payloads to 32-bit payloads improves throughput dramatically.

What specific reads are you having problems with?

Why not just use DMA like other PCIe serial port cards do, which we have
supported for decades now.

> > And are you sure it will still not fit into the 8250 format?
> As mentioned our hardware can support this, however, we see that it is less efficient due to the requirement for single byte reads and writes.

Again, which reads/writes are taking a long time?  And again, why not
use DMA for the data instead?

> > > Can I model my custom driver on serial drivers present in drivers/tty/serial/
> > directory?
> > 
> > You could, but it would be much smaller and easier to use the 8250
> > framework given that you probably do have an 8250-like device, right?
> Adding DWORD reads/writes to the hardware is a necessary enhancement for improved performance over PCIe.
> But 8250 framework looks very closely tied with reading character by character from the FIFO and I was not able to find a place in that framework where I could hook my own DWORD based Rx and Tx logic.
> Is there any DWORD based UART FIFO driver example with 8250 framework available?

All of the code is there for you to read :)

thanks,

greg k-h

RE: Reg: Serial port driver for microchip's new PCIe UART device

[Kumaravel.Thiagarajan]

Dear Greg KH,

Please find my answers inline below.
Also, I am looking at the 8250 framework again whether I can drive the DWORD FIFOs by providing my own implementation for the handle_irq function pointer in uart_port structure which I am not sure at this point.
If I can do this, this will allow my driver to fit within the 8250 framework itself and simplify the entire work as mentioned by you previously.

Thank You.

Regards,
Kumar

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Friday, February 11, 2022 5:32 PM
> To: Kumaravel Thiagarajan - I21417 <Kumaravel.Thiagarajan@microchip.com>
> Cc: Richard Petrie - M18281 <Richard.Petrie@microchip.com>; linux-
> serial@vger.kernel.org; Sundararaman Hariharaputran - I21286
> <Sundararaman.H@microchip.com>; Ronnie Kunin - C21729
> <Ronnie.Kunin@microchip.com>; Tharunkumar Pasumarthi - I67821
> <Tharunkumar.Pasumarthi@microchip.com>; Annirudh D - I64147
> <Annirudh.D@microchip.com>; Pragash Mangalapandian - I21326
> <Pragash.Mangalapandian@microchip.com>
> Subject: Re: Reg: Serial port driver for microchip's new PCIe UART device
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Thu, Feb 10, 2022 at 10:38:39AM +0000,
> Kumaravel.Thiagarajan@microchip.com wrote:
> > > > We are working on a PCIe based multi-instance UART device.
> > > > Based on the Linux community feedback few months back, we had
> > > > written
> > > it as a custom driver inside drivers/tty/serial/8250.
> > > > Now this custom driver is requiring a DWORD FIFO access for both
> > > > Tx and
> > > Rx, and I am in the process of changing my driver code.
> > >
> > > Why does the hardware not follow the normal standard here?
> > We are building a PCIe 8250 based UART.  We can absolutely support the
> normal 8250 framework and standard drivers.
> > However, the challenges we see are the round-trip delays introduced by
> PCIe reads and writes having an impact on throughput, particularly if you are
> downstream of a PCIe tree with multiple hops.
> > The sizes of the payloads are limited to 32-bit by the processor PIO,
> however, even going from 8-bit payloads to 32-bit payloads improves
> throughput dramatically.
> 
> What specific reads are you having problems with?
Function serial8250_read_char reads character by character from the Rx FIFO (byte FIFO) based on the data ready bit in the LSR register.
Instead of byte FIFO, I am trying to use the DWORD interface, and a live byte count register available in our UART IP to increase the performance.
> 
> Why not just use DMA like other PCIe serial port cards do, which we have
> supported for decades now.
Our legacy hardware IP does not support DMA.
> 
> > > And are you sure it will still not fit into the 8250 format?
> > As mentioned our hardware can support this, however, we see that it is
> less efficient due to the requirement for single byte reads and writes.
> 
> Again, which reads/writes are taking a long time?  And again, why not use
> DMA for the data instead?
Our legacy hardware IP does not support DMA.
> 
> > > > Can I model my custom driver on serial drivers present in
> > > > drivers/tty/serial/
> > > directory?
> > >
> > > You could, but it would be much smaller and easier to use the 8250
> > > framework given that you probably do have an 8250-like device, right?
> > Adding DWORD reads/writes to the hardware is a necessary enhancement
> for improved performance over PCIe.
> > But 8250 framework looks very closely tied with reading character by
> character from the FIFO and I was not able to find a place in that framework
> where I could hook my own DWORD based Rx and Tx logic.
> > Is there any DWORD based UART FIFO driver example with 8250 framework
> available?
> 
> All of the code is there for you to read :)
Yes. I found two drivers inside 8250 framework supplying their own implementation for handle_irq which I will try to imitate.
> 
> thanks,
> 
> greg k-h