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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing `vdd_l23-supply` parent supply linkage for the PM8953 L23 regulator.
- [High] Missing `vcc-supply` for the Novatek touchscreen.
- [High] The `fcs,fusb302` device specifies an incorrect interrupt polarity (`IRQ_TYPE_EDGE_RISING`) for its INT_N pin.
- [Medium] Mismatch between the configured Audio Front End (AFE) DAI, physical GPIO pin functions, and unused pinctrl nodes for the MI2S interface.
- [Low] Misleading node name `pn533@28` for an NXP NCI compatible device.
--

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

arm64: dts: qcom: Add Motorola One

Add a devicetree for the Motorola One smartphone, which is based
on the Jacala/MSM8953 SoC.

> 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
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8953-motorola-deen.dts

[ ... ]

> +&i2c_1 {
> +	status = "disabled";
> +
> +	typec-portc@22 {
> +		compatible = "fcs,fusb302";
> +		reg = <0x22>;
> +		interrupts-extended = <&tlmm 9 IRQ_TYPE_EDGE_RISING>;

[Severity: High]
The FUSB302 INT_N pin is an active-low open-drain output, but this interrupt
is configured as IRQ_TYPE_EDGE_RISING.

Will the host miss interrupts since the FUSB302 holds the line low until
cleared, causing a deadlock while the host waits for a rising edge?

Does this regression break Type-C functionality like PD negotiation and
role swapping?

[ ... ]

> +&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_LOW>;
> +
> +		iovcc-supply = <&pm8953_l6>;

[Severity: High]
Does the novatek,nt36672a-ts driver also require a vcc-supply to be
specified here?

Omitting it means devm_regulator_bulk_get() might fall back to a dummy
regulator, preventing the physical analog power rail from being enabled.
Does this cause a regression where touchscreen initialization fails?

> +
> +		pinctrl-0 = <&ts_reset_default &ts_int_default>;

[ ... ]

> +&i2c_5 {
> +	status = "okay";
> +
> +	npc100: pn533@28 {
> +		compatible = "nxp,nxp-nci-i2c";

[Severity: Low]
Since the compatible string specifies a generic nxp,nxp-nci-i2c controller,
should this node be named with a generic device class like nfc@28 instead of
a specific chip name like pn533@28 to avoid a naming regression?

> +
> +		reg = <0x28>;

[ ... ]

> +&q6afedai {
> +	dai@22 {
> +		reg = <QUATERNARY_MI2S_RX>;
> +		qcom,sd-lines = <0>;
> +	};

[Severity: Medium]
Is the Audio Front End configured correctly here?

This q6afedai node specifies QUATERNARY_MI2S_RX, but the mi2s_sec_act pinctrl
states added below configure pins 135-138 with the sec_mi2s function.

Could this interface mismatch cause an audio regression on this port?

[ ... ]

> +&rpm_requests {
> +	regulators {
> +		compatible = "qcom,rpm-pm8953-regulators";
> +		vdd_s1-supply = <&vph_pwr>;
[ ... ]
> +		vdd_l8_l11_l12_l13_l14_l15-supply = <&vph_pwr>;
> +		vdd_l9_l10_l17_l18_l22-supply = <&vph_pwr>;
> +
> +		pm8953_s1: s1 {

[Severity: High]
Is there a missing vdd_l23-supply linkage in this parent supply mapping block?

Without mapping vdd_l23-supply to pm8953_s3, the regulator framework is
unaware of the dependency. If the system disables L1, L2, and L3, the PMIC
core might power down S3, starving L23 of its input voltage.

Does this regression lead to a hardware brownout?

[ ... ]

> +		pm8953_l22: l22 {
> +			regulator-always-on;
> +			regulator-min-microvolt = <2800000>;
> +			regulator-max-microvolt = <2850000>;
> +		};
> +
> +		pm8953_l23: l23 {
> +			regulator-min-microvolt = <975000>;
> +			regulator-max-microvolt = <1225000>;
> +		};

[ ... ]

> +&tlmm {
> +	gpio-reserved-ranges = <96 4>, <111 1>, <126 1>;
[ ... ]
> +	mi2s_sec_act: mi2s-quat-active-state {
> +		pins = "gpio135", "gpio136", "gpio137", "gpio138";
> +		function = "sec_mi2s";
> +		drive-strength = <8>;
> +	};

[Severity: Medium]
These secondary MI2S pinctrl states appear to be unreferenced dead code.
Should they be tied to the DAI node above to prevent a configuration
regression?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-next-patches-v1-0-79a779d76173@gmail.com?part=3