Re: [PATCH] arm: stm32mp15x: Move mmc aliases to board files

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2426 bytes --]

Hello Ahmad,

On Mon, Mar 07, 2022 at 12:34:19PM +0100, Ahmad Fatoum wrote:
> On 07.03.22 12:23, Uwe Kleine-König wrote:
> > Having the mmc aliases in stm32mp151.dtsi is surprising as depending on
> > the order of includes these override the mmc ordering in
> > <arm/stm32mp157c-myboard.dts>. Also the ordering of mmc devices is actually
> > board specific, so it's also right to have this in the board.dts files.
> 
> NACK.

I feared you'd oppose. :-\

My motivation to deviate from the default ordering is that I want to
have eMMC as first device and SD as second, so on the board I have here
I'd like to have

	mmc0 = &sdmmc2; /* eMMC */
	mmc1 = &sdmmc1; /* µSD */

However in combination with arch/arm/dts/stm32mp151.dtsi this results in

	mmc0 = &sdmmc2; /* eMMC */
	mmc1 = &sdmmc1; /* µSD */
	mmc2 = &sdmmc3;

which is a bit ugly because sdmmc3 isn't used at all on the board in
question. Doing a /delete-property/mmc2 isn't that nice, too. Open for
alternatives ...

> MMC IPs have fixed numbering, because TF-A (and BootROM before it)
> report to barebox the number of the MMC device that it succesfully booted from.
> The aliases map these IDs to device tree nodes, so barebox can fix up a correct
> /chosen/bootsource.

And mapping the number from TF-A (or BootROM) depends on the aliases
defined in the dts? Sounds like a bug to me.

> Additionally having any alias at all ensures fixed naming that's
> not dependent on probe order.

Fine for me. And if the board doesn't define the aliases, you get random
ordering.

> I know that Linux maintainers seem to disagree with this, but as far
> as barebox is concerned, aliases are SoC-specific, not board-specific
> in general. You can override this board-level if you like, but the
> default should remain.
> 
> > There is no (relevant) change intended by this patch.
> 
> I have non-upstream boards that would be broken by this.

This is not a reason to not take this patch, is it?

> There's also a Phytec board in next that would be broken by this.
> Whereas they had a fixed mmcX before, they would now have disk0, disk1
> or disk2 depending on probe order with this patch applied.

Agreed, code in next should be adapted.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox