Patchwork [v2] PCI: qcom: Use default config space read function

login
register
mail settings
Submitter Marc Gonzalez
Date March 13, 2019, 11:45 a.m.
Message ID <7d3d788a-d6a3-a70b-adab-6c65771cacc4@free.fr>
Download mbox | patch
Permalink /patch/748081/
State New
Headers show

Comments

Marc Gonzalez - March 13, 2019, 11:45 a.m.
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);

Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
---
Changes from v1 to v2: Completely drop qcom_pcie_rd_own_conf
Srinivas, could you test this patch on apq/ipq8064?
---
 drivers/pci/controller/dwc/pcie-qcom.c | 17 -----------------
 1 file changed, 17 deletions(-)
Marc Gonzalez - March 13, 2019, 1:13 p.m.
On 13/03/2019 12:45, 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);

qcom-pcie 1c00000.pci: 1c00000.pci supply vdda not found, using dummy regulator
qcom-pcie 1c00000.pci: 1c00000.pci supply vddpe-3v3 not found, using dummy regulator
qcom-pcie 1c00000.pci: host bridge /soc/pci@1c00000 ranges:
qcom-pcie 1c00000.pci: Parsing ranges property...
qcom-pcie 1c00000.pci:    IO 0x1b200000..0x1b2fffff -> 0x1b200000
qcom-pcie 1c00000.pci:   MEM 0x1b300000..0x1bffffff -> 0x1b300000
qcom-pcie 1c00000.pci: Link up
qcom-pcie 1c00000.pci: PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [bus 00-ff]
pci_bus 0000:00: root bus resource [io  0x0000-0xfffff] (bus address [0x1b200000-0x1b2fffff])
pci_bus 0000:00: root bus resource [mem 0x1b300000-0x1bffffff]
pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [17cb:0105] type 01 class 0x060400
pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x00000fff 64bit]
pci 0000:00:00.0: PME# supported from D0 D3hot
pci 0000:00:00.0: PME# disabled
pci_bus 0000:00: fixups for bus
pci 0000:00:00.0: scanning [bus 01-ff] behind bridge, pass 0
pci_bus 0000:01: scanning bus
pci 0000:01:00.0: [1969:1083] type 00 class 0x020000
pci 0000:01:00.0: reg 0x10: [mem 0x1b300000-0x1b33ffff 64bit]
pci 0000:01:00.0: reg 0x18: [io  0x1000-0x107f]
pci 0000:01:00.0: PME# supported from D0 D1 D2 D3hot D3cold
pci 0000:01:00.0: PME# disabled
pci_bus 0000:01: fixups for bus
pci_bus 0000:01: bus scan returning with max=01
pci 0000:00:00.0: scanning [bus 01-ff] behind bridge, pass 1
pci_bus 0000:00: bus scan returning with max=ff
pci 0000:00:00.0: BAR 8: assigned [mem 0x1b300000-0x1b3fffff]
pci 0000:00:00.0: BAR 0: assigned [mem 0x1b400000-0x1b400fff 64bit]
pci 0000:00:00.0: BAR 7: assigned [io  0x1000-0x1fff]
pci 0000:01:00.0: BAR 0: assigned [mem 0x1b300000-0x1b33ffff 64bit]
pci 0000:01:00.0: BAR 2: assigned [io  0x1000-0x107f]
pci 0000:00:00.0: PCI bridge to [bus 01-ff]
pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
pci 0000:00:00.0:   bridge window [mem 0x1b300000-0x1b3fffff]
pci 0000:01:00.0: [Firmware Bug]: disabling VPD access (can't determine size of non-standard VPD format)

# lspci -vvv >tmp
pcilib: sysfs_read_vpd: read failed: Input/output error

# cat tmp
00:00.0 PCI bridge: Qualcomm Device 0105 (prog-if 00 [Normal decode])
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Interrupt: pin A routed to IRQ 25
        Region 0: Memory at 1b400000 (64-bit, non-prefetchable) [size=4K]
        Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
        I/O behind bridge: 00001000-00001fff [size=4K]
        Memory behind bridge: 1b300000-1b3fffff [size=1M]
        Prefetchable memory behind bridge: 00000000fff00000-00000000000fffff [empty]
        Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR-
        BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- FastB2B-
                PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
        Capabilities: [40] Power Management version 3
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-)
                Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [50] MSI: Enable+ Count=1/32 Maskable+ 64bit+
                Address: 00000000fbfff000  Data: 0000
                Masking: fffffffe  Pending: 00000000
        Capabilities: [70] Express (v2) Root Port (Slot-), MSI 00
                DevCap: MaxPayload 128 bytes, PhantFunc 0
                        ExtTag- RBE+
                DevCtl: Report errors: Correctable+ Non-Fatal+ Fatal+ Unsupported+
                        RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                        MaxPayload 128 bytes, MaxReadReq 512 bytes
                DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend-
                LnkCap: Port #0, Speed 5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <1us, L1 <16us
                        ClockPM- Surprise- LLActRep+ BwNot+ ASPMOptComp+
                LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- CommClk+
                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
                LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive+ BWMgmt+ ABWMgmt-
                RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna+ CRSVisible-
                RootCap: CRSVisible-
                RootSta: PME ReqID 0000, PMEStatus- PMEPending-
                DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Not Supported ARIFwd-
                         AtomicOpsCap: Routing- 32bit- 64bit- 128bitCAS-
                DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF Disabled ARIFwd-
                         AtomicOpsCtl: ReqEn- EgressBlck-
                LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-
                         Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
                         Compliance De-emphasis: -6dB
                LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1-
                         EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
        Capabilities: [100 v2] Advanced Error Reporting
                UESta:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
                UEMsk:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
                UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
                CESta:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
                CEMsk:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
                AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- ECRCChkCap+ ECRCChkEn-
                        MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap-
                HeaderLog: 00000000 00000000 00000000 00000000
                RootCmd: CERptEn+ NFERptEn+ FERptEn+
                RootSta: CERcvd- MultCERcvd- UERcvd- MultUERcvd-
                         FirstFatal- NonFatalMsg- FatalMsg- IntMsg 0
                ErrorSrc: ERR_COR: 0000 ERR_FATAL/NONFATAL: 0000
        Capabilities: [148 v1] Transaction Processing Hints
                No steering table available
        Capabilities: [1dc v1] L1 PM Substates
                L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+
                          PortCommonModeRestoreTime=70us PortTPowerOnTime=0us
                L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2- ASPM_L1.1-
                           T_CommonMode=70us LTR1.2_Threshold=0ns
                L1SubCtl2: T_PwrOn=10us
        Kernel driver in use: pcieport

01:00.0 Ethernet controller: Qualcomm Atheros AR8151 v2.0 Gigabit Ethernet (rev c0)
        Subsystem: Qualcomm Atheros AR8151 v2.0 Gigabit Ethernet
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Interrupt: pin A routed to IRQ 0
        Region 0: Memory at 1b300000 (64-bit, non-prefetchable) [size=256K]
        Region 2: I/O ports at 1000 [size=128]
        Capabilities: [40] Power Management version 3
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
                Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [48] MSI: Enable- Count=1/1 Maskable- 64bit+
                Address: 0000000000000000  Data: 0000
        Capabilities: [58] Express (v1) Endpoint, MSI 00
                DevCap: MaxPayload 4096 bytes, PhantFunc 0, Latency L0s unlimited, L1 unlimited
                        ExtTag- AttnBtn+ AttnInd+ PwrInd+ RBE+ FLReset- SlotPowerLimit 0.000W
                DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
                        RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
                        MaxPayload 128 bytes, MaxReadReq 512 bytes
                DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
                LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s unlimited, L1 unlimited
                        ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp-
                LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+
                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
                LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
        Capabilities: [6c] Vital Product Data
                Not readable
        Capabilities: [100 v1] Advanced Error Reporting
                UESta:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
                UEMsk:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
                UESvrt: DLP- SDES+ TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
                CESta:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
                CEMsk:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
                AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- ECRCChkCap+ ECRCChkEn-
                        MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap-
                HeaderLog: 00000000 00000000 00000000 00000000
        Capabilities: [180 v1] Device Serial Number ff-c0-2f-ab-1e-89-4e-ff
        Kernel driver in use: atl1c
Srinivas Kandagatla - March 13, 2019, 6:29 p.m.
Hi Marc,

On 13/03/2019 11:45, 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);
> 
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
> Changes from v1 to v2: Completely drop qcom_pcie_rd_own_conf
> Srinivas, could you test this patch on apq/ipq8064?
> ---
>   drivers/pci/controller/dwc/pcie-qcom.c | 17 -----------------
>   1 file changed, 17 deletions(-)
> 
I tried this patch on IFC6410 board which has APQ8064, and the pci does 
not work anymore after applying this patch.

non-working output of 'lspci -vvv' 
https://paste.ubuntu.com/p/ndmdsW4zkJ/ and bootlog 
ahttps://paste.ubuntu.com/p/CqVzGymnq8/

working without patch : https://paste.ubuntu.com/p/TJm4hgjGW4/

I have not debugged it any further other than just testing the patch.
Let me know if you need any more dumps for more debug.

Thanks,
srini

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 3de5510fd3d5..d0c5c7616b3e 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1131,25 +1131,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 */
>
Marc Gonzalez - March 13, 2019, 8:39 p.m.
On 13/03/2019 19:29, Srinivas Kandagatla wrote:

> On 13/03/2019 11:45, 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);
>>
>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>> ---
>> Changes from v1 to v2: Completely drop qcom_pcie_rd_own_conf
>> Srinivas, could you test this patch on apq/ipq8064?
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 17 -----------------
>>   1 file changed, 17 deletions(-)
>>
> I tried this patch on IFC6410 board which has APQ8064, and the pci does 
> not work anymore after applying this patch.
> 
> non-working output of 'lspci -vvv' 
> https://paste.ubuntu.com/p/ndmdsW4zkJ/ and bootlog 
> https://paste.ubuntu.com/p/CqVzGymnq8/
> 
> working without patch : https://paste.ubuntu.com/p/TJm4hgjGW4/
> 
> I have not debugged it any further other than just testing the patch.
> Let me know if you need any more dumps for more debug.

Could you pastebin the output of lspci -vvv -n in the working case?

I think that programming the device class might not work on APQ8064,
and therefore that platform requires the quirk.

So maybe I can add the quirk back, just for APQ8064?

Regards.
Srinivas Kandagatla - March 13, 2019, 9:52 p.m.
On 13/03/2019 20:39, Marc Gonzalez wrote:
>> working without patch :https://paste.ubuntu.com/p/TJm4hgjGW4/
>>
>> I have not debugged it any further other than just testing the patch.
>> Let me know if you need any more dumps for more debug.
> Could you pastebin the output of lspci -vvv -n in the working case?
This was already in my original reply:
working without patch : https://paste.ubuntu.com/p/TJm4hgjGW4/

--srini
Marc Gonzalez - March 13, 2019, 10:26 p.m.
On 13/03/2019 22:52, Srinivas Kandagatla wrote:

> On 13/03/2019 20:39, Marc Gonzalez wrote:
>
>> Could you pastebin the output of lspci -vvv -n in the working case?
>
> This was already in my original reply:
> working without patch : https://paste.ubuntu.com/p/TJm4hgjGW4/

I needed to see the numeric values, hence the -n flag ;-)

	lspci -vvv -n

Regards.
Srinivas Kandagatla - March 14, 2019, 11:10 a.m.
On 13/03/2019 22:26, Marc Gonzalez wrote:
> I needed to see the numeric values, hence the -n flag;-)
> 
> 	lspci -vvv -n
> 

Here is the output: https://paste.ubuntu.com/p/kFWZ4kXCxT/

Thanks,
srini

> Regards.

Patch

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 3de5510fd3d5..d0c5c7616b3e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1131,25 +1131,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 */