Hi Lukas, On Thu, Dec 6, 2018 at 7:11 AM Auer, Lukas <lukas.a...@aisec.fraunhofer.de> wrote: > > 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 > > <lukas.a...@aisec.fraunhofer.de> 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 > > > > <lukas.a...@aisec.fraunhofer.de> 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 <bmeng...@gmail.com> > > > > > > --- > > > > > > > > > > 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 cannot find the clint compatible string for Andes chip too ... > > 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). Yes, having just one timer driver for RISC-V seems doable. I will try to refine current implementation in v2. > 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 BTW: I think I can drop my last patch in my series and let you handle the SMP boot in your series. Good work! Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
