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

[Viken Dadhaniya]

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@xxxxxxxxxxx>
> > ---
> > v2 -> v3:
> >
> > - Remove duplicate pin configurations from target-specific file.
> >
> > v2 Link: https://lore.kernel.org/lkml/20250324151047.842648-1-quic_vdadhani@xxxxxxxxxxx/
> >
> > 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@xxxxxxxxxxx/
> > ---
> > ---
> > ---
> >   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;
> >   	};
> >