Hi Bin, On Wed, 2018-12-05 at 17:59 +0800, Bin Meng wrote: > Hi Lukas, > > On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas > <[email protected]> wrote: > > > > Hi Bin, > > > > On Wed, 2018-11-14 at 09:48 +0800, Bin Meng wrote: > > > Hi Lukas, > > > > > > On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas > > > <[email protected]> wrote: > > > > > > > > Hi Bin, > > > > > > > > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote: > > > > > This adds U-Boot syscon driver for RISC-V Core Local > > > > > Interruptor > > > > > (CLINT). The CLINT block holds memory-mapped control and > > > > > status > > > > > registers associated with software and timer interrupts. > > > > > > > > > > 3 APIs are provided for U-Boot to implement Supervisor Binary > > > > > Interface (SBI) as defined by the RISC-V privileged > > > > > architecture > > > > > spec v1.10. > > > > > > > > > > Signed-off-by: Bin Meng <[email protected]> > > > > > --- > > > > > > > > Would it make sense to also abstract the functions provided by > > > > the > > > > CLINT more? The reason why I am asking is because the CLINT is > > > > (unfortunately) not specified as part of RISC-V. It is > > > > developing > > > > into > > > > a de facto standard since other platforms are following > > > > SiFive's > > > > implementation, but there is nothing that would prevent them > > > > from > > > > implementing something else. > > > > > > > > > > I think your observation is correct about CLINT. Rick, does > > > Andes's > > > RISC-V processor also follow SiFive's CLINT model? > > > > > > > Two immediate examples I can think of would be mtime and the > > > > IPI > > > > implementation. Many SoC vendors will likely already have a > > > > suitable > > > > timer IP block for mtime. I can imagine that they would choose > > > > to > > > > re- > > > > use their memory map instead of following that of the CLINT. > > > > For the IPI implementation there is already an alternative, the > > > > SBI. If > > > > U-Boot should be able to run in supervisor mode, it would be > > > > helpful to > > > > support both the SBI and the CLINT interface. > > > > > > > > > > I am not sure I followed you here. This driver provides 3 APIs: > > > riscv_send_ipi() is for IPI, and the other 2 are for > > > mtime/mtimecmp. > > > > > > > It does, but I am not sure how easy it is to support different > > devices. > > Supporting the SBI is not going to be an issue, more problematic > > would > > be if we have two different devices and device tree nodes to > > implement > > the functionality of the APIs. Now we have to bind this driver to > > two > > devices and call the APIs from the correct instantiation, which > > would > > not work. > > > > Thinking about this a little more, I think the only issue is that > > we > > have both IPI- and mtime-related APIs in one driver. How about > > something like this? Instead of binding this driver to > > riscv,clint0, we > > add a new misc driver for the clint0. The only thing the driver > > does, > > is to bind the syscon driver and the timer driver (see for example > > tegra-car.c). Other architectures with separate device tree nodes > > for > > the API functionality won't need the misc driver and can just bind > > the > > devices to the syscon driver and a timer driver. > > > > Sorry it took a long time before replying this. I did have a look at > the tegra-car.c driver, and also tried various experiments. As Rick > pointed out we have to handle mixed IP blocks like Andes chip for > mtimer and IPIs. So it looks we need be able to flexibly handle the > following cases (sigh): > > - SiFive's clint model which implements mtimer and IPI > - mtimer following SiFive's clint model, but IPI does not (Andes > chip) > - IPI following SiFive's clint model, but mtimer follows > (hypothetical > model which does not exist today) > - completely different mtimer and IPI models from SiFive's clint > model >
This really is not ideal. I don't think there is a nice way of handling this since there is no way to detect which version we have. A cleaner solution would be to get everyone to use separate compatible strings so that we can load the correct driver or use the correct register offsets. I can't actually find a device node for the CLINT in the device tree of Andes' chip. If they already use a different compatible string then this should work. > I don't quite follow your idea of implementing clint as a misc > driver, then binding syscon and timer devices in the misc driver. But > I think we can only have the misc driver, and use misc uclass's > ioctl() to implement the SBI required calls (set_timecmp, get_time, > send_ipi), and we can have another ioctl code to report its > capability > to support mtimer and/or IPI functionality. This can support > flexibility. However I believe this may affect performance if we go > through many uclass function calls when doing SBI. > > The solution does not look clean to me :( > Yes I agree, that seems too complex. My idea was to only use the misc driver to bind different sub-devices, so that we can separate the functionality of the CLINT into separate drivers. If you are interested, I have pushed my WIP patch series for SMP support to Github [1]. It works, but is not finished yet and, in particular, must be rebased on top of your series. The misc driver for the CLINT0 is added in [2]. Thinking about this a bit more, I think your implementation might be best. RISC-V specifies that all implementation must have an mtime CSR. So it makes sense to have just one timer driver for RISC-V, which accesses mtime using an API. We can then have different syscon drivers to implement the IPI and timer related APIs. So for qemu, we would have one syscon driver for the CLINT0 (when U-Boot is running in machine mode) and one using the SBI functions (when U-Boot is running in supervisor mode). We would need more error handling though to, for example, handle the case where no syscon driver is bound. Essentially an interface for the syscon drivers to implement, similar to a uclass. Thanks, Lukas [1]: https://github.com/lukasauer/u-boot/commits/riscv-smp [2]: https://github.com/lukasauer/u-boot/commit/7901e68ed25ac8e3008f76a8f2c5a11e1b8e2952 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
