On Thu 24 Jun 17:59 CDT 2021, AngeloGioacchino Del Regno wrote: > Il 24/06/21 23:07, Bjorn Andersson ha scritto: > > On Tue 22 Jun 09:36 CDT 2021, AngeloGioacchino Del Regno wrote: [..] > > > Hello Jassi, Bjorn > > > > > > I've read the entire thread and I can't say that Jassi is entirely wrong > > > but I also agree with Bjorn on this matter. > > > > > > This driver is here to "simply" manage the register offset in the APCS > > > IP, which is a pretty straightforward operation. > > > If you check in this driver, you will see that there's not much > > > duplication between the various qcom_apcs_ipc_data that we have for > > > all the different SoCs. > > > > > > Checking further, we can effectively reduce the amount of compatibles > > > in this driver by simply removing some "duplicated" instances and in > > > particular: > > > ipq6018, ipq8074, msm8916, msm8994, msm8998, sdm660 > > > > > > and eventually replacing them with either of: > > > - 8bits_apcs_data qcom,apcs-apps-global-8bit > > > qcom,apcs-kpss-global-8bit > > > > I don't like those compatibles, simply because the binding is supposed > > to describe the hardware block, not the fact that Linux _currently_ only > > pokes this one register. > > > > Since you've immediately misunderstood my naming, yeah, that wouldn't be > the best thing to use as a compatible. > Sorry, that was certainly not my intention. > > We could probably "qcom,apss-global" as a catch-all for at least sc7180, > > sc7280, sdm845, sm8150, sm8250 and sm8350. > > > > Doesn't look like a bad idea, but if we want to *enforce* writing also > the platform-specific compatible, I can see patch series going back > and forth and getting refused because this will not be really understood > by everyone, I think. > In this case, if writing the platform compatible is something mandatory, > the only way to really make sure to avoid losing time with reviews like > "[...] here you have to write also the platform compatible", is to just > keep the thing as it is. > My understanding from the DT maintainers is that the dts would be checked by the schema, but I suspect that you're right in that we might have some back and forth on the DT binding, but I don't think that's a big deal. > > But look at 8996 and 8998, both named "something-hmss-something", with > > different register layout. And a quick glance seems to indicate that > > sdm660 isn't a hmss after all :/ > > > > Starting from the fact that I don't clearly remember what-when-why of > my research done more than one year ago, I do remember that conclusion > was that, in this regard, SDM630/660 were "mostly the same" as MSM8998. > In any case, this is something that, at this point, is better get > verified, maybe. > Yeah, I presume that someone with the documentation would need to go through each one of these and see what kind of grouping there might be. And I also presume that there might be cases where the CPU clocks has moved into the secure world, so that even though the hardware block is the same the presence of a in-kernel clock driver in the implementation might differ. But just to clarify, I find it annoying having to sprinkle compatibles all over the place every time I try to get a new board up. So I am not against us trying to figure this out. Regards, Bjorn
