Re: [PATCH v1 5/5] arm64: dts: exynos: exynos8895-dreamlte: enable support for microSD storage

[Markuss Broks <markuss.broks@xxxxxxxxx>]

Hi Ivaylo,

On 1/2/25 10:58 PM, Ivaylo Ivanov wrote:
> Enable MMC for the Samsung Galaxy S8, used as external microSD card
> storage. Since the main PMIC is currently not supported, assume the
> required regulators are enabled by the previous bootloader.
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@xxxxxxxxx>
> ---
>   .../boot/dts/exynos/exynos8895-dreamlte.dts   | 39 +++++++++++++++++++
>   1 file changed, 39 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos8895-dreamlte.dts b/arch/arm64/boot/dts/exynos/exynos8895-dreamlte.dts
> index 3a376ab2b..11aed89a3 100644
> --- a/arch/arm64/boot/dts/exynos/exynos8895-dreamlte.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos8895-dreamlte.dts
> @@ -16,6 +16,10 @@ / {
>   	compatible = "samsung,dreamlte", "samsung,exynos8895";
>   	chassis-type = "handset";
>   
> +	aliases {
> +		mmc0 = &mmc;
> +	};
> +
>   	chosen {
>   		#address-cells = <2>;
>   		#size-cells = <1>;
> @@ -89,12 +93,47 @@ wink-key {
>   			wakeup-source;
>   		};
>   	};
> +
> +	reg_sd_vmmc: regulator-1 {
> +		compatible = "regulator-fixed";
> +		gpio = <&gpb1 1 GPIO_ACTIVE_HIGH>;
> +		regulator-name = "sdcard-vmmc";
> +		regulator-min-microvolt = <2800000>;
> +		regulator-max-microvolt = <2800000>;
> +		regulator-boot-on;
> +		enable-active-high;
> +	};

This regulator was described in downstream, but it doesn't look like it 
exists on schematics. It could be different revisions having different 
hardware, of course, but for me it looks like it's redundant. (on 
schematics VMMC (Vdd for the card) is powered by S2MPS17 LDO29, and 
VQMMC (logic level) is powered by S2MPS17 LDO2).

I believe we should check if this hw exists by perhaps checking if 
manually changing GPIO output value changes the card behavior, and if it 
doesn't, it should be safe to drop it.

>   };
>   
>   &oscclk {
>   	clock-frequency = <26000000>;
>   };
>   
> +&mmc {
> +	assigned-clocks = <&cmu_top CLK_MOUT_CMU_FSYS1_MMC_CARD>;
> +	assigned-clock-parents = <&cmu_top CLK_FOUT_SHARED4_PLL>;
To clarify, as proven by testing, this exists because with default (on 
reset) mux MOUT_CMU_FSYS1_MMC_CARD configuration, the card doesn't show 
up, so the parent is manually set to SHARED4 PLL. With this in mind, 
maybe it makes more sense to put this into SoC dtsi, since it can't 
possibly be board-specific?
> +
> +	pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_bus1 &sd2_bus4>;
> +	pinctrl-names = "default";
> +
> +	bus-width = <4>;
> +	card-detect-delay = <200>;
> +	cd-gpios = <&gpa1 5 GPIO_ACTIVE_LOW>;
Missing pinctrl for card detect pin?
> +	clock-frequency = <50000000>;
> +	disable-wp;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
> +
> +	/* TODO: Update once PMIC is implemented */
> +	vmmc-supply = <&reg_sd_vmmc>;
> +
> +	samsung,dw-mshc-ciu-div = <3>;
> +	samsung,dw-mshc-ddr-timing = <1 2>;
> +	samsung,dw-mshc-sdr-timing = <0 3>;
> +
> +	status = "okay";
> +};
> +
>   &pinctrl_alive {
>   	key_power: key-power-pins {
>   		samsung,pins = "gpa2-4";


- Markuss