Re: [PATCH v2 5/5] arm64: dts: qcom: sm8150: Add CAMSS and CCI nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] arm64: dts: qcom: sm8150: inverted CLK/DATA comments in CCI pinctrl nodes
--

commit 338e8b9406e2ae8eb1e75907ca56f8da311da07c
Author: Andrew Gigena <mail@andrewgigena.dev>

arm64: dts: qcom: sm8150: Add CAMSS and CCI nodes

This commit adds the CAMSS and CCI controller nodes to the SM8150 SoC
devicetree, along with the corresponding TLMM pinctrl states for the CCI0
and CCI1 I2C buses.

> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index 293fcbacff346..a1fd3a4ed2e5e 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -2736,6 +2736,86 @@ wake-pins {
>  					bias-pull-up;
>  				};
>  			};
> +
> +			cci0_default: cci0-default-state {
> +				cci0_i2c0_default: cci0-i2c0-default-pins {
> +					/* CLK, DATA */
> +					pins = "gpio17", "gpio18";
> +					function = "cci_i2c";
> +
> +					bias-pull-up;
> +					drive-strength = <2>; /* 2 mA */
> +				};
> +
> +				cci0_i2c1_default: cci0-i2c1-default-pins {
> +					/* CLK, DATA */
> +					pins = "gpio19", "gpio20";
> +					function = "cci_i2c";
> +
> +					bias-pull-up;
> +					drive-strength = <2>; /* 2 mA */
> +				};
> +			};
> +
> +			cci0_sleep: cci0-sleep-state {
> +				cci0_i2c0_sleep: cci0-i2c0-sleep-pins {
> +					/* CLK, DATA */
> +					pins = "gpio17", "gpio18";
> +					function = "cci_i2c";
> +
> +					drive-strength = <2>; /* 2 mA */
> +					bias-pull-down;
> +				};
> +
> +				cci0_i2c1_sleep: cci0-i2c1-sleep-pins {
> +					/* CLK, DATA */
> +					pins = "gpio19", "gpio20";
> +					function = "cci_i2c";
> +
> +					drive-strength = <2>; /* 2 mA */
> +					bias-pull-down;
> +				};
> +			};
> +
> +			cci1_default: cci1-default-state {
> +				cci1_i2c0_default: cci1-i2c0-default-pins {
> +					/* CLK, DATA */
> +					pins = "gpio31", "gpio32";
> +					function = "cci_i2c";
> +
> +					bias-pull-up;
> +					drive-strength = <2>; /* 2 mA */
> +				};
> +
> +				cci1_i2c1_default: cci1-i2c1-default-pins {
> +					/* CLK, DATA */
> +					pins = "gpio33", "gpio34";
> +					function = "cci_i2c";
> +
> +					bias-pull-up;
> +					drive-strength = <2>; /* 2 mA */
> +				};
> +			};
> +
> +			cci1_sleep: cci1-sleep-state {
> +				cci1_i2c0_sleep: cci1-i2c0-sleep-pins {
> +					/* CLK, DATA */
> +					pins = "gpio31", "gpio32";
> +					function = "cci_i2c";
> +
> +					bias-pull-down;
> +					drive-strength = <2>; /* 2 mA */
> +				};
> +
> +				cci1_i2c1_sleep: cci1-i2c1-sleep-pins {
> +					/* CLK, DATA */
> +					pins = "gpio33", "gpio34";
> +					function = "cci_i2c";
> +
> +					bias-pull-down;
> +					drive-strength = <2>; /* 2 mA */
> +				};
> +			};

[Severity: Low]
Are the /* CLK, DATA */ comments inverted across the cci0_default,
cci0_sleep, cci1_default, and cci1_sleep pinctrl subnodes?

Looking at the TLMM GPIO line names in
arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi:

    &tlmm {
        gpio-line-names =
            ...
            "CCI_I2C_SDA0",
            "CCI_I2C_SCL0",
            "CCI_I2C_SDA1",
            "CCI_I2C_SCL1", /* GPIO_20 */
            ...
            "CCI_I2C_SDA2",
            "CCI_I2C_SCL2",
            "CCI_I2C_SDA3",
            "CCI_I2C_SCL3",
            ...

gpio17, gpio19, gpio31, and gpio33 are the SDA (DATA) lines, while gpio18,
gpio20, gpio32, and gpio34 are the SCL (CLK) lines. Should these comments
say /* SDA, SCL */ or /* DATA, CLK */ instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-sm8150-v7.4-rc1-camss-v2-0-39ee37787b00@andrewgigena.dev?part=5