mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH] arm: stm32mp15x: Move mmc aliases to board files
Date: Mon, 7 Mar 2022 14:23:52 +0100	[thread overview]
Message-ID: <20220307132352.nimuo7vofgb3mhtd@pengutronix.de> (raw)
In-Reply-To: <5bec70f6-8963-2b79-9f81-3e0a3dd30b7c@pengutronix.de>


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

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 #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

  reply	other threads:[~2022-03-07 13:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-07 11:23 Uwe Kleine-König
2022-03-07 11:34 ` Ahmad Fatoum
2022-03-07 12:04   ` Uwe Kleine-König
2022-03-07 12:19     ` Ahmad Fatoum
2022-03-07 13:23       ` Uwe Kleine-König [this message]
2022-03-11  9:31         ` Ahmad Fatoum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220307132352.nimuo7vofgb3mhtd@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox