Re: [PATCH v2 2/3] soc: sifive: Add SiFive specific Cadence DDR controller driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux