[PATCH 0/2] sunxi: Enforce consistent MMC numbering

[PATCH 0/2] sunxi: Enforce consistent MMC numbering

[Samuel Holland]

Dealing with the inconsistent numbering has been a major pain, and
there is a solution with (as far as I can tell) no tangible downsides.
So let's use it.

Yes, I know the kernel supports UUIDs for root=. But UUIDs do not help
when referencing the whole, unpartitioned device, like is needed for
updating the bootloader and firmware. So for the use case of "write a
bootloader to the SD card, regardless of where the board is currently
booted from", I know of two options:
  - Dig around in sysfs to find the mmc number from the MMIO address,
    which means I have to know the MMIO addresses for every SoC, or
  - Apply patches like these.

Samuel Holland (2):
  ARM: dts: sunxi: h3/h5: Enforce consistent MMC numbering
  arm64: dts: allwinner: Enforce consistent MMC numbering

 arch/arm/boot/dts/sunxi-h3-h5.dtsi            | 6 ++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 6 ++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 6 ++++++
 3 files changed, 18 insertions(+)

-- 
2.26.3

[PATCH 1/2] ARM: dts: sunxi: h3/h5: Enforce consistent MMC numbering

[Samuel Holland]

Traditionally, the sunxi-mmc device numbers matched the register address
order. However, that was broken by asynchronous probe, and now the MMC
device numbers are not deterministic. Add aliases to keep the device
numbers consistent between boots. Use the traditional order, since there
is no need to change it.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm/boot/dts/sunxi-h3-h5.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index e27399aa052c..1cb669c835bd 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -53,6 +53,12 @@ / {
 	#address-cells = <1>;
 	#size-cells = <1>;
 
+	aliases {
+		mmc0 = &mmc0;
+		mmc1 = &mmc1;
+		mmc2 = &mmc2;
+	};
+
 	chosen {
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
2.26.3

[PATCH 2/2] arm64: dts: allwinner: Enforce consistent MMC numbering

[Samuel Holland]

Traditionally, the sunxi-mmc device numbers matched the register address
order. However, that was broken by asynchronous probe, and now the MMC
device numbers are not deterministic. Add aliases to keep the device
numbers consistent between boots. Use the traditional order, since there
is no need to change it.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 6 ++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 9b58cfbefa6d..3df70a41b3b9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -17,6 +17,12 @@ / {
 	#address-cells = <1>;
 	#size-cells = <1>;
 
+	aliases {
+		mmc0 = &mmc0;
+		mmc1 = &mmc1;
+		mmc2 = &mmc2;
+	};
+
 	chosen {
 		#address-cells = <1>;
 		#size-cells = <1>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index d8ebc1a84af9..4bdc48caf68a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -16,6 +16,12 @@ / {
 	#address-cells = <1>;
 	#size-cells = <1>;
 
+	aliases {
+		mmc0 = &mmc0;
+		mmc1 = &mmc1;
+		mmc2 = &mmc2;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.26.3

Re: [PATCH 0/2] sunxi: Enforce consistent MMC numbering

[Chen-Yu Tsai]

Hi,

On Mon, Apr 19, 2021 at 10:52 AM Samuel Holland <samuel@sholland.org> wrote:
>
> Dealing with the inconsistent numbering has been a major pain, and
> there is a solution with (as far as I can tell) no tangible downsides.
> So let's use it.
>
> Yes, I know the kernel supports UUIDs for root=. But UUIDs do not help
> when referencing the whole, unpartitioned device, like is needed for
> updating the bootloader and firmware. So for the use case of "write a
> bootloader to the SD card, regardless of where the board is currently
> booted from", I know of two options:
>   - Dig around in sysfs to find the mmc number from the MMIO address,
>     which means I have to know the MMIO addresses for every SoC, or
>   - Apply patches like these.
>
> Samuel Holland (2):
>   ARM: dts: sunxi: h3/h5: Enforce consistent MMC numbering
>   arm64: dts: allwinner: Enforce consistent MMC numbering
>
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            | 6 ++++++
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 6 ++++++
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 6 ++++++

At least with Rockchip this is now done at the board level. IIRC it was
a request from other people to not do it at the SoC level. I don't recall
exactly who though.

ChenYu


>  3 files changed, 18 insertions(+)
>
> --
> 2.26.3
>
>

Re: [PATCH 0/2] sunxi: Enforce consistent MMC numbering

[Andre Przywara]

On Mon, 19 Apr 2021 11:17:19 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

Hi,

> On Mon, Apr 19, 2021 at 10:52 AM Samuel Holland <samuel@sholland.org> wrote:
> >
> > Dealing with the inconsistent numbering has been a major pain, and
> > there is a solution with (as far as I can tell) no tangible downsides.
> > So let's use it.

Thanks Samuel for sending this!

> > Yes, I know the kernel supports UUIDs for root=. But UUIDs do not help
> > when referencing the whole, unpartitioned device, like is needed for
> > updating the bootloader and firmware. So for the use case of "write a
> > bootloader to the SD card, regardless of where the board is currently
> > booted from", I know of two options:
> >   - Dig around in sysfs to find the mmc number from the MMIO address,
> >     which means I have to know the MMIO addresses for every SoC, or
> >   - Apply patches like these.
> >
> > Samuel Holland (2):
> >   ARM: dts: sunxi: h3/h5: Enforce consistent MMC numbering
> >   arm64: dts: allwinner: Enforce consistent MMC numbering
> >
> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi            | 6 ++++++
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 6 ++++++
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 6 ++++++  
> 
> At least with Rockchip this is now done at the board level. IIRC it was
> a request from other people to not do it at the SoC level. I don't recall
> exactly who though.

FWIW, I am very much in favour of these patches, at a SoC level:
The *SoC* BootROM imposes an order, by probing the first (by MMIO
address order) MMC controller first for boot devices. IIRC that's a
different story for Rockchip?
And if people really don't care about the order, then having a certain
order doesn't hurt, so we could as well use the "natural" order, as it
was before.

Also UUIDs only help if you boot with an initramfs to resolve them,
which proves to be extra pain if you don't compile kernels on the
device, or not inside a distribution environment.


One thing I was wondering about though is that U-Boot currently defines
"mmc1 = &mmc2;", so that the eMMC is always MMC device 1. This is also
the BootROM probe order (MMC1 is never touched). I wonder if we should
mimic this here, since MMC2 is clearly the device for eMMC (due to
8-bit bus width support)? So from a block device perspective we would
have always have mmcblk0 and mmcblk1.
Otherwise it would conflict with U-Boot's current fixup[1], so
U-Boot's DT would either need to deviate or create boot script breakages
there.

Cheers,
Andre

[1]
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/sunxi-u-boot.dtsi#L14-16

Re: [PATCH 0/2] sunxi: Enforce consistent MMC numbering

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]

On Mon, Apr 19, 2021 at 09:54:43AM +0100, Andre Przywara wrote:
> On Mon, 19 Apr 2021 11:17:19 +0800
> Chen-Yu Tsai <wens@csie.org> wrote:
> 
> Hi,
> 
> > On Mon, Apr 19, 2021 at 10:52 AM Samuel Holland <samuel@sholland.org> wrote:
> > >
> > > Dealing with the inconsistent numbering has been a major pain, and
> > > there is a solution with (as far as I can tell) no tangible downsides.
> > > So let's use it.
> 
> Thanks Samuel for sending this!
> 
> > > Yes, I know the kernel supports UUIDs for root=. But UUIDs do not help
> > > when referencing the whole, unpartitioned device, like is needed for
> > > updating the bootloader and firmware. So for the use case of "write a
> > > bootloader to the SD card, regardless of where the board is currently
> > > booted from", I know of two options:
> > >   - Dig around in sysfs to find the mmc number from the MMIO address,
> > >     which means I have to know the MMIO addresses for every SoC, or
> > >   - Apply patches like these.
> > >
> > > Samuel Holland (2):
> > >   ARM: dts: sunxi: h3/h5: Enforce consistent MMC numbering
> > >   arm64: dts: allwinner: Enforce consistent MMC numbering
> > >
> > >  arch/arm/boot/dts/sunxi-h3-h5.dtsi            | 6 ++++++
> > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 6 ++++++
> > >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 6 ++++++  
> > 
> > At least with Rockchip this is now done at the board level. IIRC it was
> > a request from other people to not do it at the SoC level. I don't recall
> > exactly who though.
> 
> FWIW, I am very much in favour of these patches, at a SoC level:
> The *SoC* BootROM imposes an order, by probing the first (by MMIO
> address order) MMC controller first for boot devices. IIRC that's a
> different story for Rockchip?
> And if people really don't care about the order, then having a certain
> order doesn't hurt, so we could as well use the "natural" order, as it
> was before.

This doesn't have anything to do with the BootRom though but what we
provide to the userspace? The userspace has no guarantee about the
kernel enumeration order for any bus (but UART for some reason), I'm not
really sure why MMC would be an exception. Especially since the kernel
will not try to catch up, this will be bound to be broken on a regular
basis.

And that aside, assuming that a device only has an eMMC this would
create the mmc2 device, which is completely weird and inconsistent with
how any other bus behaves.

> Also UUIDs only help if you boot with an initramfs to resolve them,
> which proves to be extra pain if you don't compile kernels on the
> device, or not inside a distribution environment.

I'm not sure what you mean? The kernel is perfectly able to resolve
them. You can also use PARTLABEL if you want something more user
friendly.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/2] sunxi: Enforce consistent MMC numbering

[Andre Przywara]

On Fri, 30 Apr 2021 11:10:35 +0200
Maxime Ripard <maxime@cerno.tech> wrote:

Hi Maxime,

> On Mon, Apr 19, 2021 at 09:54:43AM +0100, Andre Przywara wrote:
> > On Mon, 19 Apr 2021 11:17:19 +0800
> > Chen-Yu Tsai <wens@csie.org> wrote:
> > 
> > Hi,
> >   
> > > On Mon, Apr 19, 2021 at 10:52 AM Samuel Holland <samuel@sholland.org> wrote:  
> > > >
> > > > Dealing with the inconsistent numbering has been a major pain, and
> > > > there is a solution with (as far as I can tell) no tangible downsides.
> > > > So let's use it.  
> > 
> > Thanks Samuel for sending this!
> >   
> > > > Yes, I know the kernel supports UUIDs for root=. But UUIDs do not help
> > > > when referencing the whole, unpartitioned device, like is needed for
> > > > updating the bootloader and firmware. So for the use case of "write a
> > > > bootloader to the SD card, regardless of where the board is currently
> > > > booted from", I know of two options:
> > > >   - Dig around in sysfs to find the mmc number from the MMIO address,
> > > >     which means I have to know the MMIO addresses for every SoC, or
> > > >   - Apply patches like these.
> > > >
> > > > Samuel Holland (2):
> > > >   ARM: dts: sunxi: h3/h5: Enforce consistent MMC numbering
> > > >   arm64: dts: allwinner: Enforce consistent MMC numbering
> > > >
> > > >  arch/arm/boot/dts/sunxi-h3-h5.dtsi            | 6 ++++++
> > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 6 ++++++
> > > >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 6 ++++++    
> > > 
> > > At least with Rockchip this is now done at the board level. IIRC it was
> > > a request from other people to not do it at the SoC level. I don't recall
> > > exactly who though.  
> > 
> > FWIW, I am very much in favour of these patches, at a SoC level:
> > The *SoC* BootROM imposes an order, by probing the first (by MMIO
> > address order) MMC controller first for boot devices. IIRC that's a
> > different story for Rockchip?
> > And if people really don't care about the order, then having a certain
> > order doesn't hurt, so we could as well use the "natural" order, as it
> > was before.  
> 
> This doesn't have anything to do with the BootRom though but what we
> provide to the userspace?

My argument was that there is a "natural" order, not only by the
location in the MMIO space, but on a more logical level: the "first" SD
controller is first checked by the BootROM, so the SoC puts a "number
0" tag on this particular one. This is just because I heard the
(reasonable) argument that location in MMIO space should not be
significant, and deriving an order from that is arbitrary.

> The userspace has no guarantee about the
> kernel enumeration order for any bus (but UART for some reason), I'm not
> really sure why MMC would be an exception.

Userspace is one thing, but arguably there are ways to find the
partition you are looking for, because you have all the kernel *and*
userlevel tools at hand. The question is whether this needs to be overly
complicated and harder for the user.
But the other, more pressing problem is the kernel-only code in
init/do_mounts.c, which has very limited means to find a
certain partition (see below).

> Especially since the kernel will not try to catch up, this will be
> bound to be broken on a regular basis.

What do you mean with "will not try to catch up"? Do you mean there
might be other kernel changes that spoil the numbering again, in the
future?

> And that aside, assuming that a device only has an eMMC this would
> create the mmc2 device, which is completely weird and inconsistent
> with how any other bus behaves.

You have a point there, but I don't see how this would be a problem. I
think one advantage is *consistent* numbering across reboots, so it
will always be mmc2, even if you add a WiFi chip later (some boards
have that as an option). Also those SD-card less system could surely
override the labels in their board .dts files?

> > Also UUIDs only help if you boot with an initramfs to resolve them,
> > which proves to be extra pain if you don't compile kernels on the
> > device, or not inside a distribution environment.  
> 
> I'm not sure what you mean? The kernel is perfectly able to resolve
> them. You can also use PARTLABEL if you want something more user
> friendly.

init/do_mounts.c supports PARTUUID only, but most people/distributions
use UUID, which is filesystem metadata, so not easily accessible for
the kernel early and on a partition level only. And IIRC PARTLABEL is
only available on GPT partitioned disks.
Also PARTUUID is set at partition time, so a user formatting her own SD
card (or just changing the partition table) will have a unique PARTUUID,
and it cannot be used from within a filesystem image or
prefabricated boot script to reference its own partition
(in /etc/fstab).
And in general UUIDs are not very human friendly, so their main domain
is to be handled by code. I definitely don't memorise a single one of
them, but I do remember root=/dev/mmcblk0p2.

I find it *very* annoying to load my development kernel over TFTP, then
booting them with the rootfs from SD card or eMMC, and seeing it fail
about 50% of the time, because the numbering is off.
 
Cheers,
Andre

Re: [PATCH 0/2] sunxi: Enforce consistent MMC numbering

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 7310 bytes --]

On Fri, Apr 30, 2021 at 11:55:45AM +0100, Andre Przywara wrote:
> On Fri, 30 Apr 2021 11:10:35 +0200
> Maxime Ripard <maxime@cerno.tech> wrote:
> 
> Hi Maxime,
> 
> > On Mon, Apr 19, 2021 at 09:54:43AM +0100, Andre Przywara wrote:
> > > On Mon, 19 Apr 2021 11:17:19 +0800
> > > Chen-Yu Tsai <wens@csie.org> wrote:
> > > 
> > > Hi,
> > >   
> > > > On Mon, Apr 19, 2021 at 10:52 AM Samuel Holland <samuel@sholland.org> wrote:  
> > > > >
> > > > > Dealing with the inconsistent numbering has been a major pain, and
> > > > > there is a solution with (as far as I can tell) no tangible downsides.
> > > > > So let's use it.  
> > > 
> > > Thanks Samuel for sending this!
> > >   
> > > > > Yes, I know the kernel supports UUIDs for root=. But UUIDs do not help
> > > > > when referencing the whole, unpartitioned device, like is needed for
> > > > > updating the bootloader and firmware. So for the use case of "write a
> > > > > bootloader to the SD card, regardless of where the board is currently
> > > > > booted from", I know of two options:
> > > > >   - Dig around in sysfs to find the mmc number from the MMIO address,
> > > > >     which means I have to know the MMIO addresses for every SoC, or
> > > > >   - Apply patches like these.
> > > > >
> > > > > Samuel Holland (2):
> > > > >   ARM: dts: sunxi: h3/h5: Enforce consistent MMC numbering
> > > > >   arm64: dts: allwinner: Enforce consistent MMC numbering
> > > > >
> > > > >  arch/arm/boot/dts/sunxi-h3-h5.dtsi            | 6 ++++++
> > > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 6 ++++++
> > > > >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 6 ++++++    
> > > > 
> > > > At least with Rockchip this is now done at the board level. IIRC it was
> > > > a request from other people to not do it at the SoC level. I don't recall
> > > > exactly who though.  
> > > 
> > > FWIW, I am very much in favour of these patches, at a SoC level:
> > > The *SoC* BootROM imposes an order, by probing the first (by MMIO
> > > address order) MMC controller first for boot devices. IIRC that's a
> > > different story for Rockchip?
> > > And if people really don't care about the order, then having a certain
> > > order doesn't hurt, so we could as well use the "natural" order, as it
> > > was before.  
> > 
> > This doesn't have anything to do with the BootRom though but what we
> > provide to the userspace?
> 
> My argument was that there is a "natural" order, not only by the
> location in the MMIO space, but on a more logical level: the "first" SD
> controller is first checked by the BootROM, so the SoC puts a "number
> 0" tag on this particular one. This is just because I heard the
> (reasonable) argument that location in MMIO space should not be
> significant, and deriving an order from that is arbitrary.

Unfortunately, the bootrom order is just as arbitrary. It's even worse
since it's inconsistent across SoCs. The A31 has that boot order:
https://linux-sunxi.org/BROM#A31

Do you really expect that we have a different ordering based on the
state of these pins for a given boot?

I know you don't :) And it's my point, really. There's no logical order
that is common to all the SoCs and all the buses. So there's no point in
even trying, the userspace and installers should just acknowledge that.

> > The userspace has no guarantee about the
> > kernel enumeration order for any bus (but UART for some reason), I'm not
> > really sure why MMC would be an exception.
> 
> Userspace is one thing, but arguably there are ways to find the
> partition you are looking for, because you have all the kernel *and*
> userlevel tools at hand. The question is whether this needs to be overly
> complicated and harder for the user.
> But the other, more pressing problem is the kernel-only code in
> init/do_mounts.c, which has very limited means to find a
> certain partition (see below).

To some extent it's still a userspace issue: I never heard that
discussion for a server or desktop, even though they very much have the
same problem. It's not for those systems because they rely on UUIDs
and/or an initramfs to address it.

If you want to build a system that is generic enough that PARTLABEL or
PARTUUID doesn't work, it should be the preferred solution as well.

> > Especially since the kernel will not try to catch up, this will be
> > bound to be broken on a regular basis.
> 
> What do you mean with "will not try to catch up"? Do you mean there
> might be other kernel changes that spoil the numbering again, in the
> future?

I mean, the kernel doesn't even try. It never really did either.

We have systems where even an aliases based solution wouldn't work
today, like the kettlepop (a CHIP with an eMMC) that has an eMMC but no
SD card by default, but an extension card exists that has this
capability.

The natural order without the extension would be mmc0 for the eMMC, but
obviously that would be mmc1 if the mmc0 controller is enabled, right?

> > And that aside, assuming that a device only has an eMMC this would
> > create the mmc2 device, which is completely weird and inconsistent
> > with how any other bus behaves.
> 
> You have a point there, but I don't see how this would be a problem. I
> think one advantage is *consistent* numbering across reboots, so it
> will always be mmc2, even if you add a WiFi chip later (some boards
> have that as an option). Also those SD-card less system could surely
> override the labels in their board .dts files?

This is inconsistent with how any bus and devices behave in Linux. You
plug a USB key, the first one is going to be called sda. The first
graphics card is going to be card0, etc.

I don't see why we should make an exception for MMC buses on "embedded"
devices specifically. Or if we do, I want the embedded exception for a
few other things ;)

> > > Also UUIDs only help if you boot with an initramfs to resolve them,
> > > which proves to be extra pain if you don't compile kernels on the
> > > device, or not inside a distribution environment.  
> > 
> > I'm not sure what you mean? The kernel is perfectly able to resolve
> > them. You can also use PARTLABEL if you want something more user
> > friendly.
> 
> init/do_mounts.c supports PARTUUID only, but most people/distributions
> use UUID, which is filesystem metadata, so not easily accessible for
> the kernel early and on a partition level only. And IIRC PARTLABEL is
> only available on GPT partitioned disks.
> Also PARTUUID is set at partition time, so a user formatting her own SD
> card (or just changing the partition table) will have a unique PARTUUID,
> and it cannot be used from within a filesystem image or
> prefabricated boot script to reference its own partition
> (in /etc/fstab).
> And in general UUIDs are not very human friendly, so their main domain
> is to be handled by code. I definitely don't memorise a single one of
> them, but I do remember root=/dev/mmcblk0p2.
> 
> I find it *very* annoying to load my development kernel over TFTP, then
> booting them with the rootfs from SD card or eMMC, and seeing it fail
> about 50% of the time, because the numbering is off.

Yeah, I know, it's annoying to me too.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]