[v1] arm64: dts: qcom: ipq5424: fix and relocate uart1 gpio configurations

[PATCH v1] arm64: dts: qcom: ipq5424: fix and relocate uart1 gpio configurations

Posted by Manikanta Mylavarapu 9 months, 1 week ago

Update the bias configuration for UART1 TX and RX pins to ensure correct
settings for RDP466.

Additionally, move the UART1 GPIO configurations from the common .dtsi
file to the RDP-specific .dts files to account for differing bias
configurations across RDPs of IPQ5424.

Fixes: 1a91d2a6021e ("arm64: dts: qcom: add IPQ5424 SoC and rdp466 board support")
Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 16 +++++++++++++++-
 arch/arm64/boot/dts/qcom/ipq5424.dtsi       |  7 -------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
index b6e4bb3328b3..7b85aaa11ee8 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
@@ -147,10 +147,24 @@ data-pins {
 			bias-pull-up;
 		};
 	};
+
+	uart1_tx_state: uart1-tx-state {
+		pins = "gpio44";
+		function = "uart1";
+		drive-strength = <8>;
+		bias-pull-down;
+	};
+
+	uart1_rx_state: uart1-rx-state {
+		pins = "gpio43";
+		function = "uart1";
+		drive-strength = <8>;
+		bias-pull-up;
+	};
 };
 
 &uart1 {
-	pinctrl-0 = <&uart1_pins>;
+	pinctrl-0 = <&uart1_tx_state>, <&uart1_rx_state>;
 	pinctrl-names = "default";
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/ipq5424.dtsi b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
index 7a7ad700a382..e190c43c1b11 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
@@ -262,13 +262,6 @@ tlmm: pinctrl@1000000 {
 			gpio-ranges = <&tlmm 0 0 50>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
-
-			uart1_pins: uart1-state {
-				pins = "gpio43", "gpio44";
-				function = "uart1";
-				drive-strength = <8>;
-				bias-pull-up;
-			};
 		};
 
 		gcc: clock-controller@1800000 {

base-commit: 0a2f889128969dab41861b6e40111aa03dc57014
-- 
2.34.1

Re: [PATCH v1] arm64: dts: qcom: ipq5424: fix and relocate uart1 gpio configurations

Posted by Kathiravan Thirumoorthy 9 months, 1 week ago

On 3/12/2025 2:44 PM, Manikanta Mylavarapu wrote:
> Update the bias configuration for UART1 TX and RX pins to ensure correct
> settings for RDP466.
>
> Additionally, move the UART1 GPIO configurations from the common .dtsi
> file to the RDP-specific .dts files to account for differing bias
> configurations across RDPs of IPQ5424.
>
> Fixes: 1a91d2a6021e ("arm64: dts: qcom: add IPQ5424 SoC and rdp466 board support")
> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
> ---
>   arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 16 +++++++++++++++-
>   arch/arm64/boot/dts/qcom/ipq5424.dtsi       |  7 -------
>   2 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
> index b6e4bb3328b3..7b85aaa11ee8 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
> @@ -147,10 +147,24 @@ data-pins {
>   			bias-pull-up;
>   		};
>   	};
> +
> +	uart1_tx_state: uart1-tx-state {
> +		pins = "gpio44";
> +		function = "uart1";
> +		drive-strength = <8>;
> +		bias-pull-down;
> +	};
> +
> +	uart1_rx_state: uart1-rx-state {
> +		pins = "gpio43";
> +		function = "uart1";
> +		drive-strength = <8>;
> +		bias-pull-up;
> +	};
>   };
>   
>   &uart1 {
> -	pinctrl-0 = <&uart1_pins>;
> +	pinctrl-0 = <&uart1_tx_state>, <&uart1_rx_state>;


Change LGTM. But can we define both TX and RX pin definition under one 
node like below, to align with the SPI and SDCC gpio definition layout?

qup_uart1_default_state: qup-uart1-default-state {

         uart1_tx_state: uart1-tx-state {

          ...

         };

         uart1_rx_state: uart1-rx-state {

          ...

         };

};

and refer it like pinctrl-0 = <&qup_uart1_default_state>;


>   	pinctrl-names = "default";
>   	status = "okay";
>   };
> diff --git a/arch/arm64/boot/dts/qcom/ipq5424.dtsi b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
> index 7a7ad700a382..e190c43c1b11 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5424.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
> @@ -262,13 +262,6 @@ tlmm: pinctrl@1000000 {
>   			gpio-ranges = <&tlmm 0 0 50>;
>   			interrupt-controller;
>   			#interrupt-cells = <2>;
> -
> -			uart1_pins: uart1-state {
> -				pins = "gpio43", "gpio44";
> -				function = "uart1";
> -				drive-strength = <8>;
> -				bias-pull-up;
> -			};
>   		};
>   
>   		gcc: clock-controller@1800000 {
>
> base-commit: 0a2f889128969dab41861b6e40111aa03dc57014

Re: [PATCH v1] arm64: dts: qcom: ipq5424: fix and relocate uart1 gpio configurations

Posted by Manikanta Mylavarapu 9 months ago


On 3/12/2025 10:10 PM, Kathiravan Thirumoorthy wrote:
> On 3/12/2025 2:44 PM, Manikanta Mylavarapu wrote:
>> Update the bias configuration for UART1 TX and RX pins to ensure correct
>> settings for RDP466.
>>
>> Additionally, move the UART1 GPIO configurations from the common .dtsi
>> file to the RDP-specific .dts files to account for differing bias
>> configurations across RDPs of IPQ5424.
>>
>> Fixes: 1a91d2a6021e ("arm64: dts: qcom: add IPQ5424 SoC and rdp466 board support")
>> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 16 +++++++++++++++-
>>   arch/arm64/boot/dts/qcom/ipq5424.dtsi       |  7 -------
>>   2 files changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
>> index b6e4bb3328b3..7b85aaa11ee8 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
>> @@ -147,10 +147,24 @@ data-pins {
>>               bias-pull-up;
>>           };
>>       };
>> +
>> +    uart1_tx_state: uart1-tx-state {
>> +        pins = "gpio44";
>> +        function = "uart1";
>> +        drive-strength = <8>;
>> +        bias-pull-down;
>> +    };
>> +
>> +    uart1_rx_state: uart1-rx-state {
>> +        pins = "gpio43";
>> +        function = "uart1";
>> +        drive-strength = <8>;
>> +        bias-pull-up;
>> +    };
>>   };
>>     &uart1 {
>> -    pinctrl-0 = <&uart1_pins>;
>> +    pinctrl-0 = <&uart1_tx_state>, <&uart1_rx_state>;
> 
> 
> Change LGTM. But can we define both TX and RX pin definition under one node like below, to align with the SPI and SDCC gpio definition layout?
> 
> qup_uart1_default_state: qup-uart1-default-state {
> 
>         uart1_tx_state: uart1-tx-state {
> 
>          ...
> 
>         };
> 
>         uart1_rx_state: uart1-rx-state {
> 
>          ...
> 
>         };
> 
> };
> 
> and refer it like pinctrl-0 = <&qup_uart1_default_state>;
> 
> 

Hi Kathir,

	Thank you for reviewing the patch.
	I will incorporate the suggested change in the next version.

Thanks & Regards,
Manikanta.