From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Thu, 31 Jul 2014 17:06:15 -0600 Subject: [U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details In-Reply-To: References: <1406713793-12828-1-git-send-email-sjg@chromium.org> <1406713793-12828-15-git-send-email-sjg@chromium.org> <53DAA438.9080405@wwwdotorg.org> Message-ID: <53DACBE7.4040308@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 07/31/2014 04:10 PM, Simon Glass wrote: > Hi Stephen, > > On 31 July 2014 21:16, Stephen Warren 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 at 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 at 70006000 { >> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; >> >> reg = <0x70006000 0x40>; >> reg-shift = <2>; >> interrupts = ; >> 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 = ; So, that aspect of the Tegra UART binding isn't anything remotely unusual/non-standard.