On Mon, Nov 21, 2022 at 03:37:40PM +0200, Ilpo Järvinen wrote: > Previously you were against saying it clearly that it's Intel FPGA > specific when Matthew proposed changing the name to not sound something > too generic. If you're ok with that now, I'm happy to make such change. Saying it's for some Intel FPGA just makes it sound like it should only live within the driver for that FPGA. The issue with there being only one user: > > Perhaps you have some name for this > > interface? You're only adding one user here which isn't helping > > make the case that this is something generic. is part of it, as is the fact that the naming is so very generic. Even "Intel FPGA" seems to be heading to the generic side, this is presumably some specific thing rather than just something that everyone using a FPGA from Intel is going to need. The issue with this being overly specific isn't just the name, it's the code as well. > > I can't tell what you're trying to say here. Are you saying that > > this is somehow baked into some FPGA design so that it's memory > > mapped with only a few registers showing to the rest of the > > system rather than just having a substantial memory mapped > > window like is typically used for FPGAs, but someohow this > > register window stuff is implemented in the soft IP so people are > > just throwng vaugely similar interfaces into a random host mapped > > register layout? > > What I tried to say the users are not expected to be nicely confined into > drivers/mfd/ (and a single driver in there). So this interface is part of the physical IP surrounding the actual programmable bit of a FPGA or something? That doesn't seem entirely right though given the fact that the registers are apparently one of the things that gets moved around a lot. I still have no idea what this hardware actually looks like or what this code is trying to represent, especially given the very few things that you are trying to parameterise. It's really not obvious there's even any point in trying to share this code at the abstraction level you've gone for. Do you have any examples of even a second user that isn't this one MFD which you can share? > You didn't answer at all my question about where to place the code? > I'm repeating it with the context below since you cut it off: I keep telling you to either make this so that it's actually generic or just have register get/set operations in the regmap for the device using it. As things stand with the code you've sent there's a bunch of things like the way it's doing direct MMIO (which means it only works on top of memory mapped devices) and the absolute requirement for an idle command and a wait for completion which clearly look like this is device specific. > Whether that is "generic" enough to reside in drivers/base/regmap can > of course be debated but lets say I put it into drivers/mfd/ along with > the code currently using it. By doing that, we'll postpone this discussion > to the point when the first driver using it outside of drivers/mfd/ comes > by. At that point, having the indirect code in drivers/mfd/ is shown to > be a wrong choice. > It's of course nothing that couldn't be fixed by patches moving the code > around to some more preferred location. And that location likely turns out > to be drivers/base/regmap, no? Or do you have a better place for it in > that case? If you think this should be shared via regmap then make it shareable. That needs more work than just repainting the name.
Attachment:
signature.asc
Description: PGP signature
