Re: [PATCH V7 4/4] PCI: qcom: Add support for IPQ9574

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Wed, Nov 06, 2024 at 10:53:12AM -0600, Bjorn Helgaas wrote:
> On Wed, Nov 06, 2024 at 02:56:15PM +0000, Manivannan Sadhasivam wrote:
> > On Tue, Nov 05, 2024 at 07:40:24PM -0600, Bjorn Helgaas wrote:
> > > On Thu, Aug 01, 2024 at 11:18:03AM +0530, Sricharan R wrote:
> > > > From: devi priya <quic_devipriy@quicinc.com>
> > > > 
> > > > The IPQ9574 platform has four Gen3 PCIe controllers:
> > > > two single-lane and two dual-lane based on SNPS core 5.70a.
> > > > 
> > > > QCOM IP rev is 1.27.0 and Synopsys IP rev is 5.80a.
> > > > Reuse all the members of 'ops_2_9_0'.
> > > 
> > > Wow, this is confusing.
> > > 
> > > "Based on SNPS core 5.70a", but "Synopsys IP rev is 5.80a."
> > > Are those supposed to match?  Or is it 5.70a of one thing but 5.80a of
> > > a different thing?
> > 
> > Hmm, I'm not sure why 5.70a is mentioned here. It seems irrelevant
> > (even if it is the base).
> > 
> > > And where does ops_2_9_0 come in?  The code comment says:
> > > 
> > >   /* Qcom IP rev.: 2.9.0  Synopsys IP rev.: 5.00a */
> > >   static const struct qcom_pcie_ops ops_2_9_0 = {
> > > 
> > > which doesn't match 1.27.0 or 5.70a or 5.80a.  In fact there's nothing
> > > in the file that matches 1.*27.*0
> > > 
> > > Honestly, I don't really care if you have all the versions here in the
> > > commit log.  But if the versions *are* here, can we make them make
> > > sense?
> > 
> > We name the 'ops' structure based on Qcom IP revision. And we reuse
> > it across the SoCs which are compatible. That's why ops_2_9_0 is
> > used for this SoC which has Qcom IP rev 1.27.0.
> 
> Got it.  So a family of compatible Qcom IP starts with 2.9.0 and newer
> members are 1.27.0 etc.  With no hint in the source about what the
> members of the family are.  Perfect sEnSe.
> 

You can get that information by mapping the ops structure with compatibles. I
feel less incentive in documenting the SoC names deriving the same IP.

And the reason why 2.9.0 ops is used by 1.27 is because, we ended up adding
support for 2.9.0 IP first and the lower version followed later.

- Mani

-- 
மணிவண்ணன் சதாசிவம்