[PATCH 1/6] Documentation: DT: Add bindings for FSL NS16550A The UART

[bhupesh.sharma at freescale.com]

Hi Mark,

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland at arm.com]
> Sent: Wednesday, August 20, 2014 5:02 PM
> To: Sharma Bhupesh-B45370
> Cc: devicetree-discuss at lists.ozlabs.org; Catalin Marinas; arnd at arndb.de;
> Will Deacon; Yoder Stuart-B08248; grant.likely at secretlab.ca; Basu Arnab-
> B45036; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 1/6] Documentation: DT: Add bindings for FSL NS16550A
> The UART
> 
> Hi Bhupesh,
> 
> [...]
> 
> > > > > -----Original Message-----
> > > > > From: Mark Rutland [mailto:mark.rutland at arm.com]
> > > > > Sent: Friday, August 15, 2014 4:16 PM
> > > > > To: Sharma Bhupesh-B45370
> > > > > Cc: devicetree-discuss at lists.ozlabs.org; Catalin Marinas;
> > > > > arnd at arndb.de; Will Deacon; Yoder Stuart-B08248;
> > > > > grant.likely at secretlab.ca; Basu Arnab- B45036;
> > > > > linux-arm-kernel at lists.infradead.org
> > > > > Subject: Re: [PATCH 1/6] Documentation: DT: Add bindings for FSL
> > > > > NS16550A UART
> > > > >
> > > > > On Fri, Aug 15, 2014 at 10:49:10AM +0100, Bhupesh Sharma wrote:
> > > > > > This patch addss the device-tree documentation for Freescale's
> > > > > > NS16550 UART (also called DUART).
> > > > > >
> > > > > > There is a specific errata fix required in FSL NS16550 UART
> > > > > > which ensures that an random interrupt storm is not observed
> > > > > > when a break is provided as an input to the UART.
> > > > >
> > > > > Should this not go in
> > > > > Documentation/devicetree/bindings/serial/of-serial.txt as with
> > > > > other NS16550A variations?
> > > > >
> > > > > The only code for handling this string seems to live in
> > > > > arch/powerpc/kernel/legacy_serial.c. Is there a patch factoring
> > > > > that out into common code? Or is the erratum not applicable to
> > > > > the revision used in your ARMv8 SoC?
> > > > >
> > > >
> > > > The FSL specific 8250 (or NS16550A0 driver) at path
> > > > drivers/tty/serial/8250/8250_fsl.c
> > > > uses this string and provides the code to handle the erratum (see
> > > > [1])
> > >
> > > While the workaround itself is in that file, as far as I can see the
> > > detection and setup is not. Even in next-20140815 it seems that only
> > > happens in arch/powerpc/kernel/legacy_serial.c (see [2]):
> > >
> > > [mark at leverpostej:~/src/linux]% git grep fsl8250_handle_irq next-
> 20140815
> > > next-20140815:arch/powerpc/kernel/legacy_serial.c:              port-
> > > >handle_irq = fsl8250_handle_irq;
> > > next-20140815:drivers/tty/serial/8250/8250_fsl.c:int
> > > fsl8250_handle_irq(struct uart_port *port) next-
> > > 20140815:include/linux/serial_8250.h:extern int
> > > fsl8250_handle_irq(struct uart_port *port);
> > >
> > > I cannot see any use of the string outside of arch/powerpc:
> > >
> > > [mark at leverpostej:~/src/linux]% git grep fsl,ns16550 next-20140815 |
> > > wc -l
> > > 102
> > > [mark at leverpostej:~/src/linux]% git grep fsl,ns16550 next-20140815
> > > -- arch/powerpc | wc -l
> > > 102
> > >
> > > Am I missing something? Perhaps some macro is getting in the way of
> > > a simple grep.
> > >
> > > > The NS16550 UART present on our SoC and the corresponding
> > > > simulator model has this erratum and hence the specific way in
> > > > which IRQ are handled (fsl8250_handle_irq) by the driver (in [1])
> > > > is applicable here
> > > as well.
> > >
> > > Assuming I've not missed something, do you need to factor out the
> > > detection and setup? Do things just happen to work on the simulator
> > > without the workaround?
> >
> > You are right. The simulator platform doesn't take into account the
> > erratum, however we need to take is into account for the emulator and
> > silicon platforms.
> >
> > I am currently working to fork this out into a common code base so
> > that both PPC and ARM platforms can use the same and a patch for the
> > same is in my to-do list.
> >
> > So, I sent out the DT documentation patch early (as it is equally
> > applicable to  both PPC and ARM platforms) and the patch that makes
> > this code leg usable for both ARM and PPC platform will follow soon.
> 
> Ok.
> 
> My only issue with the DT binding is the placement; I think that should
> live with the other NS16550A variations.

Sure. I will look at how I can change the DT binding placement and then
address it in my v2.

Regards,
Bhupesh

> Cheers,
> Mark.