On 07/31/2014 04:10 PM, Simon Glass wrote:
Hi Stephen,

On 31 July 2014 21:16, Stephen Warren <swar...@wwwdotorg.org> wrote:
On 07/30/2014 03:49 AM, Simon Glass wrote:

Some Tegra device tree files do not include information about the serial
ports. Add this and also add information about the input clock speed.

The console alias needs to be set up to indicate which port is used for
the console.

Also add a binding file since this is missing.


diff --git a/arch/arm/dts/tegra114-dalmore.dts
b/arch/arm/dts/tegra114-dalmore.dts
index 435c01e..e2426ef 100644
--- a/arch/arm/dts/tegra114-dalmore.dts
+++ b/arch/arm/dts/tegra114-dalmore.dts
@@ -7,6 +7,7 @@
         compatible = "nvidia,dalmore", "nvidia,tegra114";

         aliases {
+               console = &uart_d;


I don't think that's a standard alias name. There was some recent discussion
in the devicetree mailing list re: using some property in /chosen for this
purpose instead. U-Boot and the kernel should use the same representation
here.

This is U-Boot's approach at present,

That's not the U-Boot approach on Tegra boards before this patch. I do not want Tegra U-Boot do adopt any more U-Boot-specific not-really-DT-but-pretending-to-be bindings.

> if we change it then we should
change it everywhere. I worry that 'chosen' is for Linux rather than
U-Boot and we might get very confused about what chosen is for?

That discussion should be had on the devicetree mailing list.

diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi


+       uart_a: serial@70006000 {
+               compatible = "nvidia,tegra20-uart";


This property needs to include both the specific HW (i.e. Tegra114) and any
HW it's compatible with (i.e. Tegra20).

So something like this?

compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";

Yes.

+               reg = <0x70006000 0x40>;
+               reg-shift = <2>;
+               clock-frequency = <408000000>;


This isn't a property that's defined by the Tegra serial binding. This
information should be obtained by looking up the relevant clock, and
querying its rate.

We can't do that in the ns16550 driver as yet since there is no
generic U-Boot clock infrastructure. I suspect that will come with
time.

The solution here is to put the clock infra-structure in place first. One thing I've learned from the kernel DT experience is that a lot of time would have been saved by putting the correct infra-structure in place first, then using it, rather than hacking around things the wrong way, then putting the infra-structure in place, then converting to it. That's a lot more work, and rather painful. Equally, if we don't just do the infra-structure right, there's really little guarantee that we'll ever convert to the correct approach. Just look at all the DT content in use in U-Boot that don't match the real DT bindings, even after it's been around years.

For reference, here's the DT node for this UART in the kernel DT, which
complies with the relevant binding document:

         uarta: serial@70006000 {
                 compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";

                 reg = <0x70006000 0x40>;
                 reg-shift = <2>;
                 interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
                 clocks = <&tegra_car TEGRA114_CLK_UARTA>;
                 resets = <&tegra_car 6>;
                 reset-names = "serial";
                 dmas = <&apbdma 8>, <&apbdma 8>;
                 dma-names = "rx", "tx";
                 status = "disabled";
         };

All the comment above apply to all the files in this patch.

My intent was to make this work with a more generic binding for now -
ns16550 is a pretty standard thing and I thought I could avoid making
the driver Tegra-specific. Then we could allow many SoCs to use it.
Why does Tegra have its own binding in the kernel for this standard
UART?

The HW is not a PC-style UART where all you care about is the 16550 registers, and clocks/resets/DMA/... can be ignored or deferred to firmware to set up before DT-driven SW runs.

As an aside, /almost/ all reviewed DT bindings use DT properties of the form:

clocks = <&provider parameters>;

rather than:

clock-rate = <number>;

So, that aspect of the Tegra UART binding isn't anything remotely unusual/non-standard.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to