Hi Stephen, On 1 August 2014 00:06, Stephen Warren <swar...@wwwdotorg.org> wrote: > 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.
Please go ahead if you wish, but this is not a Linux issue. The aliases are used for U-Boot and have been for some time. > > >>>> 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. OK, is there a plan to put this in place? Who is working on it? > > >>> 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. I'm not sure what you are referring to here. Are you saying that U-Boot needs to support these things? I agree it would be great to add a generic clock/reset framework and have made considerable efforts in Tegra towards this myself. But we don't have it yet. > 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. The great is the enemy of the good. In this case, I think I might just leave the clock as a #define. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot