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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1fcfc835-64aa-48d0-87db-8de14913f3a4@gmail.com>
Date: Thu, 2 Jan 2025 23:30:01 +0200
From: Markuss Broks <markuss.broks@...il.com>
To: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>,
 Ulf Hansson <ulf.hansson@...aro.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Alim Akhtar <alim.akhtar@...sung.com>, Jaehoon Chung <jh80.chung@...sung.com>
Cc: linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 5/5] arm64: dts: exynos: exynos8895-dreamlte: enable
 support for microSD storage

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@...il.com>
> ---
>   .../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