mbox series

[00/21] Add PCIe bridge node in DT for Qcom SoCs

Message ID 20240221-pcie-qcom-bridge-dts-v1-0-6c6df0f9450d@linaro.org
Headers show
Series Add PCIe bridge node in DT for Qcom SoCs | expand

Message

Manivannan Sadhasivam Feb. 21, 2024, 3:41 a.m. UTC
On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, this series adds a DT node for the
PCIe bridges across all SoCs.

There is no functionality change with this series, but the PCIe bridge
representation in DT will be necessary to add the DT node for the client
devices like the one proposed in power sequencing series [1].

- Mani

[1] https://lore.kernel.org/linux-arm-msm/20240216203215.40870-8-brgl@bgdev.pl/

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
Manivannan Sadhasivam (21):
      arm64: dts: qcom: sm8250: Add PCIe bridge node
      arm64: dts: qcom: sdm845: Add PCIe bridge node
      arm64: dts: qcom: sm8150: Add PCIe bridge node
      arm64: dts: qcom: sm8350: Add PCIe bridge node
      arm64: dts: qcom: sm8450: Add PCIe bridge node
      arm64: dts: qcom: sm8550: Add PCIe bridge node
      arm64: dts: qcom: sm8650: Add PCIe bridge node
      arm64: dts: qcom: sa8775p: Add PCIe bridge node
      arm64: dts: qcom: sc8280xp: Add PCIe bridge node
      arm64: dts: qcom: msm8998: Add PCIe bridge node
      arm64: dts: qcom: sc7280: Add PCIe bridge node
      arm64: dts: qcom: qcs404: Add PCIe bridge node
      arm64: dts: qcom: sc8180x: Add PCIe bridge node
      arm64: dts: qcom: msm8996: Add PCIe bridge node
      arm64: dts: qcom: ipq8074: Add PCIe bridge node
      arm64: dts: qcom: ipq6018: Add PCIe bridge node
      ARM: dts: qcom: ipq8064: Add PCIe bridge node
      ARM: dts: qcom: ipq4019: Add PCIe bridge node
      ARM: dts: qcom: apq8064: Add PCIe bridge node
      ARM: dts: qcom: sdx55: Add PCIe bridge node
      arm64: dts: qcom: sm8650: Use "pcie" as the node name instead of "pci"

 arch/arm/boot/dts/qcom/qcom-apq8064.dtsi           | 10 ++++++
 arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi           | 10 ++++++
 arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi           | 30 ++++++++++++++++
 arch/arm/boot/dts/qcom/qcom-sdx55.dtsi             | 10 ++++++
 arch/arm64/boot/dts/qcom/ipq6018.dtsi              | 10 ++++++
 arch/arm64/boot/dts/qcom/ipq8074.dtsi              | 20 +++++++++++
 arch/arm64/boot/dts/qcom/msm8996.dtsi              | 30 ++++++++++++++++
 arch/arm64/boot/dts/qcom/msm8998.dtsi              | 10 ++++++
 arch/arm64/boot/dts/qcom/qcs404.dtsi               | 10 ++++++
 arch/arm64/boot/dts/qcom/sa8775p.dtsi              | 20 +++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 10 ++++++
 arch/arm64/boot/dts/qcom/sc8180x.dtsi              | 40 ++++++++++++++++++++++
 .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |  8 -----
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi             | 40 ++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi               | 20 +++++++++++
 arch/arm64/boot/dts/qcom/sm8150.dtsi               | 20 +++++++++++
 arch/arm64/boot/dts/qcom/sm8250.dtsi               | 30 ++++++++++++++++
 arch/arm64/boot/dts/qcom/sm8350.dtsi               | 20 +++++++++++
 arch/arm64/boot/dts/qcom/sm8450.dtsi               | 20 +++++++++++
 arch/arm64/boot/dts/qcom/sm8550.dtsi               | 20 +++++++++++
 arch/arm64/boot/dts/qcom/sm8650.dtsi               | 24 +++++++++++--
 21 files changed, 402 insertions(+), 10 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240221-pcie-qcom-bridge-dts-b83c0d1b642b

Best regards,

Comments

Konrad Dybcio Feb. 21, 2024, 12:39 p.m. UTC | #1
On 21.02.2024 04:42, Manivannan Sadhasivam wrote:
> Qcom SoCs doesn't support legacy PCI, but only PCIe. So use the correct
> node name for the controller instances.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
Neil Armstrong Feb. 21, 2024, 12:46 p.m. UTC | #2
On 21/02/2024 04:41, Manivannan Sadhasivam wrote:
> On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> for each controller instance. Hence, add a node to represent the bridge.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 2df77123a8c7..57a1ea84aa59 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -2270,6 +2270,16 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>   			dma-coherent;
>   
>   			status = "disabled";
> +
> +			pcie@0 {
> +				device_type = "pci";
> +				reg = <0x0 0x0 0x0 0x0 0x0>;
> +				bus-range = <0x01 0xff>;
> +
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				ranges;
> +			};
>   		};
>   
>   		pcie0_phy: phy@1c06000 {
> @@ -2379,6 +2389,16 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>   				 <0x02000000 0 0x40300000 0 0x40300000 0 0x1fd00000>;
>   
>   			status = "disabled";
> +
> +			pcie@0 {
> +				device_type = "pci";
> +				reg = <0x0 0x0 0x0 0x0 0x0>;
> +				bus-range = <0x01 0xff>;
> +
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				ranges;
> +			};
>   		};
>   
>   		pcie1_phy: phy@1c0e000 {
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Manivannan Sadhasivam Feb. 22, 2024, 5:39 a.m. UTC | #3
On Wed, Feb 21, 2024 at 01:39:01PM +0100, Konrad Dybcio wrote:
> On 21.02.2024 04:41, Manivannan Sadhasivam wrote:
> > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > for each controller instance. Hence, add a node to represent the bridge.
> > 
> > While at it, let's remove the bridge properties from board dts as they are
> > now redundant.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |  8 -----
> >  arch/arm64/boot/dts/qcom/sc8280xp.dtsi             | 40 ++++++++++++++++++++++
> >  2 files changed, 40 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > index def3976bd5bb..f0a0115e08fa 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > @@ -733,14 +733,6 @@ &pcie4 {
> >  	status = "okay";
> >  
> >  	pcie@0 {
> > -		device_type = "pci";
> > -		reg = <0x0 0x0 0x0 0x0 0x0>;
> > -		#address-cells = <3>;
> > -		#size-cells = <2>;
> > -		ranges;
> > -
> > -		bus-range = <0x01 0xff>;
> > -
> >  		wifi@0 {
> 
> This doesn't seem right, pleas use a label
> 

Why? A node label is useful if we want to reference it at the root level in
board dts, but here it is not.

- Mani
Manivannan Sadhasivam March 18, 2024, 5:24 a.m. UTC | #4
On Sun, Mar 17, 2024 at 10:37:15PM -0500, Bjorn Andersson wrote:
> On Thu, Feb 22, 2024 at 11:09:58AM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Feb 21, 2024 at 01:39:01PM +0100, Konrad Dybcio wrote:
> > > On 21.02.2024 04:41, Manivannan Sadhasivam wrote:
> > > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > > for each controller instance. Hence, add a node to represent the bridge.
> > > > 
> > > > While at it, let's remove the bridge properties from board dts as they are
> > > > now redundant.
> > > > 
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > ---
> > > >  .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |  8 -----
> > > >  arch/arm64/boot/dts/qcom/sc8280xp.dtsi             | 40 ++++++++++++++++++++++
> > > >  2 files changed, 40 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > > > index def3976bd5bb..f0a0115e08fa 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > > > @@ -733,14 +733,6 @@ &pcie4 {
> > > >  	status = "okay";
> > > >  
> > > >  	pcie@0 {
> > > > -		device_type = "pci";
> > > > -		reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > -		#address-cells = <3>;
> > > > -		#size-cells = <2>;
> > > > -		ranges;
> > > > -
> > > > -		bus-range = <0x01 0xff>;
> > > > -
> > > >  		wifi@0 {
> > > 
> > > This doesn't seem right, pleas use a label
> > > 
> > 
> > Why? A node label is useful if we want to reference it at the root level in
> > board dts, but here it is not.
> > 
> 
> Giving the bridge a label and then adding wifi@0 as a child using that
> label in the dts is pretty much how we do for everything else.
> 
> I find this over-flattening hard to follow, but relying on child node
> names when extending the structure or adding properties have bitten us
> many times in the past.
> 
> As such, I think the desired result in the dts should be:
> 
> &pcie4 {
> 	status = "okay";
> };
> 
> &pcie4_bridge {
> 	wifi@0 {
> 		...
> 	};
> };
> 

Ok. Will change it in next version. I'm also waiting to conclude on representing
the PERST# and WAKE# properties properly in the schema [1]. Once that gets
finalized, I'll respin v2.

- Mani

[1] https://github.com/devicetree-org/dt-schema/pull/126

> Regards,
> Bjorn
> 
> > - Mani
> > 
> > -- 
> > மணிவண்ணன் சதாசிவம்