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

[Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>]

Hi David,

On Thu, 2026-03-12 at 19:26 -0500, David Lechner wrote:
> On 12/3/25 7:59 AM, Louis-Alexis Eyraud wrote:
> > Add support for MediaTek MT8189 SoC and its variants, and a device-
> > tree
> > for the basic hardware enablement of the Genio 720-EVK board, based
> > on
> > MT8391 SoC.
> > 
> 
> ...
> 
> > +	mmc0_default_pins: mmc0-default-pins {
> > +		pins-clk {
> > +			pinmux = <PINMUX_GPIO162__FUNC_MSDC0_CLK>;
> > +			drive-strength = <6>;
> > +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> > +		};
> > +
> > +		pins-cmd-dat {
> > +			pinmux =
> > <PINMUX_GPIO166__FUNC_MSDC0_DAT0>,
> > +				
> > <PINMUX_GPIO165__FUNC_MSDC0_DAT1>,
> > +				
> > <PINMUX_GPIO164__FUNC_MSDC0_DAT2>,
> > +				
> > <PINMUX_GPIO163__FUNC_MSDC0_DAT3>,
> > +				
> > <PINMUX_GPIO159__FUNC_MSDC0_DAT4>,
> > +				
> > <PINMUX_GPIO158__FUNC_MSDC0_DAT5>,
> > +				
> > <PINMUX_GPIO157__FUNC_MSDC0_DAT6>,
> > +				
> > <PINMUX_GPIO156__FUNC_MSDC0_DAT7>,
> > +				 <PINMUX_GPIO161__FUNC_MSDC0_CMD>;
> > +			input-enable;
> > +			drive-strength = <6>;
> > +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> > +		};
> 
> Should we also have pins-ds here to match mmc0-uhs-pins?
> 
The data strobe pin is only used for the HS modes, that is why it is
only declared for uhs state.
No other mediatek board devicetrees have it for default state too, so I
don't think it is needed here.

> > +
> > +		pins-rst {
> > +			pinmux =
> > <PINMUX_GPIO160__FUNC_MSDC0_RSTB>;
> > +			drive-strength = <6>;
> > +			bias-pull-up = <MTK_PUPD_SET_R1R0_00>;
> > +		};
> > +	};
> > +
> > +	mmc0_uhs_pins: mmc0-uhs-pins {
> > +		pins-clk {
> > +			pinmux = <PINMUX_GPIO162__FUNC_MSDC0_CLK>;
> > +			drive-strength = <8>;
> > +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> > +		};
> > +
> > +		pins-cmd-dat {
> > +			pinmux =
> > <PINMUX_GPIO166__FUNC_MSDC0_DAT0>,
> > +				
> > <PINMUX_GPIO165__FUNC_MSDC0_DAT1>,
> > +				
> > <PINMUX_GPIO164__FUNC_MSDC0_DAT2>,
> > +				
> > <PINMUX_GPIO163__FUNC_MSDC0_DAT3>,
> > +				
> > <PINMUX_GPIO159__FUNC_MSDC0_DAT4>,
> > +				
> > <PINMUX_GPIO158__FUNC_MSDC0_DAT5>,
> > +				
> > <PINMUX_GPIO157__FUNC_MSDC0_DAT6>,
> > +				
> > <PINMUX_GPIO156__FUNC_MSDC0_DAT7>,
> > +				 <PINMUX_GPIO161__FUNC_MSDC0_CMD>;
> > +			input-enable;
> > +			drive-strength = <8>;
> > +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> > +		};
> > +
> > +		pins-ds {
> > +			pinmux = <PINMUX_GPIO167__FUNC_MSDC0_DSL>;
> > +			drive-strength = <8>;
> > +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> > +		};
> > +
> > +		pins-rst {
> > +			pinmux =
> > <PINMUX_GPIO160__FUNC_MSDC0_RSTB>;
> > +			bias-pull-up = <MTK_PUPD_SET_R1R0_00>;
> > +		};
> > +	};
> > +
> > +	mmc1_default_pins: mmc1-default-pins {
> > +		pins-clk {
> > +			pinmux = <PINMUX_GPIO169__FUNC_MSDC1_CLK>;
> > +			drive-strength = <6>;
> > +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> > +		};
> > +
> > +		pins-cmd-dat {
> > +			pinmux =
> > <PINMUX_GPIO170__FUNC_MSDC1_DAT0>,
> > +				
> > <PINMUX_GPIO171__FUNC_MSDC1_DAT1>,
> > +				
> > <PINMUX_GPIO172__FUNC_MSDC1_DAT2>,
> > +				
> > <PINMUX_GPIO173__FUNC_MSDC1_DAT3>,
> > +				 <PINMUX_GPIO168__FUNC_MSDC1_CMD>;
> > +			input-enable;
> > +			drive-strength = <6>;
> > +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> > +		};
> > +
> > +		pins-insert {
> > +			pinmux = <PINMUX_GPIO2__FUNC_GPIO2>;
> > +			bias-pull-up;
> > +		};
> > +	};
> > +
> > +	mmc1_uhs_pins: mmc1-uhs-pins {
> > +		pins-clk {
> > +			pinmux = <PINMUX_GPIO169__FUNC_MSDC1_CLK>;
> > +			drive-strength = <8>;
> > +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> > +		};
> > +
> > +		pins-cmd-dat {
> > +			pinmux =
> > <PINMUX_GPIO170__FUNC_MSDC1_DAT0>,
> > +				
> > <PINMUX_GPIO171__FUNC_MSDC1_DAT1>,
> > +				
> > <PINMUX_GPIO172__FUNC_MSDC1_DAT2>,
> > +				
> > <PINMUX_GPIO173__FUNC_MSDC1_DAT3>,
> > +				 <PINMUX_GPIO168__FUNC_MSDC1_CMD>;
> > +			input-enable;
> > +			drive-strength = <8>;
> > +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> > +		};
> 
> Don't we also need pins-insert here? (to match mmc1-default-pins)
> 
From what I've found, it was done this way for other board devicetrees
to avoid possible reconfiguration happen for the card detection pin
while switching to UHS and causing a switch failure.
Also, what you declare in pinmux nodes is how the pin configuration
should change in a specific mode and you declare only what changes, and
not what stay the same.

> I was having trouble with the CD input pin not working in U-Boot
> until I added it.
I checked and debugged this on my board and did not get that kind of
issue, whether this pin config is not present for uhs state or if I add
it. 
When inserting my sd card or when I boot with it already inserted, the
mtk-sd driver first sets the pinctrl state to default before switching
to ufs. The GPIO02 pin config is also OK in both states.
The mt8189 pinctrl driver seems to apply a default config for this pin
that is the same as the one that is set here.

There might be u-boot particularities that could explain it fixes your
issue.

Regards,
Louis-Alexis
> 
> > +	};
> > +