Hi André, On Tue, 12 May 2020 at 08:27, André Przywara <andre.przyw...@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.przyw...@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
