Re: [PATCH v4] PCI: qcom: Use default config space read function

From: Stanimir Varbanov <svarbanov@mm-sol.com>
To: Marc Gonzalez <marc.w.gonzalez@free.fr>,
	Bjorn Helgaas <bhelgaas@google.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Andy Gross <andy.gross@linaro.org>,
	David Brown <david.brown@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	PCI <linux-pci@vger.kernel.org>,
	MSM <linux-arm-msm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Jeffrey Hugo <jhugo@codeaurora.org>
Subject: Re: [PATCH v4] PCI: qcom: Use default config space read function
Date: Mon, 25 Mar 2019 15:32:21 +0200	[thread overview]
Message-ID: <df27f753-9464-02c0-b621-d5d7b54a3403@mm-sol.com> (raw)
In-Reply-To: <f142da4d-303a-89f3-ba5a-b6800d613e15@free.fr>

Hi Marc,

On 3/25/19 2:11 PM, Marc Gonzalez wrote:
> Stanimir,
> 
> Is v4 good enough for Bjorn to pick up?

Yes it is good but to avoid breaking another SoCs could you add fixups
for the following SoCs:

SoC		device ID
ipq4019 	0x1001
ipq8064		0x101
ipq8074		0x108

ipq8064 has the same device ID as apq8064, but I'm not sure do we need
defines per SoC or just rename DEV_ID_8064 ? I'm fine with both ways.

> 
> Regards.
> 
> On 18/03/2019 18:14, Marc Gonzalez wrote:
> 
>> We don't need to fudge the device class in qcom_pcie_rd_own_conf()
>> because dw_pcie_setup_rc() already does the right thing:
>>
>> 	/* Program correct class for RC */
>> 	dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI);
>>
>> However, the above has no effect on 8064, thus a fixup is required.
>>
>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>> Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>> Changes from v3 to v4: Define and use DEV_ID_8064 (not in include/linux/pci_ids.h because not shared)
>> ---
>>  drivers/pci/controller/dwc/pcie-qcom.c | 25 ++++++++-----------------
>>  1 file changed, 8 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index d185ea5fe996..712a83354f9d 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -1129,25 +1129,8 @@ static int qcom_pcie_host_init(struct pcie_port *pp)
>>  	return ret;
>>  }
>>  
>> -static int qcom_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
>> -				 u32 *val)
>> -{
>> -	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> -
>> -	/* the device class is not reported correctly from the register */
>> -	if (where == PCI_CLASS_REVISION && size == 4) {
>> -		*val = readl(pci->dbi_base + PCI_CLASS_REVISION);
>> -		*val &= 0xff;	/* keep revision id */
>> -		*val |= PCI_CLASS_BRIDGE_PCI << 16;
>> -		return PCIBIOS_SUCCESSFUL;
>> -	}
>> -
>> -	return dw_pcie_read(pci->dbi_base + where, size, val);
>> -}
>> -
>>  static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
>>  	.host_init = qcom_pcie_host_init,
>> -	.rd_own_conf = qcom_pcie_rd_own_conf,
>>  };
>>  
>>  /* Qcom IP rev.: 2.1.0	Synopsys IP rev.: 4.01a */
>> @@ -1309,6 +1292,14 @@ static const struct of_device_id qcom_pcie_match[] = {
>>  	{ }
>>  };
>>  
>> +#define DEV_ID_8064 0x0101
>> +
>> +static void qcom_fixup_class(struct pci_dev *dev)
>> +{
>> +	dev->class = PCI_CLASS_BRIDGE_PCI << 8;
>> +}
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, DEV_ID_8064, qcom_fixup_class);
>> +
>>  static struct platform_driver qcom_pcie_driver = {
>>  	.probe = qcom_pcie_probe,
>>  	.driver = {

-- 
regards,
Stan