Hello Ahmad, On Mon, Mar 07, 2022 at 01:19:27PM +0100, Ahmad Fatoum wrote: > On 07.03.22 13:04, Uwe Kleine-König wrote: > > 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 ... > > Why though? Is this for compatibility with older/other hardware? It's for compatibility with developer's mind, where numbering starts with the always available devices :-) > Renumbering is what I'd suggest though. > > >> 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. > > We can change of_alias_get_id to lookup /aliases/barebox,$alias as a fallback. > Then add a new function that looks up barebox alias first and then non-barebox > adorned as fallback and use that for bootsource calculation. Afterwards, we can > prefix SoC level MMC aliases with barebox, and boards can specify their > aliases as they please. This may be the most straight-forward way > to decouple. Having mapping tables in SoC support is not my favorite. I would have gone with the mapping tables and I'd consider /aliases/barebox,mmc0 = ... more ugly. But I agree this to be a bit subjective. If the soc-code continues to depend on having the mmc aliases as defined in stm32mp151.dtsi, there should be a comment describing that IMHO. > >> 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 prefer sane defaults. I prefer sane defaults iff they can be easily adapted by board code. If you consider in board.dts: mmc0 = &sdmmc2; mmc1 = &sdmmc3; (for whatever reasons), you end up with mmc1 and mmc2 both pointing to sdmmc3. Ugly. > >> 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? > > I think most people involved have boards (often with trivial board support) > that are not upstream. I do think we should avoid breaking them for no good > reason. I think these people should mainline their trivial board support if they want to be immune to such breaking. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox
