[PATCH 1/2] ARM: dts: qcom: sdx55: Move reset and wake gpios to board dts

[PATCH 1/2] ARM: dts: qcom: sdx55: Move reset and wake gpios to board dts

[Manivannan Sadhasivam]

The reset and wake properties in the PCIe EP node belong to the board dts
as they can be customized per board design. So let's move them from SoC
dtsi.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts | 3 +++
 arch/arm/boot/dts/qcom-sdx55.dtsi                | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
index 81f33eba39e5..b73b707342af 100644
--- a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
+++ b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
@@ -255,6 +255,9 @@ &pcie_ep {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie_ep_clkreq_default &pcie_ep_perst_default
 		     &pcie_ep_wake_default>;
+
+	reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
 };
 
 &qpic_bam {
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi b/arch/arm/boot/dts/qcom-sdx55.dtsi
index 286fa92da428..bc310ed01b40 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -421,8 +421,6 @@ pcie_ep: pcie-ep@1c00000 {
 				     <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "global",
 					  "doorbell";
-			reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
-			wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
 			resets = <&gcc GCC_PCIE_BCR>;
 			reset-names = "core";
 			power-domains = <&gcc PCIE_GDSC>;
-- 
2.25.1

[PATCH 2/2] ARM: dts: qcom: sdx55-fn980: Move "status" property to the end of node

[Manivannan Sadhasivam]

To align with rest of the devicetree files, let's move the "status"
property to the end of the nodes.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 .../boot/dts/qcom-sdx55-telit-fn980-tlb.dts   | 20 ++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
index b73b707342af..91d13c40496c 100644
--- a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
+++ b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
@@ -243,21 +243,21 @@ &ipa {
 };
 
 &pcie_phy {
-	status = "okay";
-
 	vdda-phy-supply = <&vreg_l1e_bb_1p2>;
 	vdda-pll-supply = <&vreg_l4e_bb_0p875>;
-};
 
-&pcie_ep {
 	status = "okay";
+};
 
+&pcie_ep {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie_ep_clkreq_default &pcie_ep_perst_default
 		     &pcie_ep_wake_default>;
 
 	reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
+
+	status = "okay";
 };
 
 &qpic_bam {
@@ -265,8 +265,6 @@ &qpic_bam {
 };
 
 &qpic_nand {
-	status = "okay";
-
 	nand@0 {
 		reg = <0>;
 
@@ -277,11 +275,13 @@ nand@0 {
 		secure-regions = /bits/ 64 <0x500000 0x500000
 					    0xa00000 0xb00000>;
 	};
+
+	status = "okay";
 };
 
 &remoteproc_mpss {
-	status = "okay";
 	memory-region = <&mpss_adsp_mem>;
+	status = "okay";
 };
 
 &tlmm {
@@ -308,16 +308,18 @@ pcie_ep_wake_default: pcie-ep-wake-default-state {
 };
 
 &usb_hsphy {
-	status = "okay";
 	vdda-pll-supply = <&vreg_l4e_bb_0p875>;
 	vdda33-supply = <&vreg_l10e_3p1>;
 	vdda18-supply = <&vreg_l5e_bb_1p7>;
+
+	status = "okay";
 };
 
 &usb_qmpphy {
-	status = "okay";
 	vdda-phy-supply = <&vreg_l4e_bb_0p875>;
 	vdda-pll-supply = <&vreg_l1e_bb_1p2>;
+
+	status = "okay";
 };
 
 &usb {
-- 
2.25.1

Re: [PATCH 2/2] ARM: dts: qcom: sdx55-fn980: Move "status" property to the end of node

[Konrad Dybcio]

On 31.03.2023 16:59, Manivannan Sadhasivam wrote:
> To align with rest of the devicetree files, let's move the "status"
> property to the end of the nodes.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  .../boot/dts/qcom-sdx55-telit-fn980-tlb.dts   | 20 ++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
> index b73b707342af..91d13c40496c 100644
> --- a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
> +++ b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
> @@ -243,21 +243,21 @@ &ipa {
>  };
>  
>  &pcie_phy {
> -	status = "okay";
> -
>  	vdda-phy-supply = <&vreg_l1e_bb_1p2>;
>  	vdda-pll-supply = <&vreg_l4e_bb_0p875>;
> -};
>  
> -&pcie_ep {
>  	status = "okay";
> +};
>  
> +&pcie_ep {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie_ep_clkreq_default &pcie_ep_perst_default
>  		     &pcie_ep_wake_default>;
>  
>  	reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
>  	wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
> +
> +	status = "okay";
>  };
>  
>  &qpic_bam {
> @@ -265,8 +265,6 @@ &qpic_bam {
>  };
>  
>  &qpic_nand {
> -	status = "okay";
> -
>  	nand@0 {
>  		reg = <0>;
>  
> @@ -277,11 +275,13 @@ nand@0 {
>  		secure-regions = /bits/ 64 <0x500000 0x500000
>  					    0xa00000 0xb00000>;
>  	};
> +
> +	status = "okay";
Did you compiletest this?

Konrad
>  };
>  
>  &remoteproc_mpss {
> -	status = "okay";
>  	memory-region = <&mpss_adsp_mem>;
> +	status = "okay";
>  };
>  
>  &tlmm {
> @@ -308,16 +308,18 @@ pcie_ep_wake_default: pcie-ep-wake-default-state {
>  };
>  
>  &usb_hsphy {
> -	status = "okay";
>  	vdda-pll-supply = <&vreg_l4e_bb_0p875>;
>  	vdda33-supply = <&vreg_l10e_3p1>;
>  	vdda18-supply = <&vreg_l5e_bb_1p7>;
> +
> +	status = "okay";
>  };
>  
>  &usb_qmpphy {
> -	status = "okay";
>  	vdda-phy-supply = <&vreg_l4e_bb_0p875>;
>  	vdda-pll-supply = <&vreg_l1e_bb_1p2>;
> +
> +	status = "okay";
>  };
>  
>  &usb {

Re: [PATCH 2/2] ARM: dts: qcom: sdx55-fn980: Move "status" property to the end of node

[Krzysztof Kozlowski]

On 31/03/2023 16:59, Manivannan Sadhasivam wrote:
> To align with rest of the devicetree files, let's move the "status"
> property to the end of the nodes.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  .../boot/dts/qcom-sdx55-telit-fn980-tlb.dts   | 20 ++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)

These are cosmetic changes, so we should not do it board-per-board (see
recent Alex's commit).

Either two commits (one for entire arm32 and one for arm64) or per SoCs.
But not per-boards.

Best regards,
Krzysztof

Re: [PATCH 2/2] ARM: dts: qcom: sdx55-fn980: Move "status" property to the end of node

[kernel test robot]

Hi Manivannan,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20230331]
[cannot apply to robh/for-next v6.3-rc4 v6.3-rc3 v6.3-rc2 linus/master v6.3-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam/ARM-dts-qcom-sdx55-fn980-Move-status-property-to-the-end-of-node/20230331-230033
patch link:    https://lore.kernel.org/r/20230331145915.11653-2-manivannan.sadhasivam%40linaro.org
patch subject: [PATCH 2/2] ARM: dts: qcom: sdx55-fn980: Move "status" property to the end of node
config: arm-randconfig-r046-20230329 (https://download.01.org/0day-ci/archive/20230401/202304010413.fguDBfM6-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/0ec51f1198e0249d2b1b75a883d1bffb0cdbd024
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Manivannan-Sadhasivam/ARM-dts-qcom-sdx55-fn980-Move-status-property-to-the-end-of-node/20230331-230033
        git checkout 0ec51f1198e0249d2b1b75a883d1bffb0cdbd024
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304010413.fguDBfM6-lkp@intel.com/

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts:279.2-18 Properties must precede subnodes
   FATAL ERROR: Unable to parse input tree

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 1/2] ARM: dts: qcom: sdx55: Move reset and wake gpios to board dts

[Konrad Dybcio]

On 31.03.2023 16:59, Manivannan Sadhasivam wrote:
> The reset and wake properties in the PCIe EP node belong to the board dts
> as they can be customized per board design. So let's move them from SoC
> dtsi.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

On a note, is PCIe not connected to anything on the SDX55 MTP?

Konrad
>  arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts | 3 +++
>  arch/arm/boot/dts/qcom-sdx55.dtsi                | 2 --
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
> index 81f33eba39e5..b73b707342af 100644
> --- a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
> +++ b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
> @@ -255,6 +255,9 @@ &pcie_ep {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie_ep_clkreq_default &pcie_ep_perst_default
>  		     &pcie_ep_wake_default>;
> +
> +	reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
>  };
>  
>  &qpic_bam {
> diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi b/arch/arm/boot/dts/qcom-sdx55.dtsi
> index 286fa92da428..bc310ed01b40 100644
> --- a/arch/arm/boot/dts/qcom-sdx55.dtsi
> +++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
> @@ -421,8 +421,6 @@ pcie_ep: pcie-ep@1c00000 {
>  				     <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "global",
>  					  "doorbell";
> -			reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
> -			wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
>  			resets = <&gcc GCC_PCIE_BCR>;
>  			reset-names = "core";
>  			power-domains = <&gcc PCIE_GDSC>;

Re: (subset) [PATCH 1/2] ARM: dts: qcom: sdx55: Move reset and wake gpios to board dts

[Bjorn Andersson]

On Fri, 31 Mar 2023 20:29:14 +0530, Manivannan Sadhasivam wrote:
> The reset and wake properties in the PCIe EP node belong to the board dts
> as they can be customized per board design. So let's move them from SoC
> dtsi.
> 
> 

Applied, thanks!

[1/2] ARM: dts: qcom: sdx55: Move reset and wake gpios to board dts
      commit: a6d1d862638b51f2a874948f75fd7284d066f900
[2/2] ARM: dts: qcom: sdx55-fn980: Move "status" property to the end of node
      commit: 23fbf5339d4b18858bcd38811c0eb96df00e8289

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>