Hi Tom, On Mon, 6 Nov 2023 at 09:46, Tom Rini <tr...@konsulko.com> wrote: > > On Sat, Nov 04, 2023 at 05:12:12PM +0000, Andre Przywara wrote: > > On Fri, 3 Nov 2023 13:38:58 -0600 > > Simon Glass <s...@chromium.org> wrote: > > > > Hi Simon, > > > > > Hi Heinrich, > > > > > > On Wed, 1 Nov 2023 at 14:20, Heinrich Schuchardt > > > <heinrich.schucha...@canonical.com> wrote: > > > > > > > > On 11/1/23 19:05, Andre Przywara wrote: > > > > > On Tue, 31 Oct 2023 14:55:50 +0200 > > > > > Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote: > > > > > > > > > > Hi Heinrich, > > > > > > > > > >> The Zkr ISA extension (ratified Nov 2021) introduced the seed CSR. It > > > > >> provides an interface to a physical entropy source. > > > > >> > > > > >> A RNG driver based on the seed CSR is provided. It depends on > > > > >> mseccfg.sseed being set in the SBI firmware. > > > > > > > > > > As you might have seen, I added a similar driver for the respective > > > > > Arm > > > > > functionality: > > > > > https://lore.kernel.org/u-boot/20230830113230.3925868-1-andre.przyw...@arm.com/ > > > > > > > > > > And I see that you seem to use the same mechanism to probe and init > > > > > the > > > > > driver: U_BOOT_DRVINFO and fail in probe() if the feature is not > > > > > implemented. > > > > > One downside of this approach is that the driver is always loaded (and > > > > > visible in the DM tree), even with the feature not being available. > > > > > That doesn't seem too much of a problem on the first glance, but it > > > > > occupies a device number, and any subsequent other DM_RNG devices > > > > > (like virtio-rng) typically get higher device numbers. So without > > > > > the feature, but with virtio-rng, I get: > > > > > VExpress64# rng 0 > > > > > No RNG device > > > > > > Why do we get this? If the device is not there, the bind() function > > > can return -ENODEV > > > > > > I see this in U-Boot: > > > > > > U_BOOT_DRVINFO(cpu_arm_rndr) = { > > > > > > We should not use this. > > > > Agreed. > > > > > Use the devicetree. > > > > No, this is definitely not something for the DT, at least not on ARM. > > It's perfectly discoverable via the architected CPU ID registers. > > Similar to PCI and USB devices, which we don't probe via the DT as well. > > > > It's arguably not proper "driver" material per se, as I've argued before, > > but > > it's the simplest solution and fits in nicely otherwise. > > > > I was wondering if it might be something for UCLASS_CPU, something like > > a "CPU feature bus": to let devices register on one on the many CPU > > features (instead of compatible strings), then only bind() those > > drivers it the respective bit is set. > > > > Does that make sense? Would that be doable without boiling the ocean? > > As I don't know if we see many users apart from this. > > I think we have a similar problem with how we're doing with DM_TIMER and > armv7-a/armv7-m/armv8[1]. We shouldn't need the drivers in drivers/timer/ > to cover platforms where SYS_ARCH_TIMER is (or should be!) enabled. But > in turn, the code under arch/arm/cpu/*/*timer.c doesn't implement the > uclass side of things, only the regular API. This is because there's > nothing to probe even because we don't support the kind of multi-arch > binary where it'd be possible to not have the feature.
The difference here is that there is only one timer device, at least in hardware I have used. I am leaning towards NAKing this and any future use of U_BOOT_DRVINFO(), in favour of a proper DT binding. It's time we stopped making this so hard. I'll reply on the other thread. Regards, Simon > > -- > Tom > > [1]: We do have the problem of armv7-r not having this feature so things > like say TI K3 platforms need the platform driver on the Cortex-R host. > A similar issue is the pre-ARMv7 i.MX platforms.