Re: [PATCH v2 3/4] arm64: dts: mediatek: add Genio 720-EVK board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Incorrect active-high polarity for SD card detect (CD) GPIO.
- [High] Off-by-one register size definition causes unaligned MMIO mapping.
- [Low] Missing device tree files and binding headers cause a hard build failure.
- [Low] Invalid PMIC phandles due to copy-paste error.
--

commit 8e95b232dabd4f3ff024fd10c08b98d6371a52be
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

arm64: dts: mediatek: add Genio 720-EVK board

Add support for MediaTek MT8189 SoC, its variant MT8391, and a devicetree
for the basic hardware enablement of the Genio 720-EVK board.

> diff --git a/arch/arm64/boot/dts/mediatek/mt8189.dtsi b/arch/arm64/boot/dts/mediatek/mt8189.dtsi
> new file mode 100644
> index 0000000000000..272b1b34c9537
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8189.dtsi
> @@ -0,0 +1,920 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (c) 2025 MediaTek Inc.
> + *
> + * Copyright (c) 2025 Collabora Ltd.
> + * Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> + */
> +
> +#include <dt-bindings/clock/mediatek,mt8189-clk.h>

[Severity: Low]
Are these headers present in the tree? It looks like mt8189-clk.h,
mt6365.dtsi, and mt8189-pinfunc.h might be missing from this patch
series, which would cause a build failure.

> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/power/mediatek,mt8189-power.h>
> +#include <dt-bindings/phy/phy.h>

[ ... ]

> +		pwrap: pwrap@1cc04000 {
> +			compatible = "mediatek,mt8189-pwrap", "mediatek,mt8195-pwrap", "syscon";
> +			reg = <0 0x1cc04000 0 0x1000>;
> +			reg-names = "pwrap";
> +			assigned-clocks = <&vlpckgen CLK_VLP_CK_PWRAP_ULPOSC_SEL>;
> +			assigned-clock-parents = <&topckgen CLK_TOP_OSC_D10>;
> +			clocks =  <&vlpcfg CLK_VLPCFG_REG_PMIF_SPMI_M_SYS>,
> +				  <&vlpcfg CLK_VLPCFG_REG_PMIF_SPMI_M_TMR>;
> +			clock-names = "spi", "wrap";
> +			interrupts = <GIC_SPI 496 IRQ_TYPE_LEVEL_HIGH 0>;
> +		};
> +
> +		spmi: spmi@1cc06000 {
> +			compatible = "mediatek,mt8189-spmi", "mediatek,mt8195-spmi";
> +			reg = <0 0x1cc06000 0 0x0008ff>, <0 0x1cc00000 0 0x000100>;

[Severity: High]
Does the first register size 0x8ff cause an unaligned MMIO mapping?

A size of 0x8ff (2303 bytes) isn't a multiple of 4, so the mapping will
terminate exactly before the last byte of a 32-bit register at offset 0x8fc.
Accessing that register might cause a memory fault.

Should this size be 0x900 instead?

> +			reg-names = "pmif", "spmimst";
> +			clocks = <&vlpcfg CLK_VLPCFG_REG_PMIF_SPMI_P_SYS>,
> +				 <&vlpcfg CLK_VLPCFG_REG_PMIF_SPMI_P_TMR>,
> +				 <&vlpckgen CLK_VLP_CK_SPMI_P_MST_SEL>;
> +			clock-names = "pmif_sys_ck", "pmif_tmr_ck", "spmimst_clk_mux";
> +			#address-cells = <2>;
> +			#size-cells = <0>;
> +		};

[ ... ]

> diff --git a/arch/arm64/boot/dts/mediatek/mt8391-genio-common.dtsi b/arch/arm64/boot/dts/mediatek/mt8391-genio-common.dtsi
> new file mode 100644
> index 0000000000000..fbb202e344746
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8391-genio-common.dtsi
> @@ -0,0 +1,673 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (c) 2025 Collabora Ltd.
> + * Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> + */
> +
> +#include "mt6365.dtsi"

[Severity: Low]
This is related to the previous header issue, but just checking if
mt6365.dtsi and mt8189-pinfunc.h were accidentally omitted from this
patch series?

> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/spmi/spmi.h>
> +#include "mt8189-pinfunc.h"

[ ... ]

> +&auxadc {
> +	status = "okay";
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&mt6359_vmodem_buck_reg>;
> +};

[Severity: Low]
Does this reference an undefined label?

The board configuration includes mt6365.dtsi, but this uses an mt6359
prefix. This might cause a dtc compilation error due to undefined
phandles.

> +
> +&cpu1 {
> +	cpu-supply = <&mt6359_vmodem_buck_reg>;
> +};

[ ... ]

> +&mmc1 {
> +	bus-width = <4>;
> +	cap-sd-highspeed;
> +	cd-gpios = <&pio 2 GPIO_ACTIVE_HIGH>;

[Severity: High]
Is the SD card detect polarity inverted here?

The pinctrl node mmc1_default_pins configures this pin with a
bias-pull-up, meaning the pin defaults high when empty and is pulled
low on card insertion.

Should this be configured as GPIO_ACTIVE_LOW instead to prevent the
system from constantly seeing a card when empty?

> +	max-frequency = <200000000>;
> +	no-mmc;
> +	no-sdio;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
> +	pinctrl-names = "default", "state_uhs";
> +	pinctrl-0 = <&mmc1_default_pins>;
> +	pinctrl-1 = <&mmc1_uhs_pins>;
> +	vmmc-supply = <&mt6359_vpa_buck_reg>;
> +	vqmmc-supply = <&mt6359_vsim1_ldo_reg>;
> +	status = "okay";
> +};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-add-mediatek-genio-520-720-evk-v2-0-19d5da4ef984@collabora.com?part=3