Re: [PATCH 11/11] arm64: dts: qcom: Add Motorola Signature (vantage)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential
issues to consider:
- [Medium] arm64: dts: qcom: vantage open-codes B_E0 PMH0101 as PM8550
  on spmi_bus0
- [Low] arm64: dts: qcom: vantage defines serial0 alias for disabled
  uart7 node
- [Low] arm64: dts: qcom: vantage places firmware memory map comment on
  gpio-keys
- [Low] arm64: dts: qcom: vantage static reserved-memory nodes set
  alignment
- [Low] arm64: dts: qcom: vantage nodes and label overrides are out of
  sort order
--

commit ec19559277a19246b45ee197f04e140dbe0920a8
Author: Venkat Yadlapati <venkat.yadlapati@yahoo.com>

arm64: dts: qcom: Add Motorola Signature (vantage)

This commit adds the devicetree for the Motorola Signature (vantage)
smartphone based on the Qualcomm Molokai (SM8845) SoC.

> diff --git a/arch/arm64/boot/dts/qcom/molokai-motorola-vantage.dts b/arch/arm64/boot/dts/qcom/molokai-motorola-vantage.dts
> new file mode 100644
> index 0000000000000..c3a0fa5f4e0e8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/molokai-motorola-vantage.dts
> @@ -0,0 +1,737 @@

[ ... ]

> +/ {
> +	model = "Motorola Signature";
> +	compatible = "motorola,vantage", "qcom,molokai";
> +	chassis-type = "handset";
> +
> +	aliases {
> +		serial0 = &uart7;
> +	};

[Severity: Low]
Should &uart7 be enabled, or should the serial0 alias be dropped?  uart7
has status = "disabled" in kaanapali.dtsi and is never enabled in
molokai.dtsi or this board file.

[ ... ]

> +	clocks {
> +		xo_board: xo-board {
> +			compatible = "fixed-clock";
> +			clock-frequency = <76800000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		sleep_clk: sleep-clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <32764>;
> +			#clock-cells = <0>;
> +		};
> +
> +		bi_tcxo_div2: bi-tcxo-div2-clk {
> +			compatible = "fixed-factor-clock";
> +			#clock-cells = <0>;
> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
> +			clock-mult = <1>;
> +			clock-div = <2>;
> +		};
> +
> +		bi_tcxo_ao_div2: bi-tcxo-ao-div2-clk {
> +			compatible = "fixed-factor-clock";
> +			#clock-cells = <0>;
> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK_A>;
> +			clock-mult = <1>;
> +			clock-div = <2>;
> +		};
> +	};

[Severity: Low]
This isn't a bug, but the unaddressed child nodes under clocks are not in
alphabetical order (bi-tcxo-ao-div2-clk, bi-tcxo-div2-clk, sleep-clk,
xo-board), which triggers warnings from scripts/dtc/dt-check-style.
/delete-node/ &adspslpi_mem at the top of the file and the &label
overrides at the bottom are also out of alphabetical order.

> +
> +	/* Memory map of the SM8845 firmware shipped with A171WVH.21-16-16 */
> +	gpio-keys {
> +		compatible = "gpio-keys";

[Severity: Low]
This isn't a bug, but this comment describes the reserved-memory node
further down rather than gpio-keys.

[ ... ]

> +	reserved-memory {

[ ... ]

> +		qdss_apps_mem: qdss-apps@82800000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x0 0x82800000 0x0 0x2000000>;
> +			alignment = <0x0 0x400000>;
> +			reusable;
> +		};

[Severity: Low]
Should alignment be dropped from qdss-apps@82800000, oem-vm@f7c00000, and
lcp-dare@880000000?

Static reserved-memory nodes that specify reg without size ignore
alignment in __reserved_mem_reserve_reg(), and the reserved-memory schema
requires size whenever alignment is present, so make dtbs_check fails on
these three nodes.

[ ... ]

> +&apps_rsc {
> +	regulators-0 {
> +		compatible = "qcom,pmh0101-rpmh-regulators";
> +		qcom,pmic-id = "B_E0";

[ ... ]

> +&spmi_bus0 {
> +	pmic@1 {
> +		compatible = "qcom,pm8550", "qcom,spmi-pmic";
> +		reg = <0x1 SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		pm8550_gpios: gpio@8800 {
> +			compatible = "qcom,pm8550-gpio", "qcom,spmi-gpio";
> +			reg = <0x8800>;
> +			gpio-controller;
> +			gpio-ranges = <&pm8550_gpios 0 0 12>;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +
> +			key_vol_up_default: key-vol-up-default-state {
> +				pins = "gpio6";
> +				function = "normal";
> +				power-source = <1>;
> +				bias-pull-up;
> +				input-enable;
> +			};
> +		};
> +	};

[Severity: Medium]
Should this include pmh0101.dtsi and reference &pmh0101_gpios instead of
open-coding pmic@1 as a PM8550?

regulators-0 under &apps_rsc declares PMIC B_E0 (SPMI bus 0, USID 1) as
qcom,pmh0101-rpmh-regulators, whereas pmic@1 on &spmi_bus0 describes the
same USID 1 device as qcom,pm8550 with qcom,pm8550-gpio.  As a result,
pinctrl-spmi-gpio registers 12 GPIOs instead of PMH0101's 18 GPIOs, and
the PMH0101 thermal sensor and peripherals from pmh0101.dtsi are omitted.
The commit message also refers to PM8550 and PMK8550 even though the board
uses PMH0101 and includes pmk8850.dtsi.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261116-molokai-soc-v1-0-3b2ee920fa3b@yahoo.com?part=11