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

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

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.

- Mani

> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > Co-developed-by: Anusha Rao <quic_anusha@quicinc.com>
> > Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
> > Signed-off-by: devi priya <quic_devipriy@quicinc.com>
> > Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> > ---
> >  [V7] Rebased on top of [1] to avoid DBI/ATU mirroring. With that dropped
> >       the need for separate ops.
> >  [1] https://lore.kernel.org/linux-arm-msm/a01404d2-2f4d-4fb8-af9d-3db66d39acf7@quicinc.com/
> > 
> >  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 6976efb8e2f0..e9371f945900 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -1752,6 +1752,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> >  	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> >  	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> >  	{ .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> > +	{ .compatible = "qcom,pcie-ipq9574", .data = &cfg_2_9_0 },
> >  	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> >  	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> >  	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
> > -- 
> > 2.34.1
> > 

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