Re: [PATCH 4/6] arm64: dts: qcom: Add sm6115-motorola-common

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] arm64: dts: qcom: undefined vreg_l9a phandle in framebuffer0
  vddio-supply
- [High] arm64: dts: qcom: sm6115-motorola-common: unbound pmi632_lcdb
  panel supplies
- [Medium] arm64: dts: qcom: missing pinctrl-0 on uart3 breaks
  pinctrl_dt_to_map()
- [Low] arm64: dts: qcom: framebuffer@5c000000 has unit address without
  reg property
--

commit 2b899e84d3b2843d96896aa9d295e59ce527c5e2
Author: Val Packett <val@packett.cool>

arm64: dts: qcom: Add sm6115-motorola-common

This commit adds the shared devicetree include for SM6115/SM4250-based
Motorola smartphones, configuring common regulators, storage, connectivity,
buttons, USB Type-C, and display nodes.

> diff --git a/arch/arm64/boot/dts/qcom/sm6115-motorola-common.dtsi b/arch/arm64/boot/dts/qcom/sm6115-motorola-common.dtsi
> new file mode 100644
> index 0000000000000..6a5110319bc3d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm6115-motorola-common.dtsi
> @@ -0,0 +1,541 @@

[ ... ]

> +	chosen {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		framebuffer0: framebuffer@5c000000 {
> +			compatible = "simple-framebuffer";
> +			memory-region = <&cont_splash_memory>;

[Severity: Low]
Does framebuffer@5c000000 trigger a dtc unit_address_vs_reg warning?  The
node has a unit address in its name, but uses memory-region without a reg
property.  Dropping @5c000000 (along with #address-cells, #size-cells, and
ranges in /chosen) or providing a reg property avoids the warning.

> +			width = <720>;
> +			height = <1600>;
> +			stride = <(720 * 4)>;
> +			format = "a8r8g8b8";
> +
> +			clocks = <&gcc GCC_DISP_AHB_CLK>,
> +				 <&gcc GCC_DISP_HF_AXI_CLK>;
> +			vddio-supply = <&vreg_l9a>;
> +		};
> +	};

[Severity: High]
Should vddio-supply reference &pm6125_l9a instead of &vreg_l9a?  No
vreg_l9a label exists in this file or the included dtsi files (LDO9 is
defined below as pm6125_l9a: l9), so dtc fails to build any board DTB that
includes sm6115-motorola-common.dtsi.

[ ... ]

> +&mdss {
> +	status = "okay";
> +};
> +
> +&mdss_dsi0 {
> +	vdda-supply = <&pm6125_l18a>;
> +
> +	pinctrl-0 = <&mdss_dsi_active &mdss_te_active_sleep>;
> +	pinctrl-1 = <&mdss_dsi_sleep &mdss_te_active_sleep>;
> +	pinctrl-names = "default", "sleep";
> +
> +	status = "okay";
> +
> +	panel: panel@0 {
> +		reg = <0>;
> +
> +		avdd-supply = <&pmi632_lcdb_ldo>;
> +		avee-supply = <&pmi632_lcdb_ncp>;
> +
> +		status = "disabled";

[Severity: High]
Will wiring avdd-supply and avee-supply to &pmi632_lcdb_ldo and
&pmi632_lcdb_ncp cause panel@0 to defer probe forever?

pmi632_lcdb (added in commit "arm64: dts: qcom: pmi632: Add LCDB regulator
node" with compatible "qcom,pmi632-lcdb", "qcom,pm8150l-lcdb") has no
driver in the kernel tree.  When a board enables &panel and &pmi632_lcdb
(as in commit "arm64: dts: qcom: sm6115: Add Motorola Moto G9 Play
(guamp)"), both fw_devlink and devm_regulator_get() return -EPROBE_DEFER
permanently because the supplier node exists but never registers a
regulator_dev.  Meanwhile, mdss_probe() -> msm_mdss_init() ->
msm_mdss_reset() tears down the bootloader simple-framebuffer splash, and
because the DSI panel never attaches, the msm_drm component master never
binds either the display or the Adreno GPU component.

Additionally, enabling &mdss, &mdss_dsi0, and &mdss_dsi0_phy unconditionally
in this common dtsi breaks boards that disable &dispcc to keep
simple-framebuffer working (such as commit "arm64: dts: qcom: sm6115: Add
Motorola Moto G9 Power (cebu)"), since &mdss cannot attach its disabled
&dispcc MDSS_GDSC power domain and prevents the Adreno GPU from binding.

[ ... ]

> +&uart3 {
> +	status = "okay";
> +
> +	interrupts-extended = <&intc GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>,
> +				<&tlmm 11 IRQ_TYPE_EDGE_FALLING>;
> +
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-1 = <&qup_uart3_sleep>;

[Severity: Medium]
Does &uart3 fail to bind its pin states because pinctrl-0 is missing?

Neither this node nor the base uart3 node in sm6115.dtsi defines pinctrl-0.
When uart3 probes, pinctrl_dt_to_map() checks state 0 first and aborts with
-ENODEV when pinctrl-0 is absent, so neither the active qup3 pinmux nor
qup_uart3_sleep is applied.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109221453.658316-1-val@packett.cool?part=4