Thread (48 messages) 48 messages, 10 authors, 2011-06-24

Re: [PATCH 1/3] serial/imx: add device tree support

From: Grant Likely <hidden>
Date: 2011-06-19 18:44:56
Also in: linux-arm-kernel, lkml, netdev

On Sun, Jun 19, 2011 at 9:15 AM, Rob Herring [off-list ref] wrote:
On 06/19/2011 10:05 AM, Grant Likely wrote:
quoted
On Sun, Jun 19, 2011 at 1:30 AM, Shawn Guo [off-list ref] wrote:
quoted
On Sat, Jun 18, 2011 at 10:19:34AM -0600, Grant Likely wrote:
quoted
On Sat, Jun 18, 2011 at 11:19:12PM +0800, Shawn Guo wrote:
quoted
It adds device tree data parsing support for imx tty/serial driver.

Signed-off-by: Jeremy Kerr <redacted>
Signed-off-by: Jason Liu <redacted>
Signed-off-by: Shawn Guo <redacted>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../bindings/tty/serial/fsl-imx-uart.txt           |   21 +++++
 drivers/tty/serial/imx.c                           |   81 +++++++++++++++++---
 2 files changed, 92 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
new file mode 100644
index 0000000..7648e17
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
@@ -0,0 +1,21 @@
+* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART)
+
+Required properties:
+- compatible : should be "fsl,<soc>-uart", "fsl,imx-uart"
I'd make this "fsl,<soc>-uart", "fsl,imx51-uart"

It's better to anchor these things on real silicon, or a real ip block
specification rather than something pseudo-generic.  Subsequent chips,
like the imx53, should simply claim compatibility with the older
fsl,imx51-uart.
It is a real IP block on all imx silicons (except imx23 and imx28
which are known as former stmp).
quoted
(in essence, "fsl,imx51-uart" becomes the generic string without the
downside of having no obvious recourse when new silicon shows up that
is an imx part, but isn't compatible with the imx51 uart.
In this case, should imx1 the ancestor of imx family than imx51
becomes part of that generic string?  Claiming uart of imx1, imx21
and imx31 (senior than imx51) compatible with the imx51 uart seems
odd to me.

That said, IMO, "fsl,imx-uart" stands a real IP block specification
here and can be a perfect generic compatibility string to tell the
recourse of any imx silicon using this IP.
Yes, but which /version/ of the IP block?  Hardware designers are
notorious for changing hardware designs for newer silicon, sometimes
to add features, sometimes to fix bugs.  While I understand the
temptation to boil a compatible value down to a nice clean generic
string, doing so only works in a perfect world.  In the real world,
you still need to have some information about the specific
implementation.  I prefer this specifying it to the SoC name, but I've
been known to be convinced that specifying it to the ip-block name &
version in certain circumstances, like for IP blocks in an FPGA, or
some of the Freescale powerpc pXXXX SoCs which actually had an IP
block swapped out midway through the life of the chip.
There are definitely uart changes along the way with each generation.
quoted
Besides, encoding an soc or ip block version into the 'generic'
compatible values is not just good practice, it has *zero downside*.
That's the beauty of the compatible property semantics.  Any node can
claim compatibility with an existing device.  If no existing device
fits correctly, then the node simple does not claim compatibility.
Drivers can bind to any number of compatible strings, so it would be
just fine for the of_match_table list to include both "fsl,imx-21" and
"fsl,imx-51" (assuming that is the appropriate solution in this case).
Don't you need uart or serial in here somewhere.
you are of course correct.  The examples should be "fsl,imx21-uart" &
"fsl,imx51-uart".  I was just writing too quickly.

g.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help