2020-11-04 10:23:37

by Markus Reichl

[permalink] [raw]
Subject: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.

Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in evironments where UUIDs
are not practical. Use newly introduced aliases for mmcblk devices from [1].

[1]
https://patchwork.kernel.org/patch/11747669/

Signed-off-by: Markus Reichl <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
index e7a459fa4322..bc9482b59428 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
@@ -13,6 +13,11 @@ / {
model = "Firefly ROC-RK3399-PC Board";
compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";

+ aliases {
+ mmc0 = &sdmmc;
+ mmc1 = &sdhci;
+ };
+
chosen {
stdout-path = "serial2:1500000n8";
};
--
2.28.0


2020-11-04 10:55:24

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.

Hi Markus,

Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
> Recently introduced async probe on mmc devices can shuffle block IDs.
> Pin them to fixed values to ease booting in evironments where UUIDs
> are not practical. Use newly introduced aliases for mmcblk devices from [1].
>
> [1]
> https://patchwork.kernel.org/patch/11747669/
>
> Signed-off-by: Markus Reichl <[email protected]>
> ---
> arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> index e7a459fa4322..bc9482b59428 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> @@ -13,6 +13,11 @@ / {
> model = "Firefly ROC-RK3399-PC Board";
> compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
>
> + aliases {
> + mmc0 = &sdmmc;
> + mmc1 = &sdhci;
> + };
> +

Any reason for this odering?

I.e. some previous incarnations had it ordered as (emmc, mmc, sdio).
This is also true for the ChromeOS out-of-tree usage of those, the
rk3399 dts in the chromeos-4.4 tree also orders this as sdhci, sdmmc, sdio.

And I guess a further question would be when we're doing arbitary orderings
anyway, why is this not in rk3399.dtsi ;-) ?


Heiko


2020-11-04 11:19:59

by Markus Reichl

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.

Hi Heiko,

Am 04.11.20 um 11:51 schrieb Heiko Stübner:
> Hi Markus,
>
> Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
>> Recently introduced async probe on mmc devices can shuffle block IDs.
>> Pin them to fixed values to ease booting in evironments where UUIDs
>> are not practical. Use newly introduced aliases for mmcblk devices from [1].
>>
>> [1]
>> https://patchwork.kernel.org/patch/11747669/
>>
>> Signed-off-by: Markus Reichl <[email protected]>
>> ---
>> arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>> index e7a459fa4322..bc9482b59428 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>> @@ -13,6 +13,11 @@ / {
>> model = "Firefly ROC-RK3399-PC Board";
>> compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
>>
>> + aliases {
>> + mmc0 = &sdmmc;
>> + mmc1 = &sdhci;
>> + };
>> +
>
> Any reason for this odering?

Without pinning roc-pc mostly booted as
mmcblk0 = sdmmc = µSD
mmcblk1 = sdhci = eMMC
so I kept this behaviour in aliases

roc-pc-mezzanine with populated SDIO-M2-slot booted
mmc0 = sdio = (no mmcblk)
mmcblk1 = sdmmc = µSD
mmcblk2 = sdhci = eMMC

With my aliases both boards behave the same now and the optional SDIO slot
goes out of the way to mmc2.

>
> I.e. some previous incarnations had it ordered as (emmc, mmc, sdio).
> This is also true for the ChromeOS out-of-tree usage of those, the
> rk3399 dts in the chromeos-4.4 tree also orders this as sdhci, sdmmc, sdio.

The boards from my zoo (exynos, rk3399) mostly come up with SD-card as mmc0
and eMMC as mmc1 in mainline as opposed in some vendor kernels.
but I have no objection to set it the other way round if this is more common
with rk3399 boards.

>
> And I guess a further question would be when we're doing arbitary orderings
> anyway, why is this not in rk3399.dtsi ;-) ?

I restricted the ordering to the boards I have, not to confuse other established
use cases, but if a standard ordering is desired this can go to rk3399.dtsi.

>
>
> Heiko
>
>

Gruß,
--
Markus Reichl

2020-11-04 12:20:32

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.

On 2020-11-04 11:15, Markus Reichl wrote:
> Hi Heiko,
>
> Am 04.11.20 um 11:51 schrieb Heiko Stübner:
>> Hi Markus,
>>
>> Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
>>> Recently introduced async probe on mmc devices can shuffle block IDs.
>>> Pin them to fixed values to ease booting in evironments where UUIDs
>>> are not practical. Use newly introduced aliases for mmcblk devices
>>> from [1].
>>>
>>> [1]
>>> https://patchwork.kernel.org/patch/11747669/
>>>
>>> Signed-off-by: Markus Reichl <[email protected]>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> index e7a459fa4322..bc9482b59428 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> @@ -13,6 +13,11 @@ / {
>>>      model = "Firefly ROC-RK3399-PC Board";
>>>      compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
>>>
>>> +    aliases {
>>> +        mmc0 = &sdmmc;
>>> +        mmc1 = &sdhci;
>>> +    };
>>> +
>>
>> Any reason for this odering?
>
> Without pinning roc-pc mostly booted as
> mmcblk0 = sdmmc = µSD
> mmcblk1 = sdhci = eMMC
> so I kept this behaviour in aliases
>
> roc-pc-mezzanine with populated SDIO-M2-slot booted
> mmc0 = sdio = (no mmcblk)
> mmcblk1 = sdmmc = µSD
> mmcblk2 = sdhci = eMMC

FWIW that's also how my NanoPC-T4 behaves. Given that it's the order
they appear in the DT, not too surprising ;)

Robin.

> With my aliases both boards behave the same now and the optional SDIO slot
> goes out of the way to mmc2.
>
>>
>> I.e. some previous incarnations had it ordered as (emmc, mmc, sdio).
>> This is also true for the ChromeOS out-of-tree usage of those, the
>> rk3399 dts in the chromeos-4.4 tree also orders this as sdhci, sdmmc,
>> sdio.
>
> The boards from my zoo (exynos, rk3399) mostly come up with SD-card as mmc0
> and eMMC as mmc1 in mainline as opposed in some vendor kernels.
> but I have no objection to set it the other way round if this is more
> common
> with rk3399 boards.
>
>>
>> And I guess a further question would be when we're doing arbitary
>> orderings
>> anyway, why is this not in rk3399.dtsi ;-) ?
>
> I restricted the ordering to the boards I have, not to confuse other
> established
> use cases, but if a standard ordering is desired this can go to
> rk3399.dtsi.
>
>>
>>
>> Heiko
>>
>>
>
> Gruß,

2020-11-04 15:45:51

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.

Hi,

On Wed, Nov 4, 2020 at 2:51 AM Heiko Stübner <[email protected]> wrote:
>
> Hi Markus,
>
> Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
> > Recently introduced async probe on mmc devices can shuffle block IDs.
> > Pin them to fixed values to ease booting in evironments where UUIDs
> > are not practical. Use newly introduced aliases for mmcblk devices from [1].
> >
> > [1]
> > https://patchwork.kernel.org/patch/11747669/
> >
> > Signed-off-by: Markus Reichl <[email protected]>
> > ---
> > arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > index e7a459fa4322..bc9482b59428 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > @@ -13,6 +13,11 @@ / {
> > model = "Firefly ROC-RK3399-PC Board";
> > compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
> >
> > + aliases {
> > + mmc0 = &sdmmc;
> > + mmc1 = &sdhci;
> > + };
> > +
>
> Any reason for this odering?
>
> I.e. some previous incarnations had it ordered as (emmc, mmc, sdio).
> This is also true for the ChromeOS out-of-tree usage of those, the
> rk3399 dts in the chromeos-4.4 tree also orders this as sdhci, sdmmc, sdio.
>
> And I guess a further question would be when we're doing arbitary orderings
> anyway, why is this not in rk3399.dtsi ;-) ?

Though I personally like the idea of eMMC, which is typically
built-in, as being the "0" number, I'm personally happy with any
numbering scheme that's consistent. Ordering them by base address is
OK w/ me and seems less controversial. That seems like it could go in
rk3399.dtsi and then if a particular board wanted a different order
they could override it in their board file. The downside of putting
in rk3399 is that boards that don't have all SD/MMC interfaces enabled
would definitely get a new number compared to old kernels, but
hopefully this is the last time?

-Doug

2020-11-04 15:58:13

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.

Am Mittwoch, 4. November 2020, 16:42:01 CET schrieb Doug Anderson:
> Hi,
>
> On Wed, Nov 4, 2020 at 2:51 AM Heiko St?bner <[email protected]> wrote:
> >
> > Hi Markus,
> >
> > Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
> > > Recently introduced async probe on mmc devices can shuffle block IDs.
> > > Pin them to fixed values to ease booting in evironments where UUIDs
> > > are not practical. Use newly introduced aliases for mmcblk devices from [1].
> > >
> > > [1]
> > > https://patchwork.kernel.org/patch/11747669/
> > >
> > > Signed-off-by: Markus Reichl <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > index e7a459fa4322..bc9482b59428 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > @@ -13,6 +13,11 @@ / {
> > > model = "Firefly ROC-RK3399-PC Board";
> > > compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
> > >
> > > + aliases {
> > > + mmc0 = &sdmmc;
> > > + mmc1 = &sdhci;
> > > + };
> > > +
> >
> > Any reason for this odering?
> >
> > I.e. some previous incarnations had it ordered as (emmc, mmc, sdio).
> > This is also true for the ChromeOS out-of-tree usage of those, the
> > rk3399 dts in the chromeos-4.4 tree also orders this as sdhci, sdmmc, sdio.
> >
> > And I guess a further question would be when we're doing arbitary orderings
> > anyway, why is this not in rk3399.dtsi ;-) ?
>
> Though I personally like the idea of eMMC, which is typically
> built-in, as being the "0" number, I'm personally happy with any
> numbering scheme that's consistent. Ordering them by base address is
> OK w/ me and seems less controversial. That seems like it could go in
> rk3399.dtsi and then if a particular board wanted a different order
> they could override it in their board file.

Yep that sounds sensible and ordering by base address at least is one
"simple" type of order without too much explanation needed.

So I guess we'd get a sdio + sdmmc + sdhci ordering


@Markus: if nobody else complains, can you do a "simple" rk3399.dtsi
change with that please?


> The downside of putting
> in rk3399 is that boards that don't have all SD/MMC interfaces enabled
> would definitely get a new number compared to old kernels, but
> hopefully this is the last time?

With that new asynchronous mmc-probe-thingy in 5.10 that "caused" this,
it sounds like everything gets a new number anyway ;-) .


Heiko


2020-11-04 17:47:55

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.

On Wed, Nov 4, 2020 at 11:55 PM Heiko Stübner <[email protected]> wrote:
>
> Am Mittwoch, 4. November 2020, 16:42:01 CET schrieb Doug Anderson:
> > Hi,
> >
> > On Wed, Nov 4, 2020 at 2:51 AM Heiko Stübner <[email protected]> wrote:
> > >
> > > Hi Markus,
> > >
> > > Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
> > > > Recently introduced async probe on mmc devices can shuffle block IDs.
> > > > Pin them to fixed values to ease booting in evironments where UUIDs
> > > > are not practical. Use newly introduced aliases for mmcblk devices from [1].
> > > >
> > > > [1]
> > > > https://patchwork.kernel.org/patch/11747669/
> > > >
> > > > Signed-off-by: Markus Reichl <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > > index e7a459fa4322..bc9482b59428 100644
> > > > --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > > @@ -13,6 +13,11 @@ / {
> > > > model = "Firefly ROC-RK3399-PC Board";
> > > > compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
> > > >
> > > > + aliases {
> > > > + mmc0 = &sdmmc;
> > > > + mmc1 = &sdhci;
> > > > + };
> > > > +
> > >
> > > Any reason for this odering?
> > >
> > > I.e. some previous incarnations had it ordered as (emmc, mmc, sdio).
> > > This is also true for the ChromeOS out-of-tree usage of those, the
> > > rk3399 dts in the chromeos-4.4 tree also orders this as sdhci, sdmmc, sdio.
> > >
> > > And I guess a further question would be when we're doing arbitary orderings
> > > anyway, why is this not in rk3399.dtsi ;-) ?
> >
> > Though I personally like the idea of eMMC, which is typically
> > built-in, as being the "0" number, I'm personally happy with any
> > numbering scheme that's consistent. Ordering them by base address is
> > OK w/ me and seems less controversial. That seems like it could go in
> > rk3399.dtsi and then if a particular board wanted a different order
> > they could override it in their board file.
>
> Yep that sounds sensible and ordering by base address at least is one
> "simple" type of order without too much explanation needed.
>
> So I guess we'd get a sdio + sdmmc + sdhci ordering
>
>
> @Markus: if nobody else complains, can you do a "simple" rk3399.dtsi
> change with that please?

Please also fix the LED triggers. :)

> > The downside of putting
> > in rk3399 is that boards that don't have all SD/MMC interfaces enabled
> > would definitely get a new number compared to old kernels, but
> > hopefully this is the last time?
>
> With that new asynchronous mmc-probe-thingy in 5.10 that "caused" this,
> it sounds like everything gets a new number anyway ;-) .

Yup.