On Wed, Nov 08, 2023 at 07:25:22AM -0800, Heinrich Schuchardt wrote:
On 11/8/23 06:37, Tom Rini wrote:
On Wed, Nov 08, 2023 at 06:23:37AM -0800, Heinrich Schuchardt wrote:
On 11/7/23 16:34, Tom Rini wrote:
On Wed, Nov 08, 2023 at 12:29:03AM +0000, Conor Dooley wrote:
On Tue, Nov 07, 2023 at 06:23:05PM -0500, Tom Rini wrote:
[snip]
Thanks. Setting aside Simon's follow-up, this is what I was looking for. We might have to wait for Heinrich to return from the conference to have time to look at how to utilize the above and see what we can do from there.
I did read that, but I don't think most of it is relevant to the binding itself. His five things were: | - U-Boot models hardware (and other things) as devices in driver model [1]
This I think should be satisfied. The Zkr CSR is a property of the CPU, and shouldn't have its own DT node IMO. Is it problematic for U-Boot to populate multiple devices for its driver model based on one DT node?
Devices in U-Boot are bound on the basis of a compatible string. All RISC-V CPU nodes have a compatible string 'riscv' but that does not provide any information about the existence of the Zkr extension. That information is in the 'riscv,isa-extensions' property of the cpu nodes (see Documentation/devicetree/bindings/riscv/cpus.yaml).
I know in Linux that I can create devices using something like platform_device_register(), does U-Boot have a similar facility?
This is what the U_BOOT_DRVINFO() macro in my driver does and which Simon discourages.
My current thoughts are that in this case we could use U_BOOT_DRVINFO() like today and then have riscv_zkr_probe() be what checks the riscv,isa-extensions property for an appropriate match? This would mean we don't need any new nodes/compatibles/etc, and possibly not need any bootph- properties added either? I assume we don't need the RNG so early as for that to be an issue.
The presence of the Zkr extension in the device-tree 'riscv,isa-extensions' property does not indicate if the machine mode firmware has enabled access in supervisor mode via the mseccfg.sseed flag. This is why my driver tries to read the seed register and checks if an exception occurs.
This I think is a question for Cody or someone else in the RISC-V community? If just checking the property isn't sufficient, what is? Or, what's the best / most reliable way? I don't know and I'll let the RISC-V community sort that out instead of making incorrect assumptions myself.
Additionally checking the device-tree would increase code size. Is it really needed?
I mean, it depends? My biggest issue right now is that in-tree I don't see any RISC-V targets that select any of the RISCV_ISA options so I can't evaluate what platforms grow by how much where. We shouldn't be talking about kilobytes of change, so no, I don't think somehow checking the device tree for this is out of bounds. But it comes back to what I just asked above, first. We need the correct and expected way to check for this feature to be known, then we decide how to handle that.
We may have multiple RNG drivers enabled on the same device, e.g. TPM, Zkr, virtio-rng. In several code locations we try to use the first RNG device (not the first successfully probed RNG device). This is why
[PATCH 1/1] rng: detect RISC-V Zkr RNG device in bind method https://lore.kernel.org/u-boot/20231104065107.23623-1-heinrich.schuchardt@ca...
moves the detection from probe() to bind(). The patch further corrects the return code if the RNG device is not available.
Sounds like we have some other general issues to sort out too then. Filing an issue on https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/ would be good so it doesn't get forgotten.