Re: [PATCH v4 7/7] arm64: dts: qcom: Add Xiaomi Redmi 9T

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The `brightness-levels` property in the `backlight_dsi` node defines a single element (`<255>`), which violates the led-backlight DT schema and causes a runtime kernel warning.
- [Low] The USB SuperSpeed PHY (`usb_qmpphy`) is needlessly enabled and routed to the Type-C connector on a USB 2.0-only device, misrepresenting the board topology and wasting power.
--

--- Patch [7]: [PATCH v4 7/7] arm64: dts: qcom: Add Xiaomi Redmi 9T ---
commit b6a263c7aa2dea142dda12d7e116e55fb2d00b44
Author: Adam Skladowski <[email protected]>

arm64: dts: qcom: Add Xiaomi Redmi 9T

This commit adds a devicetree for the Xiaomi Redmi 9T smartphone based
on the Bengal/SM6115 SoC. It supports basic functionality like display,
GPU, I2C peripherals, keys, regulators, remoteprocs, UFS/SD, and USB.

> diff --git a/arch/arm64/boot/dts/qcom/sm6115-xiaomi-lemon.dts b/arch/arm64/boot/dts/qcom/sm6115-xiaomi-lemon.dts
> new file mode 100644
> index 0000000000000..881e05b70116b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm6115-xiaomi-lemon.dts
> @@ -0,0 +1,497 @@
[ ... ]
> +	backlight_dsi: backlight-dsi {
> +		compatible = "led-backlight";
> +		leds = <&led_backlight>;
> +		brightness-levels = <255>;
> +		default-brightness-level = <190>;

[Severity: Low]
Could this single-element brightness-levels array trigger a dt-schema
validation failure and a kernel warning during boot?

In the led_bl.c driver, parsing this node calls of_property_count_u32_elems()
which warns when num_levels == 1 with "Not enough levels defined\n", causing
the property to be ignored.

Additionally, the default-brightness-level index of 190 would be out of
bounds for an array with only one element.

> +		/* Backlight controllers can differ between panel variants */
> +		status = "disabled";
> +	};
[ ... ]
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				pmi632_hs_in: endpoint {
> +					remote-endpoint = <&usb_dwc3_hs>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				pmi632_ss_in: endpoint {
> +					remote-endpoint = <&usb_qmpphy_out>;
> +				};
> +			};
> +		};

[Severity: Low]
Is it intentional to route the SuperSpeed PHY endpoint to the Type-C connector
here, given that the device is USB 2.0 only?

The usb_dwc3 node restricts the maximum speed to high-speed. Leaving the
SuperSpeed PHY routed and enabled could misrepresent the hardware topology if
there are no physical SS traces.

> +	};
> +};
[ ... ]
> +&usb_qmpphy {
> +	vdda-phy-supply = <&pm6125_l4>;
> +	vdda-pll-supply = <&pm6125_l12>;
> +
> +	status = "okay";
> +};

[Severity: Low]
Should the usb_qmpphy node remain disabled for this high-speed only device?

Enabling the QMP SuperSpeed PHY wastes power initializing and bringing up an
unused PHY when the USB controller probes. 

Usually, devices in this situation also remove the usb3-phy reference from
the phys property in the usb_dwc3 node so the driver does not attempt to
initialize it at all.

> +
> +&usb_qmpphy_out {
> +	remote-endpoint = <&pmi632_ss_in>;
> +};
> +
> +&xo_board {
> +	clock-frequency = <19200000>;
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7