[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Stephen Warren <swarren@wwwdotorg.org>]

On 08/04/2014 04:43 AM, Simon Glass wrote:
> On 1 August 2014 15:50, Stephen Warren <swarren@wwwdotorg.org> 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:-)