From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Mon, 04 Aug 2014 11:47:56 -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> <53DACBE7.4040308@wwwdotorg.org> <53DC0BA9.8050301@wwwdotorg.org> Message-ID: <53DFC74C.2010807@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 08/04/2014 04:43 AM, Simon Glass wrote: > On 1 August 2014 15:50, Stephen Warren wrote: ... >> DT schemas/bindings MUST be identical between U-Boot, Linux, FreeBSD, >> Barebox, ... (all of which use DT). As such, all the DT bindings MUST be >> discussed on the devicetree mailing list. >> >> Since you're the author of the patch, it's your responsibility to have that >> discussion. > > Are you referring to the linux,stdout-path discussion, or something > more DT-generic?I suppose we could have a 'u-boot,console' for our > part. But in any case you are talking about code and a convention that > is already in mainline U-Boot. I'm saying that any and all additions or changes to DT schemas/bindings must be discussed on the devicetree mailing list, not made/reviewed in isolation on only the U-Boot mailing list. > While I accept that we might change to > something DT-generic if Linux points the way to something better, I > don't want to stop using it just because Linux hasn't decided yet. The > early console stuff and early debug UART stuff in Linux is not yet a > shining example of perfection. I strongly believe that if U-Boot continues to use DT, the current DT usage in U-Boot needs to be actively moved in line with the bindings that the Linux kernel, Barebox, FreeBSD, ... use. I'd prefer this to happen even before U-Boot starts making additional use of DT, so the conversion doesn't get forgotten. However, I suppose it's a bit draconian to prevent further usage until the existing usage is cleaned up, except where new usage introduces additional dependencies on any current usage that's inconsistent with the standard bindings. > That said, it's a good time to adopt 'u-boot,console' if that's what we need? It's certainly a good time to start that discussion on the devicetree mailing list, and get such a new property reviewed/ack'd there. >>>>>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi >>>>>>> + uart_a: serial at 70006000 { >>>>>>> + compatible = "nvidia,tegra20-uart"; >>>>>>> + 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? >> >> >> I don't know whether anyone is or not. However, the fact that nobody is >> working on a clock driver is no excuse for using the incorrect DT bindings >> in order to hack around its lack of existence. > > Actually the bindings are correct, you are referring to a single new > addition which is the input clock speed of the UART. That property (or the definition-of/schema-for it) is part of the bindings. Unless that addition is reviewed in the appropriate place, I think it's legitimate to refer the such a change to the binding as incorrect. >> If you want to move the serial port information into DT, and part of doing >> so requires extracting clock-related information from DT, and that in turn >> requires a clock driver to do so, then yes, U-Boot needs to have that clock >> driver implemented before you can move the serial port information into DT. > > Again, your objection is purely about the clock-frequency property I > think. The binding is otherwise correct, right? I think that was the primary issue, yes. If the U-Boot usage is consistent with Documentation/devicetree/bindings/serial/ of-serial.txt and/or nvidia,tegra20-hsuart.txt (as currently hosted in the Linux kernel source tree), then it's fine. ... >> To be honest, I'm not sure that using DT is going to buy U-Boot much, or >> even the kernel in retrospect. > > It's not that bad :-) My experience working on DT support in the kernel says otherwise, but I'm happy to disagree on that point for now:-)