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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241106014024.GA1499855@bhelgaas>
Date: Tue, 5 Nov 2024 19:40:24 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Sricharan R <quic_srichara@...cinc.com>
Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kw@...ux.com,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	andersson@...nel.org, konrad.dybcio@...aro.org,
	manivannan.sadhasivam@...aro.org, linux-arm-msm@...r.kernel.org,
	linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V7 4/4] PCI: qcom: Add support for IPQ9574

On Thu, Aug 01, 2024 at 11:18:03AM +0530, Sricharan R wrote:
> From: devi priya <quic_devipriy@...cinc.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?

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?

> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Reviewed-by: Manivannan Sadhasivam <mani@...nel.org>
> Co-developed-by: Anusha Rao <quic_anusha@...cinc.com>
> Signed-off-by: Anusha Rao <quic_anusha@...cinc.com>
> Signed-off-by: devi priya <quic_devipriy@...cinc.com>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@...cinc.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
>