diff mbox series

sunxi: Bring back SD card as MMC device 0

Message ID 20210416110809.25421-1-andre.przywara@arm.com
State Accepted
Commit 6785434709b74b9a821a21e7228a4c9eb4843095
Delegated to: Andre Przywara
Headers show
Series sunxi: Bring back SD card as MMC device 0 | expand

Commit Message

Andre Przywara April 16, 2021, 11:08 a.m. UTC
Commit 2243d19e5618 ("mmc: mmc-uclass: Use dev_seq() to read aliases
node's index") now actually enforces U-Boot's device enumeration policy,
where explicitly named devices come first, then any other non-named
devices follow, without filling gaps.

For quite a while we have had an "mmc1 = &mmc2;" alias in our
sunxi-u-boot.dtsi, which now leads to the problem that the SD card
(which was always mmc device 0) now gets to be number 2.
I guess this breaks quite some boot scripts, and is confusing at least.

Just add an explicit mmc0 alias in the very same file to fix this and
restore the old behaviour.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reported-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm/dts/sunxi-u-boot.dtsi | 1 +
 1 file changed, 1 insertion(+)

Comments

Andre Przywara April 25, 2021, 9:59 a.m. UTC | #1
On Fri, 16 Apr 2021 12:08:09 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

Hi,

> Commit 2243d19e5618 ("mmc: mmc-uclass: Use dev_seq() to read aliases
> node's index") now actually enforces U-Boot's device enumeration policy,
> where explicitly named devices come first, then any other non-named
> devices follow, without filling gaps.
> 
> For quite a while we have had an "mmc1 = &mmc2;" alias in our
> sunxi-u-boot.dtsi, which now leads to the problem that the SD card
> (which was always mmc device 0) now gets to be number 2.
> I guess this breaks quite some boot scripts, and is confusing at least.
> 
> Just add an explicit mmc0 alias in the very same file to fix this and
> restore the old behaviour.

Can someone please say if this is the right solution?
I think the SD card has always been mmc device 0 in U-Boot, so I think
it's worth keeping that. Just not sure if this is the right way of
fixing that?

Cheers,
Andre

> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reported-by: Samuel Holland <samuel@sholland.org>
> ---
>  arch/arm/dts/sunxi-u-boot.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi
> index a4227a3c22d..06da009fa28 100644
> --- a/arch/arm/dts/sunxi-u-boot.dtsi
> +++ b/arch/arm/dts/sunxi-u-boot.dtsi
> @@ -12,6 +12,7 @@
>  
>  / {
>  	aliases {
> +		mmc0 = &mmc0;
>  		mmc1 = &mmc2;
>  	};
>
Jagan Teki April 25, 2021, 12:33 p.m. UTC | #2
On Sun, Apr 25, 2021 at 3:30 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Fri, 16 Apr 2021 12:08:09 +0100
> Andre Przywara <andre.przywara@arm.com> wrote:
>
> Hi,
>
> > Commit 2243d19e5618 ("mmc: mmc-uclass: Use dev_seq() to read aliases
> > node's index") now actually enforces U-Boot's device enumeration policy,
> > where explicitly named devices come first, then any other non-named
> > devices follow, without filling gaps.
> >
> > For quite a while we have had an "mmc1 = &mmc2;" alias in our
> > sunxi-u-boot.dtsi, which now leads to the problem that the SD card
> > (which was always mmc device 0) now gets to be number 2.
> > I guess this breaks quite some boot scripts, and is confusing at least.
> >
> > Just add an explicit mmc0 alias in the very same file to fix this and
> > restore the old behaviour.
>
> Can someone please say if this is the right solution?
> I think the SD card has always been mmc device 0 in U-Boot, so I think
> it's worth keeping that. Just not sure if this is the right way of
> fixing that?

Playing with aliases always gets confused and might get a different
behavior, IMHO. Detect the dev number by U-Boot itself and look at
traverse bootenv by all possible dev numbers in sunxi-common.h, but
this has one slide effect that we mark mmc2 as devnum 1 for the sake
of fastboot so if we mark fastboot number for specific board properly
(by static or runtime) then explicit aliases wouldn't required.

Jagan.
Andre Przywara April 26, 2021, 11:27 a.m. UTC | #3
On Sun, 25 Apr 2021 18:03:05 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

Hi Jagan,

thanks for your input!

> On Sun, Apr 25, 2021 at 3:30 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Fri, 16 Apr 2021 12:08:09 +0100
> > Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > Hi,
> >  
> > > Commit 2243d19e5618 ("mmc: mmc-uclass: Use dev_seq() to read aliases
> > > node's index") now actually enforces U-Boot's device enumeration policy,
> > > where explicitly named devices come first, then any other non-named
> > > devices follow, without filling gaps.
> > >
> > > For quite a while we have had an "mmc1 = &mmc2;" alias in our
> > > sunxi-u-boot.dtsi, which now leads to the problem that the SD card
> > > (which was always mmc device 0) now gets to be number 2.
> > > I guess this breaks quite some boot scripts, and is confusing at least.
> > >
> > > Just add an explicit mmc0 alias in the very same file to fix this and
> > > restore the old behaviour.  
> >
> > Can someone please say if this is the right solution?
> > I think the SD card has always been mmc device 0 in U-Boot, so I think
> > it's worth keeping that. Just not sure if this is the right way of
> > fixing that?  
> 
> Playing with aliases always gets confused and might get a different
> behavior, IMHO.

I am not so sure about that, since aliases force a fixed numbering, so
it should be less confusing. We have the same problem in the kernel
now, and Samuel sent some patches to have aliases in the mainline DTs
as well [1].

> Detect the dev number by U-Boot itself and look at
> traverse bootenv by all possible dev numbers in sunxi-common.h, but

OK, I will have a look at how the automatic distro boot behaves with
this change. My concern was more the interactive user, who is used to
have the SD card at device 0 (I certainly am).

> this has one slide effect that we mark mmc2 as devnum 1 for the sake
> of fastboot so if we mark fastboot number for specific board properly
> (by static or runtime) then explicit aliases wouldn't required.

Ah, good point, thanks for the heads up. I guess this is the actual
reason for the alias in our -u-boot.dtsi? Maybe we find a different way
to let fastboot find the eMMC? Then we can drop the extra mmc1 alias,
get our numbering back, and can cope with the incoming aliases from the
mainline DTs as well?

Cheers,
Andre

[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2021-April/652946.html
Jagan Teki April 26, 2021, 12:22 p.m. UTC | #4
On Mon, Apr 26, 2021 at 4:57 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Sun, 25 Apr 2021 18:03:05 +0530
> Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Jagan,
>
> thanks for your input!
>
> > On Sun, Apr 25, 2021 at 3:30 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > >
> > > On Fri, 16 Apr 2021 12:08:09 +0100
> > > Andre Przywara <andre.przywara@arm.com> wrote:
> > >
> > > Hi,
> > >
> > > > Commit 2243d19e5618 ("mmc: mmc-uclass: Use dev_seq() to read aliases
> > > > node's index") now actually enforces U-Boot's device enumeration policy,
> > > > where explicitly named devices come first, then any other non-named
> > > > devices follow, without filling gaps.
> > > >
> > > > For quite a while we have had an "mmc1 = &mmc2;" alias in our
> > > > sunxi-u-boot.dtsi, which now leads to the problem that the SD card
> > > > (which was always mmc device 0) now gets to be number 2.
> > > > I guess this breaks quite some boot scripts, and is confusing at least.
> > > >
> > > > Just add an explicit mmc0 alias in the very same file to fix this and
> > > > restore the old behaviour.
> > >
> > > Can someone please say if this is the right solution?
> > > I think the SD card has always been mmc device 0 in U-Boot, so I think
> > > it's worth keeping that. Just not sure if this is the right way of
> > > fixing that?
> >
> > Playing with aliases always gets confused and might get a different
> > behavior, IMHO.
>
> I am not so sure about that, since aliases force a fixed numbering, so
> it should be less confusing. We have the same problem in the kernel
> now, and Samuel sent some patches to have aliases in the mainline DTs
> as well [1].

Okay.

>
> > Detect the dev number by U-Boot itself and look at
> > traverse bootenv by all possible dev numbers in sunxi-common.h, but
>
> OK, I will have a look at how the automatic distro boot behaves with
> this change. My concern was more the interactive user, who is used to
> have the SD card at device 0 (I certainly am).
>
> > this has one slide effect that we mark mmc2 as devnum 1 for the sake
> > of fastboot so if we mark fastboot number for specific board properly
> > (by static or runtime) then explicit aliases wouldn't required.
>
> Ah, good point, thanks for the heads up. I guess this is the actual
> reason for the alias in our -u-boot.dtsi? Maybe we find a different way
> to let fastboot find the eMMC? Then we can drop the extra mmc1 alias,
> get our numbering back, and can cope with the incoming aliases from the
> mainline DTs as well?

Agreed.
Andre Przywara May 24, 2021, 12:48 a.m. UTC | #5
On Mon, 26 Apr 2021 17:52:51 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

Hi Jagan,

> On Mon, Apr 26, 2021 at 4:57 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Sun, 25 Apr 2021 18:03:05 +0530
> > Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Hi Jagan,
> >
> > thanks for your input!
> >  
> > > On Sun, Apr 25, 2021 at 3:30 PM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > >
> > > > On Fri, 16 Apr 2021 12:08:09 +0100
> > > > Andre Przywara <andre.przywara@arm.com> wrote:
> > > >
> > > > Hi,
> > > >  
> > > > > Commit 2243d19e5618 ("mmc: mmc-uclass: Use dev_seq() to read aliases
> > > > > node's index") now actually enforces U-Boot's device enumeration policy,
> > > > > where explicitly named devices come first, then any other non-named
> > > > > devices follow, without filling gaps.
> > > > >
> > > > > For quite a while we have had an "mmc1 = &mmc2;" alias in our
> > > > > sunxi-u-boot.dtsi, which now leads to the problem that the SD card
> > > > > (which was always mmc device 0) now gets to be number 2.
> > > > > I guess this breaks quite some boot scripts, and is confusing at least.
> > > > >
> > > > > Just add an explicit mmc0 alias in the very same file to fix this and
> > > > > restore the old behaviour.  
> > > >
> > > > Can someone please say if this is the right solution?
> > > > I think the SD card has always been mmc device 0 in U-Boot, so I think
> > > > it's worth keeping that. Just not sure if this is the right way of
> > > > fixing that?  
> > >
> > > Playing with aliases always gets confused and might get a different
> > > behavior, IMHO.  
> >
> > I am not so sure about that, since aliases force a fixed numbering, so
> > it should be less confusing. We have the same problem in the kernel
> > now, and Samuel sent some patches to have aliases in the mainline DTs
> > as well [1].  
> 
> Okay.
> 
> >  
> > > Detect the dev number by U-Boot itself and look at
> > > traverse bootenv by all possible dev numbers in sunxi-common.h, but  
> >
> > OK, I will have a look at how the automatic distro boot behaves with
> > this change. My concern was more the interactive user, who is used to
> > have the SD card at device 0 (I certainly am).
> >  
> > > this has one slide effect that we mark mmc2 as devnum 1 for the sake
> > > of fastboot so if we mark fastboot number for specific board properly
> > > (by static or runtime) then explicit aliases wouldn't required.  
> >
> > Ah, good point, thanks for the heads up. I guess this is the actual
> > reason for the alias in our -u-boot.dtsi? Maybe we find a different way
> > to let fastboot find the eMMC? Then we can drop the extra mmc1 alias,
> > get our numbering back, and can cope with the incoming aliases from the
> > mainline DTs as well?  
> 
> Agreed.

So I had a look at fastboot, and posted an RFC[1] to try and fix this
properly. This seems to somewhat work, but there are more assumptions
in (sunxi) U-Boot about the eMMC being device 1 (boot source,
distro_boot scripts, ...). I agree that we should remove this
assumption, but this requires more work and scrutiny to find all those
cases.

For now the change in numbering breaks right, left, and centre: boot.scr
cannot be found anymore, fastboot cannot find the SD card, custom
scripts stop working.

So I would very much like to merge this patch here, to fix the
currently bad user experience and get a clean v2021.07 release.
We can meanwhile look into a proper solution for this problem, to
become more robust against enumeration changes (for instance due to
aliases).

Thanks,
Andre
Simon Baatz May 29, 2021, 9:21 a.m. UTC | #6
Hi,

On Mon, May 24, 2021 at 01:48:38AM +0100, Andre Przywara wrote:
> ...
> 
> For now the change in numbering breaks right, left, and centre: boot.scr
> cannot be found anymore, fastboot cannot find the SD card, custom
> scripts stop working.
> 
> So I would very much like to merge this patch here, to fix the
> currently bad user experience and get a clean v2021.07 release.
> We can meanwhile look into a proper solution for this problem, to
> become more robust against enumeration changes (for instance due to
> aliases).


As a user who spent quite some time trying to find out what's going
on, I can only second that.

If it helps, I can offer a "tested by".  I tested this patch on a
Pine 64+ using pine64_plus_defconfig.

Tested-by: Simon Baatz <gmbnomis@gmail.com>
diff mbox series

Patch

diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi
index a4227a3c22d..06da009fa28 100644
--- a/arch/arm/dts/sunxi-u-boot.dtsi
+++ b/arch/arm/dts/sunxi-u-boot.dtsi
@@ -12,6 +12,7 @@ 
 
 / {
 	aliases {
+		mmc0 = &mmc0;
 		mmc1 = &mmc2;
 	};