Re: [PATCH v2 3/7] arm64: dts: qcom: Add Motorola One

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Touchscreen `reset-gpios` logic is inverted, which will hold the controller in reset during operation.
- [Medium] The touchscreen node is disabled, contradicting the commit message claim of supported functionality.
- [Low] The `usb-role-switch` property in the `fcs,fusb302` node is a DT schema violation and misdirected.
--

--- Patch [3]: [PATCH v2 3/7] arm64: dts: qcom: Add Motorola One ---
commit a01f938ee143a61f1aad41106fd89f197d7a249a
Author: Adam Skladowski <a39.skl@gmail.com>

arm64: dts: qcom: Add Motorola One

This commit adds a devicetree for the Motorola One smartphone based on the
Jacala/MSM8953 SoC. It introduces support for basic functionality including
I2C peripherals, physical buttons, regulators, remoteprocs, and USB.

> diff --git a/arch/arm64/boot/dts/qcom/msm8953-motorola-deen.dts b/arch/arm64/boot/dts/qcom/msm8953-motorola-deen.dts
> new file mode 100644
> index 0000000000000..acb0b61b8f947
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8953-motorola-deen.dts

[ ... ]

> +&i2c_1 {
> +	status = "okay";
> +
> +	typec-portc@22 {
> +		compatible = "fcs,fusb302";
> +		reg = <0x22>;
> +		interrupts-extended = <&tlmm 9 IRQ_TYPE_LEVEL_LOW>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&fusb302_int_default>;
> +
> +		usb-role-switch = <&usb3>;

[Severity: Low]
Does this property violate the fcs,fusb302 device tree schema? The binding
strictly disallows additional properties like usb-role-switch.

Also, should this point to the DWC3 core node (&usb3_dwc3) instead of the
wrapper node (&usb3) where the role switch is actually registered?

> +		vbus-supply = <&fusb302_vbus>;

[ ... ]

> +&i2c_3 {
> +	status = "okay";
> +
> +	touchscreen@62 {
> +		compatible = "novatek,nt36672a-ts";
> +		reg = <0x62>;
> +		interrupts-extended = <&tlmm 65 IRQ_TYPE_EDGE_RISING>;
> +
> +		reset-gpios = <&tlmm 64 GPIO_ACTIVE_HIGH>;

[Severity: High]
Will this hold the touchscreen controller in reset during operation?

During driver probe, the reset GPIO is requested with GPIOD_OUT_LOW to bring
the controller out of reset. If the hardware reset line is active-low,
defining it as GPIO_ACTIVE_HIGH here will cause the driver to incorrectly
drive the pin physically low and hold the controller in reset.

> +		iovcc-supply = <&pm8953_l6>;
> +		/* vcc-supply should follow lab from panel */
> +
> +		pinctrl-0 = <&ts_reset_default &ts_int_default>;
> +		pinctrl-1 = <&ts_reset_sleep &ts_int_sleep>;
> +		pinctrl-names = "default", "sleep";
> +
> +		touchscreen-size-x = <720>;
> +		touchscreen-size-y = <1520>;
> +
> +		status = "disabled";

[Severity: Medium]
The commit message explicitly lists the touchscreen as supported functionality
in this initial submission, but it is disabled here.

Should the commit message be updated to reflect that it remains disabled
due to missing dependencies?

> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-next-patches-v2-0-7130d68183d9@gmail.com?part=3