Messages in this thread | | | Date | Tue, 08 Sep 2020 20:12:16 -0700 (PDT) | Subject | Re: [PATCH v2 2/3] soc: sifive: Add SiFive specific Cadence DDR controller driver | From | Palmer Dabbelt <> |
| |
On Sun, 06 Sep 2020 23:11:26 PDT (-0700), Christoph Hellwig wrote: > On Mon, Sep 07, 2020 at 11:17:58AM +0530, Yash Shah wrote: >> Add a driver to manage the Cadence DDR controller present on SiFive SoCs >> At present the driver manages the EDAC feature of the DDR controller. >> Additional features may be added to the driver in future to control >> other aspects of the DDR controller. > > So if this is a generic(ish) Cadence IP block shouldn't it be named > Cadence and made generic? Or is the frontend somehow SiFive specific?
For some reason I thought we had a SiFive-specific interface to this, but I may have gotten that confused with something else as it's been a while. Someone from SiFive would probably have a better idea, but it looks like the person I'd ask isn't thereany more so I'm all out of options ;)
It looks like there was a very similar driver posted by Dhananjay Kangude from Cadence in April: https://lkml.org/lkml/2020/4/6/358 . Some of the register definitions seem to be different, but the code I looked at is very similar so there's at least some bits that could be shared. I found a v4 of that patch set, but that was back in May: https://lkml.org/lkml/2020/5/11/912 . It alludes to a v5, but I can't find one. I've added Dhananjay, maybe he knows what's up?
I don't know enough about the block to know if the subtle difference in register names/offsets means. They look properly jumbled up (ie, not just an offset), so maybe there's just different versions or that's the SiFive-specific part I had bouncing around my head? Either way, it seems like one driver with some simple configuration could handle both of these -- either sticking the offsets in the DT (if they're going to be different everywhere) or by coming up with some version sort of thing (if there's a handful of these).
I'm now also a bit worried about the provenace of this code. The two drivers are errily similar -- for example, the variable definitions in handle_ce()
u64 err_c_addr = 0x0; u64 err_c_data = 0x0; u32 err_c_synd, err_c_id; u32 sig_val_l, sig_val_h;
are exactly the same.
| |