Hi André,
On Tue, 12 May 2020 at 08:27, André Przywara andre.przywara@arm.com wrote:
On 28/04/2020 18:57, Simon Glass wrote:
Hi,
sorry for the delay, found this, slightly mouldy already, in my draft folder.
First, thanks for the review! I saw the Tom merged this already, but wanted to come back to the DT hacks:
Hi Andre,
On Mon, 27 Apr 2020 at 12:18, Andre Przywara andre.przywara@arm.com wrote:
Even though the PL011 UART driver claims to be DM compliant, it does not really a good job with parsing DT nodes. U-Boot seems to adhere to a non-standard binding, either requiring to have a "skip-init" property in the node, or to have an extra "clock" property holding the base *frequency* value for the baud rate generator. DTs in the U-Boot tree seem to have been hacked to match this requirement.
One problem is that we want a 'debug UART' to work before the clock driver is running, so we want to do the *minimum possible* amount of init to get the UART running. So we don't want to start up driver model, clock drivers, etc.
I understand this very well - having an UART up and running as early as possible is crucial for debugging.
I think we should have useful helpers like the 'clock' property to avoid lots of parsing very early in U-Boot. Of course such things are hard for kernel people to understand / agree to but that doesn't make them wrong.
I agree, but I don't think we should mess around with the DT for this purpose. This is basically a U-Boot requirement or debug feature, not a machine property. And deviating from the official DT binding is not a good idea.
I think for those U-Boot specific debug features we can just have CONFIG options - for instance we already have CONFIG_PL011_CLOCK. Also I strongly believe that skip-init does not belong into the DT. It's a *U-Boot* decision to not *re*-init the UART, not a machine property. There are PL011 compatible UARTs which should *not* be initialised (SBSA-UART), but both TX1 and RPi don't have those, but instead real PL011s. So if we desperately wanted this in the DT, we could actually use compatible = "arm,sbsa-uart", then we don't need any clock at all.
Yes of course these are U-Boot decisions in some sense. But they are also hardware-related. There is nothing wrong with having a fixed clock as a default, for simple software to use.
We have a persistent problem here because of this 'linux' idea that we cannot have config in the DT. It is generally the only thing available to U-Boot. It is certainly the only thing available for runtime config.
Why not put a 'u-boot' prefix on it and be done?
If we could just get over this hangup, it would be so great for U-Boot :-) It doesn't have the ability to rely on user space for policy.
But I was more thinking about turning skip-init into a config symbol and defining this for TX1 and RPi. We do already something similar for the RPi4 in Trusted Firmware [1]. This would allow us to remove the skip-init property from the u-boot.dtsi, and would help with booting with the DT from the SD card instead (for which the GPU firmware puts the pointer to into the beginning of memory [2]).
You mean we have to build U-Boot differently depending on what it is booting from? I wonder if we could pass this information through to U-Boot instead.
I have a patch for that already, will send it soonish.
Regards, Simon