Message-ID: <8fd04935-0553-e04b-7d8c-470573816e6f@samsung.com>
Date:   Thu, 9 Feb 2023 15:50:50 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0)
        Gecko/20100101 Thunderbird/102.7.1
Subject: Re: [PATCH v2 2/2] ARM: dts: exynos: add mmc aliases
Content-Language: en-US
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Henrik Grimler <henrik@grimler.se>, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, alim.akhtar@samsung.com,
        jenneron@protonmail.com, markuss.broks@gmail.com,
        martin.juecker@gmail.com, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
        phone-devel@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht
Cc:     Valentine Iourine <iourine@iourine.msk.su>
From:   Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <99a17d21-2cf9-a573-29cb-827568c9709b@linaro.org>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20230128133151.29471-1-henrik@grimler.se>
        <20230128133151.29471-3-henrik@grimler.se>
        <CGME20230209142330eucas1p2d7ba56b6496bb90ed6af2054fe929c9d@eucas1p2.samsung.com>
        <99a17d21-2cf9-a573-29cb-827568c9709b@linaro.org>
Precedence: bulk

On 09.02.2023 15:23, Krzysztof Kozlowski wrote:
> On 28/01/2023 14:31, Henrik Grimler wrote:
>> Add aliases for eMMC, SD card and WiFi where applicable, so that
>> assigned mmcblk numbers are always the same.
>>
>> Co-developed-by: Anton Bambura <jenneron@protonmail.com>
>> Signed-off-by: Anton Bambura <jenneron@protonmail.com>
>> [ Tested on exynos5800-peach-pi ]
>> Tested-by: Valentine Iourine <iourine@iourine.msk.su>
>> Signed-off-by: Henrik Grimler <henrik@grimler.se>
>> ---
>>   arch/arm/boot/dts/exynos3250-artik5-eval.dts        | 5 +++++
>>   arch/arm/boot/dts/exynos3250-artik5.dtsi            | 5 +++++
>>   arch/arm/boot/dts/exynos3250-monk.dts               | 1 +
>>   arch/arm/boot/dts/exynos3250-rinato.dts             | 2 ++
>>   arch/arm/boot/dts/exynos4210-i9100.dts              | 6 ++++++
>>   arch/arm/boot/dts/exynos4210-origen.dts             | 5 +++++
>>   arch/arm/boot/dts/exynos4210-smdkv310.dts           | 4 ++++
>>   arch/arm/boot/dts/exynos4210-trats.dts              | 6 ++++++
>>   arch/arm/boot/dts/exynos4210-universal_c210.dts     | 6 ++++++
>>   arch/arm/boot/dts/exynos4412-itop-elite.dts         | 5 +++++
>>   arch/arm/boot/dts/exynos4412-midas.dtsi             | 3 +++
>>   arch/arm/boot/dts/exynos4412-odroid-common.dtsi     | 5 +++++
>>   arch/arm/boot/dts/exynos4412-origen.dts             | 5 +++++
>>   arch/arm/boot/dts/exynos4412-p4note.dtsi            | 6 ++++++
>>   arch/arm/boot/dts/exynos4412-smdk4412.dts           | 4 ++++
>>   arch/arm/boot/dts/exynos4412-tiny4412.dts           | 4 ++++
>>   arch/arm/boot/dts/exynos5250-arndale.dts            | 5 +++++
>>   arch/arm/boot/dts/exynos5250-smdk5250.dts           | 2 ++
>>   arch/arm/boot/dts/exynos5250-snow-common.dtsi       | 3 +++
>>   arch/arm/boot/dts/exynos5250-spring.dts             | 5 +++++
>>   arch/arm/boot/dts/exynos5260-xyref5260.dts          | 5 +++++
>>   arch/arm/boot/dts/exynos5410-odroidxu.dts           | 2 ++
>>   arch/arm/boot/dts/exynos5410-smdk5410.dts           | 5 +++++
>>   arch/arm/boot/dts/exynos5420-arndale-octa.dts       | 5 +++++
>>   arch/arm/boot/dts/exynos5420-galaxy-tab-common.dtsi | 5 +++++
>>   arch/arm/boot/dts/exynos5420-peach-pit.dts          | 3 +++
>>   arch/arm/boot/dts/exynos5420-smdk5420.dts           | 5 +++++
>>   arch/arm/boot/dts/exynos5422-odroid-core.dtsi       | 5 +++++
>>   arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi  | 4 ++++
>>   arch/arm/boot/dts/exynos5422-samsung-k3g.dts        | 4 ++++
>>   arch/arm/boot/dts/exynos5800-peach-pi.dts           | 3 +++
>>   31 files changed, 133 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos3250-artik5-eval.dts b/arch/arm/boot/dts/exynos3250-artik5-eval.dts
>> index a1e22f630638..83266a66124b 100644
>> --- a/arch/arm/boot/dts/exynos3250-artik5-eval.dts
>> +++ b/arch/arm/boot/dts/exynos3250-artik5-eval.dts
>> @@ -16,6 +16,11 @@ / {
>>   	model = "Samsung ARTIK5 evaluation board";
>>   	compatible = "samsung,artik5-eval", "samsung,artik5",
>>   			"samsung,exynos3250", "samsung,exynos3";
>> +
>> +	aliases {
>> +		mmc2 = &mshc_2;
>> +	};
>> +
>>   };
>>   
>>   &mshc_2 {
>> diff --git a/arch/arm/boot/dts/exynos3250-artik5.dtsi b/arch/arm/boot/dts/exynos3250-artik5.dtsi
>> index 0ac3f284fbb8..a6e2f46917a8 100644
>> --- a/arch/arm/boot/dts/exynos3250-artik5.dtsi
>> +++ b/arch/arm/boot/dts/exynos3250-artik5.dtsi
>> @@ -17,6 +17,11 @@
>>   / {
>>   	compatible = "samsung,artik5", "samsung,exynos3250", "samsung,exynos3";
>>   
>> +	aliases {
>> +		mmc0 = &mshc_0;
>> +		mmc1 = &mshc_1;
>> +	};
>> +
>>   	chosen {
>>   		stdout-path = &serial_2;
>>   	};
>> diff --git a/arch/arm/boot/dts/exynos3250-monk.dts b/arch/arm/boot/dts/exynos3250-monk.dts
>> index 80d90fe7fad1..a68e5f81404c 100644
>> --- a/arch/arm/boot/dts/exynos3250-monk.dts
>> +++ b/arch/arm/boot/dts/exynos3250-monk.dts
>> @@ -22,6 +22,7 @@ / {
>>   
>>   	aliases {
>>   		i2c7 = &i2c_max77836;
>> +		mmc0 = &mshc_0;
>>   	};
>>   
>>   	memory@40000000 {
>> diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
>> index 1f9cba0607e1..f0fb6890e4a8 100644
>> --- a/arch/arm/boot/dts/exynos3250-rinato.dts
>> +++ b/arch/arm/boot/dts/exynos3250-rinato.dts
>> @@ -23,6 +23,8 @@ / {
>>   
>>   	aliases {
>>   		i2c7 = &i2c_max77836;
>> +		mmc0 = &mshc_0;
>> +		mmc1 = &mshc_1;
>>   	};
>>   
>>   	chosen {
>> diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
>> index bba85011ecc9..7051e2c4b391 100644
>> --- a/arch/arm/boot/dts/exynos4210-i9100.dts
>> +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
>> @@ -25,6 +25,12 @@ memory@40000000 {
>>   		reg = <0x40000000 0x40000000>;
>>   	};
>>   
>> +	aliases {
>> +		mmc0 = &sdhci_0;
>> +		mmc2 = &sdhci_2;
>> +		mmc3 = &sdhci_3;
> 1. Is this actually correct? Since mmc1 was disabled, sdhci_2 had mmc1
> index but now will have mmc2.
>
> 2. I tested Odroid U3 and the ID changed. emmc went from 1 to 0. Any
> idea why? Both patches should be transparent.

Nope, eMMC and SD order changed a few times in the past and now they get 
indices based on the probe time, so any order is possible depending on 
the presence of the sd card / eMMC module.

> 3. Patchset does not look bisectable, so both patches should be squashed.

Why? First patch removes obsolete mshc aliases, which don't determine 
the logical MMC device number in the system. The second one adds fixed 
indices to the local MMC devices created by the respective MMC host 
controllers.

> 4. Your patch has several whitespace issues:
> patch:264: trailing whitespace.
>
> patch:279: trailing whitespace.
>
> patch:461: trailing whitespace.
>
>
>
> Best regards,
> Krzysztof
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland