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,

[...]

quoted

quoted

quoted

quoted

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

quoted

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

quoted

quoted

next-20140815:arch/powerpc/kernel/legacy_serial.c:              port-

quoted

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.

quoted

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.