Re: [PATCH v2 3/3] arm64: dts: qcom: add initial Motorola Moto E4 (perry) support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Critical pins for the touchscreen interrupt, sensor I2C bus, sensor interrupt, and regulator enable lack pinctrl configurations.
- [Medium] Battery design capacity is incorrectly specified for the hardware.
--

--- Patch [3]: [PATCH v2 3/3] arm64: dts: qcom: add initial Motorola Moto E4 (perry) support ---
commit 456da8a21ca3d3b4649cf6cd9b340f36af9edbf2
Author: Catherine Frederick (coolguy) <serenity@floorchan.org>

arm64: dts: qcom: add initial Motorola Moto E4 (perry) support
    
Adds MSM8917 and MSM8920 board files for the Motorola Moto E4 (perry) sharing a
common dtsi. Includes regulators, eMMC/SD, USB OTG, gpio-keys, touchscreen,
IMU, Wi-Fi, and WLED.

> diff --git a/arch/arm64/boot/dts/qcom/msm8917-motorola-perry-common.dtsi b/arch/arm64/boot/dts/qcom/msm8917-motorola-perry-common.dtsi
> new file mode 100644
> index 0000000000000..1870958976fbe
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8917-motorola-perry-common.dtsi
> @@ -0,0 +1,412 @@
> +// SPDX-License-Identifier: GPL-2.0-only
[ ... ]
> +	battery: battery {
> +		compatible = "simple-battery";
> +
> +		charge-full-design-microamp-hours = <4000000>;

[Severity: Medium]
Does the Moto E4 have a 4000 mAh battery? The design capacity here specifies
4000 mAh, but this device usually ships with a 2800 mAh battery. 

Could this incorrect capacity confuse the fuel gauge algorithms and lead to
inaccurate state-of-charge calculations?

> +		constant-charge-current-max-microamp = <1000000>;
> +		voltage-min-design-microvolt = <3400000>;
> +		voltage-max-design-microvolt = <4400000>;
> +	};
[ ... ]
> +	i2c-sensors {
> +		compatible = "i2c-gpio";
> +		sda-gpios = <&tlmm 14 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +		scl-gpios = <&tlmm 15 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;

[Severity: Medium]
Are we missing pinctrl configurations for the I2C bitbanging pins (gpio14,
gpio15) and the IMU interrupt pin (gpio41) below? 

Relying on bootloader state for these pins could lead to initialization
failures if they are left floating or improperly muxed.

> +		i2c-gpio,delay-us = <2>; /* ~100 kHz */
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		imu@18 {
> +			compatible = "bosch,bma253";
> +			reg = <0x18>;
> +
> +			interrupts-extended = <&tlmm 41 IRQ_TYPE_EDGE_RISING>;
[ ... ]
> +	ts_default: ts-default-state {
> +		pins = "gpio64";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};

[Severity: Medium]
Should the touchscreen interrupt pin (gpio65) be configured in this state as
well?

The touchscreen node explicitly uses it:

&blsp1_i2c3 {
	touchscreen@20 {
		interrupts = <65 IRQ_TYPE_EDGE_FALLING>;
		pinctrl-0 = <&ts_default>;
		reset-gpios = <&tlmm 64 GPIO_ACTIVE_LOW>;
	}
}

Missing pinctrl on this falling-edge interrupt could result in an interrupt
storm if the bootloader doesn't set the correct pin bias.

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723014627.63310-1-aneeshpradhan@acm.org?part=3