Re: [PATCH v3] arm64: dts: qcom: sa8775p: Add default pin configurations for QUP SEs

[Viken Dadhaniya <quic_vdadhani@quicinc.com>]

On 5/3/2025 11:11 AM, Dmitry Baryshkov wrote:
> On Tue, Apr 29, 2025 at 11:03:34PM +0530, Viken Dadhaniya wrote:
>> Default pinctrl configurations for all QUP (Qualcomm Universal Peripheral)
>> Serial Engines (SEs) are missing in the SoC device tree. These
>> configurations are required by client teams when enabling any SEs as I2C,
>> SPI, or Serial protocols.
>>
>> Add default pin configurations for Serial Engines (SEs) for all supported
>> protocols, including I2C, SPI, and UART, to the sa8775p device tree.  This
>> change facilitates slave device driver clients to enable usecase with
>> minimal modifications.
>>
>> Remove duplicate pin configurations from target-specific file as same pin
>> configuration is included in the SoC device tree.
>>
>> Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com>
>> ---
>> v2 -> v3:
>>
>> - Remove duplicate pin configurations from target-specific file.
>>
>> v2 Link: https://lore.kernel.org/lkml/20250324151047.842648-1-quic_vdadhani@quicinc.com/
>>
>> v1 -> v2:
>>
>> - Drop drive-strength and bias property from soc dtsi.
>> - Update commit log.
>>
>> v1 Link: https://lore.kernel.org/lkml/20250225154136.3052757-1-quic_vdadhani@quicinc.com/
>> ---
>> ---
>> ---
>>   arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi |  35 -
>>   arch/arm64/boot/dts/qcom/sa8775p.dtsi      | 750 +++++++++++++++++++++
>>   2 files changed, 750 insertions(+), 35 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
>> index 967913169539..3b4243ef37e7 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
>> @@ -508,15 +508,11 @@ queue3 {
>>   
>>   &i2c11 {
>>   	clock-frequency = <400000>;
>> -	pinctrl-0 = <&qup_i2c11_default>;
>> -	pinctrl-names = "default";
>>   	status = "okay";
>>   };
>>   
>>   &i2c18 {
>>   	clock-frequency = <400000>;
>> -	pinctrl-0 = <&qup_i2c18_default>;
>> -	pinctrl-names = "default";
>>   	status = "okay";
>>   };
>>   
>> @@ -678,8 +674,6 @@ &sleep_clk {
>>   };
>>   
>>   &spi16 {
>> -	pinctrl-0 = <&qup_spi16_default>;
>> -	pinctrl-names = "default";
>>   	status = "okay";
>>   };
>>   
>> @@ -712,80 +706,53 @@ ethernet0_mdio: ethernet0-mdio-pins {
>>   		};
>>   	};
>>   
>> -	qup_uart10_default: qup-uart10-state {
>> -		pins = "gpio46", "gpio47";
>> -		function = "qup1_se3";
>> -	};
>> -
>>   	qup_spi16_default: qup-spi16-state {
>> -		pins = "gpio86", "gpio87", "gpio88", "gpio89";
>> -		function = "qup2_se2";
>>   		drive-strength = <6>;
>>   		bias-disable;
>>   	};
> 
> If you are movign parts of these pinstates to the sa8775p.dtsi, then you
> should also turn these declarations into label-based updates:
> 
> &qup_spi16_default {
> 	drive-strength = <6>;
> 	bias-disable;
> };
> 
> Otherwise the configuration is fragile: if the name gets changed, then
> two parts will not match.
> 

Sure, Updated in v4.

>>   
>>   	qup_i2c11_default: qup-i2c11-state {
>> -		pins = "gpio48", "gpio49";
>> -		function = "qup1_se4";
>>   		drive-strength = <2>;
>>   		bias-pull-up;
>>   	};
>>   
>>
>