Hi I am late to the party but
[...]
I can't help to think that you like the FDT as a well understood and flexible general purpose data structure. And it can indeed be used as a configuration file, especially since you have the parser in your code already - the FIT image is a good example. But this is distinct from the Devicetree as a hardware description.
Indeed, although I don't see such a bright line between the hardware/software concepts. In any case, the RNG is a hardware feature, surely?
Would it help to separate the two use cases: to go with one DTB that is strictly for hardware configuration, as described by the DT bindings in the Linux kernel tree, and a *separate* DT blob that carries configuration information, U-Boot specific data (like memory layout, SRAM usage, device priorities, packaging information)? TF-A chose this approach: there is hw-config, which is the hardware DTB, and there is fw-config, which is a FDT blob as well, but describes the firmware layout and other configuration information.
Or we could just not do that and have a single DT :-)
I wondered where that idea came from and it has been mentioned before.
Yes, that was me. I still think this would solve a *ton* of problems and I am willing to explore that. The only thing I haven't thought through is DTs and limited SRAM in the SPL, but I don't think this should be a showstopper.
Thanks /Ilias
TF-A should be a C library called by bootloaders, BTW.
I haven't replied to everything above...I believe I have had this same discussion about a dozen times over the last 6 years. I did try to send a patch to state my POV at some point, but I think it got NAKed :-)
Regards, Simon
Cheers, Andre
> > But as Heinrich also said: those instructions are not > > peripherals, they are part of an instruction set extensions, > > the same story as with x86's RDRAND instruction. We don't have > > those in ACPI or so as well, because CPUID has you covered. > > The same on ARM, ID_AA64ISAR0_EL1 is readable on every chip > > (outside of EL0), and tells you whether you have the RNDR > > register or not. IIUC RISC-V is slightly different here, since > > not all ISA extensions are covered by CSRs, hence some of them > > indeed listed in the DT. > > > > So a proper solution(TM) would be to split this up in > > architectural *instructions* and proper TRNG *devices*, maybe > > wrapping this up in some function that tests both. This is > > roughly what the kernel does, somewhat abstracted by the > > concept of "entropy sources", which could be TRNG devices, CPU > > instructions, interrupt jitter or even "instruction execution > > jitter"[1], with the latter two definitely not being devices > > really at all. > > > > But I don't know if U-Boot wants to go through the hassle of > > this whole framework, as we tend to implement things much > > easier. But a simple get_cpu_random() function, implemented > > per architecture, and with some kind of success flag, should > > be easy enough to do. Then either the users (UEFI?) explicitly > > call this before trying UCLASS_RNG, or we wrap this for every > > RNG user. > > > > Cheers, > > Andre > > > > > > > > > VExpress64# rng 1 > > > > > > > 00000000: f3 88 b6 d4 24 da 49 ca 49 f7 9e 66 5f 12 > > > > > > > 07 b2 ....$.I.I..f_... > > > > > > > > > > > > > > > > > > Essentially in any case were you have multiple drivers > > > > > > for the same device using uclass_get_device(, 0, ) and > > > > > > uclass_find_first_device() will only give you the > > > > > > first bound device and not the first successfully > > > > > > probed device. Furthermore neither of this functions > > > > > > causes probing. This is not restricted to the RNG > > > > > > drivers but could also happen with multiple TPM > > > > > > drivers or multiple watchdogs. > > > > > > > > > > > > This patch is related to the problem: > > > > > > > > > > > > [PATCH v1] rng: add dm_rng_read_default() helper > > > > > > https://lore.kernel.org/u-boot/4e28a388-f5b1-4cf7-b0e3-b12a876d0567@gmx.de/T... > > > > > > > > > > > > We have weak function platform_get_rng_device() which > > > > > > should be moved to drivers/rng/rng-uclass.c. > > > > > > > > > > > > We could add a function to drivers/core/uclass.c to > > > > > > retrieve the first successfully probed device. Another > > > > > > approach would be to implement > > > > > > uclass_driver.post_probe() in the RNG uclass to take > > > > > > note of the first successfully probed device. > > > > > > > > > > > > @Simon: > > > > > > What would make most sense from a DM design > > > > > > standpoint? > > > > > > > > > > I am sure I provided feedback on this at the time, but I > > > > > don't remember. OK I just found it here [1]. So the > > > > > problem is entirely because my feedback was not > > > > > addressed. Please just address it and avoid this sort of > > > > > mess. > > > > > > > > Yeah, Tom just merged it, but that's not Heinrich's fault > > > > ;-) > > > > > So arm_rndr should have a devicetree compatible string > > > > > and be bound like anything else. If for some reason the > > > > > device doesn't exist in the hardware, it can return > > > > > -ENODEV from its bind() method. > > > > > > > > > > If you want to control which RNG device is used for > > > > > booting, you could add a property to /bootstd with a > > > > > phandle to the device. We are trying to provide a > > > > > standard approach to booting in U-Boot, used by all > > > > > methods. Doing one-off things for particular cases is > > > > > best avoided. > > > > > > > > Picking the first usable device doesn't sound much like a > > > > one-off to me. After all the caller (be it UEFI or the rng > > > > command) later detect that this is not usable. So there > > > > might be some merit to cover this more automatically, > > > > either in the caller, or by providing a suitable wrapper > > > > function? > > > > > > Or just follow the existing mechanisms which have been in > > > U-Boot for years. Please...! > > > > > > [..] > > > > > > > > > Regards, > > > Simon > > > > > > > > > [1] > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230830113230.3925868-1-an... > > > > > > > > >
Regards, SImon