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

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Contradiction in Supported Features regarding backlight.
- [Low] Unnecessary Hardware Enablement of SPI2 bus.
- [Low] Invalid `brightness-levels` array in `backlight-dsi` causes boot warning.
- [Low] The dtb targets in `arch/arm64/boot/dts/qcom/Makefile` are not sorted alphabetically.
--

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

arm64: dts: qcom: Add Xiaomi Redmi 9T

This commit adds the devicetree for the Xiaomi Redmi 9T smartphone, based on
the Bengal/SM6115 SoC. It defines memory regions, regulators, and enables
various peripherals like SimpleFB and remoteprocs.

> Supported functionality as of this initial submission:
> 
> * Display via SimpleFB/SimpleDRM
> * GPU
> * I2C peripherals (Backlight)

[Severity: Medium]
Is the commit message accurate about backlight support? The device tree marks
both the backlight-dsi and lm36923_bl nodes with status = "disabled", which
would prevent the driver from probing.

[ ... ]
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 821101c6d338e..63d678ab215e4 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -340,6 +340,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm4250-oneplus-billie2.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm4450-qrd.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm6115-fxtec-pro1x.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm6115p-lenovo-j606f.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= sm6115-xiaomi-lemon.dtb

[Severity: Low]
Does this break alphabetical ordering in the Makefile? The hyphen character
comes before 'p' in ASCII, so sm6115-xiaomi-lemon.dtb should probably be
placed before sm6115p-lenovo-j606f.dtb to prevent future merge conflicts.

>  dtb-$(CONFIG_ARCH_QCOM)	+= sm6125-sony-xperia-seine-pdx201.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm6125-xiaomi-ginkgo.dtb
[ ... ]
> 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
[ ... ]
> +	backlight_dsi: backlight-dsi {
> +		compatible = "led-backlight";
> +		leds = <&led_backlight>;
> +		brightness-levels = <255>;

[Severity: Low]
Does this single-element brightness-levels array trigger a boot warning? In
led_bl_parse_levels(), the driver checks if there is more than 1 level and
emits a "Not enough levels defined" warning if only one is present.

> +		default-brightness-level = <190>;
> +		/* Backlight controllers can differ between panel variants */
> +		status = "disabled";
> +	};
[ ... ]
> +&spi2 {
> +	status = "okay";
> +};

[Severity: Low]
Is it necessary to enable the spi2 bus here? It appears to be enabled
without any child nodes or pinctrl configurations, which might hold
unnecessary power and clock resources.

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