[PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100

Qiang Yu posted 6 patches 2 months ago
There is a newer version of this series
[PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Qiang Yu 2 months ago
Describe PCIe3 controller and PHY. Also add required system resources like
regulators, clocks, interrupts and registers configuration for PCIe3.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 204 ++++++++++++++++++++++++-
 1 file changed, 203 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index a36076e3c56b..c615c930cf0c 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -744,7 +744,7 @@ gcc: clock-controller@100000 {
 
 			clocks = <&bi_tcxo_div2>,
 				 <&sleep_clk>,
-				 <0>,
+				 <&pcie3_phy>,
 				 <&pcie4_phy>,
 				 <&pcie5_phy>,
 				 <&pcie6a_phy>,
@@ -2907,6 +2907,208 @@ mmss_noc: interconnect@1780000 {
 			#interconnect-cells = <2>;
 		};
 
+		pcie3: pcie@1bd0000 {
+			device_type = "pci";
+			compatible = "qcom,pcie-x1e80100";
+			reg = <0x0 0x01bd0000 0x0 0x3000>,
+			      <0x0 0x78000000 0x0 0xf1d>,
+			      <0x0 0x78000f40 0x0 0xa8>,
+			      <0x0 0x78001000 0x0 0x1000>,
+			      <0x0 0x78100000 0x0 0x100000>,
+			      <0x0 0x01bd3000 0x0 0x1000>;
+			reg-names = "parf",
+				    "dbi",
+				    "elbi",
+				    "atu",
+				    "config",
+				    "mhi";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 0x100000>,
+				 <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 0x3d00000>,
+				 <0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 0x40000000>;
+			bus-range = <0x00 0xff>;
+
+			dma-coherent;
+
+			linux,pci-domain = <3>;
+			num-lanes = <8>;
+
+			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi0",
+					  "msi1",
+					  "msi2",
+					  "msi3",
+					  "msi4",
+					  "msi5",
+					  "msi6",
+					  "msi7",
+					  "global";
+
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 0 GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 0 GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 0 GIC_SPI 237 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 0 GIC_SPI 238 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE_3_AUX_CLK>,
+				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
+				 <&gcc GCC_PCIE_3_MSTR_AXI_CLK>,
+				 <&gcc GCC_PCIE_3_SLV_AXI_CLK>,
+				 <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>,
+				 <&gcc GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK>,
+				 <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>;
+			clock-names = "aux",
+				      "cfg",
+				      "bus_master",
+				      "bus_slave",
+				      "slave_q2a",
+				      "noc_aggr",
+				      "cnoc_sf_axi";
+
+			assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>;
+			assigned-clock-rates = <19200000>;
+
+			interconnects = <&pcie_south_anoc MASTER_PCIE_3 QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
+					 &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>;
+			interconnect-names = "pcie-mem",
+					     "cpu-pcie";
+
+			resets = <&gcc GCC_PCIE_3_BCR>,
+				 <&gcc GCC_PCIE_3_LINK_DOWN_BCR>;
+			reset-names = "pci",
+				      "link_down";
+
+			power-domains = <&gcc GCC_PCIE_3_GDSC>;
+
+			phys = <&pcie3_phy>;
+			phy-names = "pciephy";
+
+			operating-points-v2 = <&pcie3_opp_table>;
+
+			status = "disabled";
+
+			pcie3_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				/* GEN 1 x1 */
+				opp-2500000 {
+					opp-hz = /bits/ 64 <2500000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <250000 1>;
+				};
+
+				/* GEN 1 x2 and GEN 2 x1 */
+				opp-5000000 {
+					opp-hz = /bits/ 64 <5000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <500000 1>;
+				};
+
+				/* GEN 1 x4 and GEN 2 x2 */
+				opp-10000000 {
+					opp-hz = /bits/ 64 <10000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <1000000 1>;
+				};
+
+				/* GEN 1 x8 and GEN 2 x4 */
+				opp-20000000 {
+					opp-hz = /bits/ 64 <20000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <2000000 1>;
+				};
+
+				/* GEN 2 x8 */
+				opp-40000000 {
+					opp-hz = /bits/ 64 <40000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <4000000 1>;
+				};
+
+				/* GEN 3 x1 */
+				opp-8000000 {
+					opp-hz = /bits/ 64 <8000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <984500 1>;
+				};
+
+				/* GEN 3 x2 and GEN 4 x1 */
+				opp-16000000 {
+					opp-hz = /bits/ 64 <16000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <1969000 1>;
+				};
+
+				/* GEN 3 x4 and GEN 4 x2 */
+				opp-32000000 {
+					opp-hz = /bits/ 64 <32000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <3938000 1>;
+				};
+
+				/* GEN 3 x8 and GEN 4 x4 */
+				opp-64000000 {
+					opp-hz = /bits/ 64 <64000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <7876000 1>;
+				};
+
+				/* GEN 4 x8 */
+				opp-128000000 {
+					opp-hz = /bits/ 64 <128000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <15753000 1>;
+				};
+			};
+		};
+
+		pcie3_phy: phy@1be0000 {
+			compatible = "qcom,x1e80100-qmp-gen4x8-pcie-phy";
+			reg = <0 0x01be0000 0 0x10000>;
+
+			clocks = <&gcc GCC_PCIE_3_PHY_AUX_CLK>,
+				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
+				 <&tcsr TCSR_PCIE_8L_CLKREF_EN>,
+				 <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>,
+				 <&gcc GCC_PCIE_3_PIPE_CLK>,
+				 <&gcc GCC_PCIE_3_PIPEDIV2_CLK>;
+			clock-names = "aux",
+				      "cfg_ahb",
+				      "ref",
+				      "rchng",
+				      "pipe",
+				      "pipediv2";
+
+			resets = <&gcc GCC_PCIE_3_PHY_BCR>,
+				 <&gcc GCC_PCIE_3_NOCSR_COM_PHY_BCR>;
+			reset-names = "phy",
+				      "phy_nocsr";
+
+			assigned-clocks = <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>;
+			assigned-clock-rates = <100000000>;
+
+			power-domains = <&gcc GCC_PCIE_3_PHY_GDSC>;
+
+			#clock-cells = <0>;
+			clock-output-names = "pcie3_pipe_clk";
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
 		pcie6a: pci@1bf8000 {
 			device_type = "pci";
 			compatible = "qcom,pcie-x1e80100";
-- 
2.34.1
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Johan Hovold 2 months ago
On Tue, Sep 24, 2024 at 03:14:44AM -0700, Qiang Yu wrote:
> Describe PCIe3 controller and PHY. Also add required system resources like
> regulators, clocks, interrupts and registers configuration for PCIe3.

> @@ -2907,6 +2907,208 @@ mmss_noc: interconnect@1780000 {
>  			#interconnect-cells = <2>;
>  		};
>  
> +		pcie3: pcie@1bd0000 {
> +			device_type = "pci";
> +			compatible = "qcom,pcie-x1e80100";

> +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "msi0",
> +					  "msi1",
> +					  "msi2",
> +					  "msi3",
> +					  "msi4",
> +					  "msi5",
> +					  "msi6",
> +					  "msi7",
> +					  "global";

This ninth "global" interrupt is not described by the bindings, which
would also need to be updated. What is it used for?

Johan
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Qiang Yu 2 months ago
On 9/24/2024 10:43 PM, Johan Hovold wrote:
> On Tue, Sep 24, 2024 at 03:14:44AM -0700, Qiang Yu wrote:
>> Describe PCIe3 controller and PHY. Also add required system resources like
>> regulators, clocks, interrupts and registers configuration for PCIe3.
>> @@ -2907,6 +2907,208 @@ mmss_noc: interconnect@1780000 {
>>   			#interconnect-cells = <2>;
>>   		};
>>   
>> +		pcie3: pcie@1bd0000 {
>> +			device_type = "pci";
>> +			compatible = "qcom,pcie-x1e80100";
>> +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "msi0",
>> +					  "msi1",
>> +					  "msi2",
>> +					  "msi3",
>> +					  "msi4",
>> +					  "msi5",
>> +					  "msi6",
>> +					  "msi7",
>> +					  "global";
> This ninth "global" interrupt is not described by the bindings, which
> would also need to be updated. What is it used for?

As of now, the global interrupts is mainly used to get link up event so
that the device driver can enumerate the PCIe endpoint devices without
user intervention. You can refer to
https://lore.kernel.org/linux-pci/20240828-pci-qcom-hotplug-v4-11-263a385fbbcb@linaro.org.

I see this global interrupts has been documented in qcom,pcie-sm8450.yaml.
Do I need to move it to qcom,pcie-common.yaml?

Thanks,
Qiang
>
> Johan
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Manivannan Sadhasivam 2 months ago
On Wed, Sep 25, 2024 at 02:37:41PM +0800, Qiang Yu wrote:
> 
> On 9/24/2024 10:43 PM, Johan Hovold wrote:
> > On Tue, Sep 24, 2024 at 03:14:44AM -0700, Qiang Yu wrote:
> > > Describe PCIe3 controller and PHY. Also add required system resources like
> > > regulators, clocks, interrupts and registers configuration for PCIe3.
> > > @@ -2907,6 +2907,208 @@ mmss_noc: interconnect@1780000 {
> > >   			#interconnect-cells = <2>;
> > >   		};
> > > +		pcie3: pcie@1bd0000 {
> > > +			device_type = "pci";
> > > +			compatible = "qcom,pcie-x1e80100";
> > > +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
> > > +			interrupt-names = "msi0",
> > > +					  "msi1",
> > > +					  "msi2",
> > > +					  "msi3",
> > > +					  "msi4",
> > > +					  "msi5",
> > > +					  "msi6",
> > > +					  "msi7",
> > > +					  "global";
> > This ninth "global" interrupt is not described by the bindings, which
> > would also need to be updated. What is it used for?
> 
> As of now, the global interrupts is mainly used to get link up event so
> that the device driver can enumerate the PCIe endpoint devices without
> user intervention. You can refer to
> https://lore.kernel.org/linux-pci/20240828-pci-qcom-hotplug-v4-11-263a385fbbcb@linaro.org.
> 
> I see this global interrupts has been documented in qcom,pcie-sm8450.yaml.
> Do I need to move it to qcom,pcie-common.yaml?
> 

No, you need to describe it in qcom,pcie-x1e80100.yaml.

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Konrad Dybcio 2 months ago
On 24.09.2024 12:14 PM, Qiang Yu wrote:
> Describe PCIe3 controller and PHY. Also add required system resources like
> regulators, clocks, interrupts and registers configuration for PCIe3.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Qiang, Mani

I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop.
Adding the global irq breaks sdcard detection (the chip still comes
up fine) somehow. Removing the irq makes it work again :|

I've confirmed that the irq number is correct

Konrad
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Manivannan Sadhasivam 2 months ago
On Tue, Sep 24, 2024 at 04:26:34PM +0200, Konrad Dybcio wrote:
> On 24.09.2024 12:14 PM, Qiang Yu wrote:
> > Describe PCIe3 controller and PHY. Also add required system resources like
> > regulators, clocks, interrupts and registers configuration for PCIe3.
> > 
> > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> 
> Qiang, Mani
> 
> I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop.

Is it based on x1e80100?

> Adding the global irq breaks sdcard detection (the chip still comes
> up fine) somehow. Removing the irq makes it work again :|
> 
> I've confirmed that the irq number is correct
> 

Yeah, I did see some issues with MSI on SM8250 (RB5) when global interrupts are
enabled and I'm working with the hw folks to understand what is going on. But
I didn't see the same issues on newer platforms (sa8775p etc...).

Can you please confirm if the issue is due to MSI not being received from the
device? Checking the /proc/interrutps is enough.

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Konrad Dybcio 2 months ago
On 25.09.2024 10:05 AM, Manivannan Sadhasivam wrote:
> On Tue, Sep 24, 2024 at 04:26:34PM +0200, Konrad Dybcio wrote:
>> On 24.09.2024 12:14 PM, Qiang Yu wrote:
>>> Describe PCIe3 controller and PHY. Also add required system resources like
>>> regulators, clocks, interrupts and registers configuration for PCIe3.
>>>
>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>
>> Qiang, Mani
>>
>> I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop.
> 
> Is it based on x1e80100?

You would think so :P

> 
>> Adding the global irq breaks sdcard detection (the chip still comes
>> up fine) somehow. Removing the irq makes it work again :|
>>
>> I've confirmed that the irq number is correct
>>
> 
> Yeah, I did see some issues with MSI on SM8250 (RB5) when global interrupts are
> enabled and I'm working with the hw folks to understand what is going on. But
> I didn't see the same issues on newer platforms (sa8775p etc...).
> 
> Can you please confirm if the issue is due to MSI not being received from the
> device? Checking the /proc/interrutps is enough.

There's no msi-map for PCIe3. I recall +Johan talking about some sort of
a bug that prevents us from adding it?

Konrad
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Konrad Dybcio 2 months ago
On 25.09.2024 11:30 AM, Konrad Dybcio wrote:
> On 25.09.2024 10:05 AM, Manivannan Sadhasivam wrote:
>> On Tue, Sep 24, 2024 at 04:26:34PM +0200, Konrad Dybcio wrote:
>>> On 24.09.2024 12:14 PM, Qiang Yu wrote:
>>>> Describe PCIe3 controller and PHY. Also add required system resources like
>>>> regulators, clocks, interrupts and registers configuration for PCIe3.
>>>>
>>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> ---
>>>
>>> Qiang, Mani
>>>
>>> I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop.
>>
>> Is it based on x1e80100?
> 
> You would think so :P
> 
>>
>>> Adding the global irq breaks sdcard detection (the chip still comes
>>> up fine) somehow. Removing the irq makes it work again :|
>>>
>>> I've confirmed that the irq number is correct
>>>
>>
>> Yeah, I did see some issues with MSI on SM8250 (RB5) when global interrupts are
>> enabled and I'm working with the hw folks to understand what is going on. But
>> I didn't see the same issues on newer platforms (sa8775p etc...).
>>
>> Can you please confirm if the issue is due to MSI not being received from the
>> device? Checking the /proc/interrutps is enough.
> 
> There's no msi-map for PCIe3. I recall +Johan talking about some sort of
> a bug that prevents us from adding it?

Unless you just meant the msi0..=7 interrupts, then yeah, I only get one irq
event with "global" in place and it seems to never get more

Konrad
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Manivannan Sadhasivam 2 months ago
On Wed, Sep 25, 2024 at 11:46:35AM +0200, Konrad Dybcio wrote:
> On 25.09.2024 11:30 AM, Konrad Dybcio wrote:
> > On 25.09.2024 10:05 AM, Manivannan Sadhasivam wrote:
> >> On Tue, Sep 24, 2024 at 04:26:34PM +0200, Konrad Dybcio wrote:
> >>> On 24.09.2024 12:14 PM, Qiang Yu wrote:
> >>>> Describe PCIe3 controller and PHY. Also add required system resources like
> >>>> regulators, clocks, interrupts and registers configuration for PCIe3.
> >>>>
> >>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> >>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>> ---
> >>>
> >>> Qiang, Mani
> >>>
> >>> I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop.
> >>
> >> Is it based on x1e80100?
> > 
> > You would think so :P
> > 
> >>
> >>> Adding the global irq breaks sdcard detection (the chip still comes
> >>> up fine) somehow. Removing the irq makes it work again :|
> >>>
> >>> I've confirmed that the irq number is correct
> >>>
> >>
> >> Yeah, I did see some issues with MSI on SM8250 (RB5) when global interrupts are
> >> enabled and I'm working with the hw folks to understand what is going on. But
> >> I didn't see the same issues on newer platforms (sa8775p etc...).
> >>
> >> Can you please confirm if the issue is due to MSI not being received from the
> >> device? Checking the /proc/interrutps is enough.
> > 
> > There's no msi-map for PCIe3. I recall +Johan talking about some sort of
> > a bug that prevents us from adding it?
> 

Yeah, that's for using GIC-ITS to receive MSIs. But the default one is the
internal MSI controller in DWC.

> Unless you just meant the msi0..=7 interrupts, then yeah, I only get one irq
> event with "global" in place and it seems to never get more
> 

Ok. Then most likely the same issue I saw on SM8250 as well. But I'm confused
why Qiang didn't see the issue. I specifically asked him to add the global
interrupt and test it with the endpoint to check if the issue arises or not.

Qiang, can you please confirm?

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Qiang Yu 2 months ago
On 9/25/2024 8:52 PM, Manivannan Sadhasivam wrote:
> On Wed, Sep 25, 2024 at 11:46:35AM +0200, Konrad Dybcio wrote:
>> On 25.09.2024 11:30 AM, Konrad Dybcio wrote:
>>> On 25.09.2024 10:05 AM, Manivannan Sadhasivam wrote:
>>>> On Tue, Sep 24, 2024 at 04:26:34PM +0200, Konrad Dybcio wrote:
>>>>> On 24.09.2024 12:14 PM, Qiang Yu wrote:
>>>>>> Describe PCIe3 controller and PHY. Also add required system resources like
>>>>>> regulators, clocks, interrupts and registers configuration for PCIe3.
>>>>>>
>>>>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>> ---
>>>>> Qiang, Mani
>>>>>
>>>>> I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop.
>>>> Is it based on x1e80100?
>>> You would think so :P
>>>
>>>>> Adding the global irq breaks sdcard detection (the chip still comes
>>>>> up fine) somehow. Removing the irq makes it work again :|
>>>>>
>>>>> I've confirmed that the irq number is correct
>>>>>
>>>> Yeah, I did see some issues with MSI on SM8250 (RB5) when global interrupts are
>>>> enabled and I'm working with the hw folks to understand what is going on. But
>>>> I didn't see the same issues on newer platforms (sa8775p etc...).
>>>>
>>>> Can you please confirm if the issue is due to MSI not being received from the
>>>> device? Checking the /proc/interrutps is enough.
>>> There's no msi-map for PCIe3. I recall +Johan talking about some sort of
>>> a bug that prevents us from adding it?
> Yeah, that's for using GIC-ITS to receive MSIs. But the default one is the
> internal MSI controller in DWC.
>
>> Unless you just meant the msi0..=7 interrupts, then yeah, I only get one irq
>> event with "global" in place and it seems to never get more
>>
> Ok. Then most likely the same issue I saw on SM8250 as well. But I'm confused
> why Qiang didn't see the issue. I specifically asked him to add the global
> interrupt and test it with the endpoint to check if the issue arises or not.
>
> Qiang, can you please confirm?
Sorry, I misunderstood what you mean. I only verified if link was up but 
ignored the status of ep device
driver.

But look like this issue is because snpsys MSI irq is msked during probe.
Can you please also unmask BIT(23) - BIT(30) when unmask 
PARF_INT_ALL_LINK_UP,
like this

@@ -1772,7 +1772,8 @@ static int qcom_pcie_probe(struct platform_device 
*pdev)
                         goto err_host_deinit;
                 }

-               writel_relaxed(PARF_INT_ALL_LINK_UP, pcie->parf + 
PARF_INT_ALL_MASK);
+               writel_relaxed(PARF_INT_ALL_LINK_UP | GENMASK(30, 23), 
pcie->parf + PARF_INT_ALL_MASK);
+               dev_err(dev, "INT_ALL_MASK: 0x%x\n", 
readl_relaxed(pcie->parf + PARF_INT_ALL_MASK));
         }

After that, this issue is fixed on my setup.

Thanks,
Qiang
>
> - Mani
>
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Manivannan Sadhasivam 1 month, 4 weeks ago
On Thu, Sep 26, 2024 at 11:15:34AM +0800, Qiang Yu wrote:
> 
> On 9/25/2024 8:52 PM, Manivannan Sadhasivam wrote:
> > On Wed, Sep 25, 2024 at 11:46:35AM +0200, Konrad Dybcio wrote:
> > > On 25.09.2024 11:30 AM, Konrad Dybcio wrote:
> > > > On 25.09.2024 10:05 AM, Manivannan Sadhasivam wrote:
> > > > > On Tue, Sep 24, 2024 at 04:26:34PM +0200, Konrad Dybcio wrote:
> > > > > > On 24.09.2024 12:14 PM, Qiang Yu wrote:
> > > > > > > Describe PCIe3 controller and PHY. Also add required system resources like
> > > > > > > regulators, clocks, interrupts and registers configuration for PCIe3.
> > > > > > > 
> > > > > > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> > > > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > > ---
> > > > > > Qiang, Mani
> > > > > > 
> > > > > > I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop.
> > > > > Is it based on x1e80100?
> > > > You would think so :P
> > > > 
> > > > > > Adding the global irq breaks sdcard detection (the chip still comes
> > > > > > up fine) somehow. Removing the irq makes it work again :|
> > > > > > 
> > > > > > I've confirmed that the irq number is correct
> > > > > > 
> > > > > Yeah, I did see some issues with MSI on SM8250 (RB5) when global interrupts are
> > > > > enabled and I'm working with the hw folks to understand what is going on. But
> > > > > I didn't see the same issues on newer platforms (sa8775p etc...).
> > > > > 
> > > > > Can you please confirm if the issue is due to MSI not being received from the
> > > > > device? Checking the /proc/interrutps is enough.
> > > > There's no msi-map for PCIe3. I recall +Johan talking about some sort of
> > > > a bug that prevents us from adding it?
> > Yeah, that's for using GIC-ITS to receive MSIs. But the default one is the
> > internal MSI controller in DWC.
> > 
> > > Unless you just meant the msi0..=7 interrupts, then yeah, I only get one irq
> > > event with "global" in place and it seems to never get more
> > > 
> > Ok. Then most likely the same issue I saw on SM8250 as well. But I'm confused
> > why Qiang didn't see the issue. I specifically asked him to add the global
> > interrupt and test it with the endpoint to check if the issue arises or not.
> > 
> > Qiang, can you please confirm?
> Sorry, I misunderstood what you mean. I only verified if link was up but
> ignored the status of ep device
> driver.
> 
> But look like this issue is because snpsys MSI irq is msked during probe.
> Can you please also unmask BIT(23) - BIT(30) when unmask
> PARF_INT_ALL_LINK_UP,
> like this
> 
> @@ -1772,7 +1772,8 @@ static int qcom_pcie_probe(struct platform_device
> *pdev)
>                         goto err_host_deinit;
>                 }
> 
> -               writel_relaxed(PARF_INT_ALL_LINK_UP, pcie->parf +
> PARF_INT_ALL_MASK);
> +               writel_relaxed(PARF_INT_ALL_LINK_UP | GENMASK(30, 23),
> pcie->parf + PARF_INT_ALL_MASK);
> +               dev_err(dev, "INT_ALL_MASK: 0x%x\n",
> readl_relaxed(pcie->parf + PARF_INT_ALL_MASK));
>         }
> 
> After that, this issue is fixed on my setup.
> 

I did see those bits, but they were mentioned as diagnostic interrupts. So I was
not sure why disabling them masks MSI. Furthermore, MSI seems to work on SM8450
without enabling these bits.

Anyway, since you mentioned that it helps in bringing back MSI on this platform,
it doesn't hurt to enable these interrupts. I will post a patch for that,
thanks!

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Qiang Yu 2 months ago
On 9/24/2024 10:26 PM, Konrad Dybcio wrote:
> On 24.09.2024 12:14 PM, Qiang Yu wrote:
>> Describe PCIe3 controller and PHY. Also add required system resources like
>> regulators, clocks, interrupts and registers configuration for PCIe3.
>>
>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
> Qiang, Mani
>
> I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop.
> Adding the global irq breaks sdcard detection (the chip still comes
> up fine) somehow. Removing the irq makes it work again :|
>
> I've confirmed that the irq number is correct
Actually, I verified this patch with several devices (SDX75, AIC100 and
R8169 of TP-LINK) attached to X1E80100 QCP. But I didn't meet this issue.
Each device was detected. Can you please share boot log if it is possible?

Mani, did you ever meet similar issue on other platforms?

0003:01:00.0 Class 0200: Device 10ec:8161 (rev 15)
         Subsystem: Device 10ec:8168
         I/O ports at 100000 [size=256]
         Memory at 78304000 (64-bit, non-prefetchable) [size=4K]
         Memory at 78300000 (64-bit, non-prefetchable) [size=16K]
         Kernel driver in use: r8169

0003:01:00.0 Class ff00: Device 17cb:0309
         Subsystem: Device 17cb:0309
         Memory at 78580000 (64-bit, non-prefetchable) [size=4K]
         Memory at 78581000 (64-bit, non-prefetchable) [size=4K]
         Kernel driver in use: mhi-pci-generic

0003:01:00.0 Class 1200: Device 17cb:a100
         Subsystem: Device 17cb:a100
         Region 0: Memory at 78300000 (64-bit, non-prefetchable) 
[disabled] [size=4K]
         Region 2: Memory at 78400000 (64-bit, non-prefetchable) 
[disabled] [size=2M]
         Region 4: Memory at 78600000 (64-bit, prefetchable) [disabled] 
[size=64K]

Thanks,
Qiang Yu
>
> Konrad
Re: [PATCH v4 6/6] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
Posted by Manivannan Sadhasivam 2 months ago
On Tue, Sep 24, 2024 at 03:14:44AM -0700, Qiang Yu wrote:
> Describe PCIe3 controller and PHY. Also add required system resources like
> regulators, clocks, interrupts and registers configuration for PCIe3.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 204 ++++++++++++++++++++++++-
>  1 file changed, 203 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index a36076e3c56b..c615c930cf0c 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -744,7 +744,7 @@ gcc: clock-controller@100000 {
>  
>  			clocks = <&bi_tcxo_div2>,
>  				 <&sleep_clk>,
> -				 <0>,
> +				 <&pcie3_phy>,
>  				 <&pcie4_phy>,
>  				 <&pcie5_phy>,
>  				 <&pcie6a_phy>,
> @@ -2907,6 +2907,208 @@ mmss_noc: interconnect@1780000 {
>  			#interconnect-cells = <2>;
>  		};
>  
> +		pcie3: pcie@1bd0000 {
> +			device_type = "pci";
> +			compatible = "qcom,pcie-x1e80100";
> +			reg = <0x0 0x01bd0000 0x0 0x3000>,
> +			      <0x0 0x78000000 0x0 0xf1d>,
> +			      <0x0 0x78000f40 0x0 0xa8>,
> +			      <0x0 0x78001000 0x0 0x1000>,
> +			      <0x0 0x78100000 0x0 0x100000>,
> +			      <0x0 0x01bd3000 0x0 0x1000>;
> +			reg-names = "parf",
> +				    "dbi",
> +				    "elbi",
> +				    "atu",
> +				    "config",
> +				    "mhi";
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 0x100000>,
> +				 <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 0x3d00000>,
> +				 <0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 0x40000000>;
> +			bus-range = <0x00 0xff>;
> +
> +			dma-coherent;
> +
> +			linux,pci-domain = <3>;
> +			num-lanes = <8>;
> +
> +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "msi0",
> +					  "msi1",
> +					  "msi2",
> +					  "msi3",
> +					  "msi4",
> +					  "msi5",
> +					  "msi6",
> +					  "msi7",
> +					  "global";
> +
> +			#interrupt-cells = <1>;
> +			interrupt-map-mask = <0 0 0 0x7>;
> +			interrupt-map = <0 0 0 1 &intc 0 0 GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 2 &intc 0 0 GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 3 &intc 0 0 GIC_SPI 237 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 4 &intc 0 0 GIC_SPI 238 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			clocks = <&gcc GCC_PCIE_3_AUX_CLK>,
> +				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
> +				 <&gcc GCC_PCIE_3_MSTR_AXI_CLK>,
> +				 <&gcc GCC_PCIE_3_SLV_AXI_CLK>,
> +				 <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>,
> +				 <&gcc GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK>,
> +				 <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>;
> +			clock-names = "aux",
> +				      "cfg",
> +				      "bus_master",
> +				      "bus_slave",
> +				      "slave_q2a",
> +				      "noc_aggr",
> +				      "cnoc_sf_axi";
> +
> +			assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>;
> +			assigned-clock-rates = <19200000>;
> +
> +			interconnects = <&pcie_south_anoc MASTER_PCIE_3 QCOM_ICC_TAG_ALWAYS
> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
> +					 &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>;
> +			interconnect-names = "pcie-mem",
> +					     "cpu-pcie";
> +
> +			resets = <&gcc GCC_PCIE_3_BCR>,
> +				 <&gcc GCC_PCIE_3_LINK_DOWN_BCR>;
> +			reset-names = "pci",
> +				      "link_down";
> +
> +			power-domains = <&gcc GCC_PCIE_3_GDSC>;
> +
> +			phys = <&pcie3_phy>;
> +			phy-names = "pciephy";
> +
> +			operating-points-v2 = <&pcie3_opp_table>;
> +
> +			status = "disabled";
> +
> +			pcie3_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				/* GEN 1 x1 */
> +				opp-2500000 {
> +					opp-hz = /bits/ 64 <2500000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <250000 1>;
> +				};
> +
> +				/* GEN 1 x2 and GEN 2 x1 */
> +				opp-5000000 {
> +					opp-hz = /bits/ 64 <5000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <500000 1>;
> +				};
> +
> +				/* GEN 1 x4 and GEN 2 x2 */
> +				opp-10000000 {
> +					opp-hz = /bits/ 64 <10000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <1000000 1>;
> +				};
> +
> +				/* GEN 1 x8 and GEN 2 x4 */
> +				opp-20000000 {
> +					opp-hz = /bits/ 64 <20000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <2000000 1>;
> +				};
> +
> +				/* GEN 2 x8 */
> +				opp-40000000 {
> +					opp-hz = /bits/ 64 <40000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <4000000 1>;
> +				};
> +
> +				/* GEN 3 x1 */
> +				opp-8000000 {
> +					opp-hz = /bits/ 64 <8000000>;
> +					required-opps = <&rpmhpd_opp_svs>;
> +					opp-peak-kBps = <984500 1>;
> +				};
> +
> +				/* GEN 3 x2 and GEN 4 x1 */
> +				opp-16000000 {
> +					opp-hz = /bits/ 64 <16000000>;
> +					required-opps = <&rpmhpd_opp_svs>;
> +					opp-peak-kBps = <1969000 1>;
> +				};
> +
> +				/* GEN 3 x4 and GEN 4 x2 */
> +				opp-32000000 {
> +					opp-hz = /bits/ 64 <32000000>;
> +					required-opps = <&rpmhpd_opp_svs>;
> +					opp-peak-kBps = <3938000 1>;
> +				};
> +
> +				/* GEN 3 x8 and GEN 4 x4 */
> +				opp-64000000 {
> +					opp-hz = /bits/ 64 <64000000>;
> +					required-opps = <&rpmhpd_opp_svs>;
> +					opp-peak-kBps = <7876000 1>;
> +				};
> +
> +				/* GEN 4 x8 */
> +				opp-128000000 {
> +					opp-hz = /bits/ 64 <128000000>;
> +					required-opps = <&rpmhpd_opp_svs>;
> +					opp-peak-kBps = <15753000 1>;
> +				};
> +			};
> +		};
> +
> +		pcie3_phy: phy@1be0000 {
> +			compatible = "qcom,x1e80100-qmp-gen4x8-pcie-phy";
> +			reg = <0 0x01be0000 0 0x10000>;
> +
> +			clocks = <&gcc GCC_PCIE_3_PHY_AUX_CLK>,
> +				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
> +				 <&tcsr TCSR_PCIE_8L_CLKREF_EN>,
> +				 <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>,
> +				 <&gcc GCC_PCIE_3_PIPE_CLK>,
> +				 <&gcc GCC_PCIE_3_PIPEDIV2_CLK>;
> +			clock-names = "aux",
> +				      "cfg_ahb",
> +				      "ref",
> +				      "rchng",
> +				      "pipe",
> +				      "pipediv2";
> +
> +			resets = <&gcc GCC_PCIE_3_PHY_BCR>,
> +				 <&gcc GCC_PCIE_3_NOCSR_COM_PHY_BCR>;
> +			reset-names = "phy",
> +				      "phy_nocsr";
> +
> +			assigned-clocks = <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>;
> +			assigned-clock-rates = <100000000>;
> +
> +			power-domains = <&gcc GCC_PCIE_3_PHY_GDSC>;
> +
> +			#clock-cells = <0>;
> +			clock-output-names = "pcie3_pipe_clk";
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
>  		pcie6a: pci@1bf8000 {
>  			device_type = "pci";
>  			compatible = "qcom,pcie-x1e80100";
> -- 
> 2.34.1
> 

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