[U-Boot] [PATCH v3 06/11] arm: rpi: Enable device tree control for Rasberry Pi

[Simon Glass <sjg@chromium.org>]

Hi Stephen,

On 14 August 2015 at 21:32, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/14/2015 01:20 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On 10 August 2015 at 21:47, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 08/07/2015 07:42 AM, Simon Glass wrote:
>>>> Enable device tree control so that we can use driver model fully and avoid
>>>> using platform data.
>>>
>>> I'm still not convinced about this change.
>>>
>>> Re: the commit message about: What about the driver model is not being
>>> fully used without DT?
>>
>> Device tree control?
>
> I am not convince about that argument. It seems rather self-fulfilling,
> and irrelevant to me.
>
> The fact that a feature exists shouldn't necessarily mean that it
> absolutely must be used in all cases. There needs to be some benefit
> from using the feature. What stability, performance, ... benefit does DT
> give to a maintainer or user?
>
> On a system that sources device information from ACPI, must DT still be
> used because DM has that feature and without using it, DM isn't being
> fully used?
>
> I would strongly hope that DM is not tied to any particular source of
> device information. A device model should be generic. The actions of
> enumerating what devices exist (via C structures, DT, ACPI, ...) should
> be completely independent from the process that then manages all of
> those devices once they're enumerating/instantiated.

So far driver model uses device tree. Devices come into existence
because of nodes in the device tree. We are trying to avoid platform
data (C structures) and encourage people to standardize on device
tree. The ACPI support that is being added is just tables for Linux to
use - there is no plan at present for U-Boot to make use of it for its
own configuration.

I don't see the benefit of additional configuration mechanisms. By
using a single one we reduce confusion across boards and
architectures.

>
>>> Overall: What advantage does using DT have to either a developer or an
>>> end-user?
>>>
>>> I don't believe this patch fixes and bugs or enables any new features
>>> for an end-user.
>>>
>>> From the maintainer perspective: It seems to me that it's far simpler to
>>> have a tiny struct for each device in the C code than to pull in a whole
>>> slew of DT parsing cruft just to work out the same struct at run-time.
>>> As such, this patch can only make it harder to maintain the code since
>>> there's more of it, and it's more complex.
>>>
>>> I just don't see the advantage of switching to DT for U-Boot control.
>>
>> It allows us to share configuration with the kernel (same device tree
>> file). This should be more familiar to people coming from there than
>> our own configuration system. It's nice to have all the configuration
>> in one place. We can then avoid using platform data in U-Boot unless
>> it is necessary.
>
> But at the cost of extra complexity in the U-Boot code that I don't
> think is warranted. Equally, U-Boot's DT support is built on some
> assumptions about DT structure and parsing rules that are inaccurate
> (e.g. not honoring #address-cells, not parsing the DT in a hierarchical
> manner thus not ensuring correct driver "probe" ordering, missing
> features such as clock frameworks with pushback on supporting the
> standard clock bindings rather than implementing U-Boot-specific
> properties, etc.). It's not quite DT, but almost. It's quite unlikely
> that any Linux DT will "just work". Once it doesn't always just work,
> the advantage of similarity with kernel DTs is lost. As someone who's
> ported U-Boot to various Tegra and RPi SoCs/boards, I honestly don't
> think that using DT rather the C structures would have saved me any
> time, and likely has caused me to spend more time debugging and fixing
> DT issues in U-Boot.

I think this is taking a rather negative view of the development
process. U-Boot had device tree on Tegra before it existed in Linux,
and before we had driver model.

#address-cells is supposed to describe the size of addresses on the
platform. My push back here was that we added an extremely inefficient
function to read the address (scan entire device tree for parent node,
etc.) when we need that function to work in SPL quickly and with
minimal code. Your new approach looks good to me so I hope that can be
put to bed.

Hierarchical parsing is how driver model works, and as things convert
over we drop the fdtdec parsing.

A basic clock framework was merged recently - perhaps you could take a
look at supporting this on Tegra and seeing how it helps with the
issues you raise? Also you will have seen Masahiro's series to support
pinctrl. But nothing gets done unless people work on it. Sometimes
infrastructure is harder because the effort to get agreement and
convert over existing users becomes substantial

I think we have to look to board /SoC maintainers to keep the device
tree files in sync. It would be great if we could get to the point
where it could be scripted and then perhaps the fdt maintainer
(currently me) could do it each cycle.

>
>> I really don't like the idea of filling up the code with platform data
>> when that approach has already been rejected by Linux.
>
> The Linux situation is entirely different from U-Boot.
>
> Linux distros want to distribute a single generic Linux kernel binary
> (and indeed generic install media, etc.) that runs on arbitrary systems
> without having to worry about system-specific details. Ideally, the
> distro can ship a single OS image which will work on arbitrary systems,
> provided the system vendor ships the DT as part of the firmware and
> provides it to the kernel. Of course, that hasn't actually happened yet
> since the DTs are still in the kernel source tree and DT ABI isn't
> anywhere near universal.
>
> A bootloader or firmware is by definition system-specific. There's no
> concept of a single image working across *arbitrary* systems. (Of
> course, some U-Boot builds run on a small number of boards via runtime
> detection, but by no means is any U-Boot binary entirely generic).
> There's no need for it to be generic, since system vendors or
> enterprising users build and install the firmware for a specific known
> board/system.
>
> As such, any arguments about Linux having chosen to use DT are likely
> irrelevant to whether a firmware or bootloader should use it.

I have found it *much* easier to support platform features across
multiple boards using device tree. For example, I was recently
updating the TPM support to move it to driver model. I was able to add
TPM support to Nyan and several exynos and x86 boards just by copying
in the tpm node and enabling the driver.

Similarly a downstream board can start by making device tree changes
to match their board, and expect to get basic functionality without a
lot of effort.

Many of the boards in U-Boot are very similar to each other and a
device tree is enough to capture the differences. This gives people
the option to go this way. It helps reduce the proliferation of code
paths and builds which is often seen in firmware. Even if the build
produces separate binaries, perhaps the only difference might be the
device tree.

I have heard arguments that firmware is always hard-coded and run-time
configuration is pointless. But to a point, injecting the data into
generic code is much easier to manage then #ifdefs and conditional
build-time configuration code.

I really like where U-Boot has got to with device tree. It fits really
nicely with driver model and things are pretty clean and ordered. We
are in a much better place now I think.

Regards,
Simon