Hi Tom,
On Mon, 6 Nov 2023 at 09:46, Tom Rini trini@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 sjg@chromium.org wrote:
Hi Simon,
Hi Heinrich,
On Wed, 1 Nov 2023 at 14:20, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/1/23 19:05, Andre Przywara wrote:
On Tue, 31 Oct 2023 14:55:50 +0200 Heinrich Schuchardt heinrich.schuchardt@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.przywara@arm.c...
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.