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/[email protected]/ > > 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. -- Tom
signature.asc
Description: PGP signature
