Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Ulf Hansson]

On Tue, 15 Dec 2020 at 09:19, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Ulf,
>
> On Mon, Dec 14, 2020 at 9:23 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > On Tue, 8 Dec 2020 at 08:32, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > On Mon, Dec 7, 2020 at 11:15 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
> > > > On Mon, Dec 7, 2020 at 1:55 PM Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
> > > > > On Mon, Dec 7, 2020 at 9:23 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > > > > On Tue, Dec 1, 2020 at 3:06 PM Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
> > > > > > > On Tue, Dec 1, 2020 at 12:39 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > > > > > > > So, I think we have two options. If people are willing to move to
> > > > > > > > "disk labels" or to patch their DTBs with mmc aliases, things can stay
> > > > > > > > as is. Otherwise, we can revert the async probe parts of the mmc host
> > > > > > > > drivers, but that would still leave us in a fragile situation.
> > > > > > >
> > > > > > > Can you reliably detect whether the mmc aliases in the dt exist?
> > > > > > > If that's possible, maybe the async flag could be masked out to only have
> > > > > > > an effect when the device number is known.
> > > > > >
> > > > > > IMHO DT aliases are not a proper solution for this.
> > > > > >
> > > > > > Yes, you can detect reliably if an alias exists in the DT.
> > > > > > The problems start when having multiple devices, some with aliases,
> > > > > > some without.  And when devices can appear dynamically (without
> > > > > > aliases, as there is no support for dynamically updating the aliases
> > > > > > list).
> > > > >
> > > > > Actually you hit a problem earlier than that: the async probe is a
> > > > > property of the host controller driver, which may be a pci_driver,
> > > > > platform_driver, usb_driver, or anything else really. To figure out
> > > > > whether to probe it asynchronously, it would have to be the driver
> > > > > core, or each bus type that can host these to understand which
> > > > > device driver is responsible for probing an eMMC device attached
> > > > > to the host.
> > > >
> > > > From what I've seen so far, my current thought on this issue is that
> > > > it's up to Ulf as the MMC maintainer what the next steps are.  For me,
> > > > at least, his argument that MMC block numbers have already shuffled
> > > > around several times in the last several years is at least some
> > > > evidence that they weren't exactly stable to begin with.  While we
> > > > could go back to the numbers that happened to be chosen as of kernel
> > > > v5.9, if someone was updating from a much older kernel then they may
> > > > have different expectations of what numbers are good / bad I think.
> > > >
> > > > I will also offer one possible suggestion: what about a KConfig option
> > > > here?  In theory we could add a KConfig option like
> > > > "CONFIG_MMC_LEGACY_PROBE" or something that.  One can argue about what
> > > > the default ought to be, but maybe that would satisfy folks?  If you
> > > > were happy giving up a little bit of boot speed to get the v5.9 block
> > > > numbers then you could set this.
> > >
> > > This is not limited to MMC.
> > > The same is true for sdX, ethX (e* these days), ttyS*, i2cX, spiX, ...
> > > The rule has always been to handle it by udev, disklabels, ...
> >
> > That's right.
> >
> > Although, those rules haven't been sufficient for some cases, which
> > has been discussed many many times by now. I can dig out some of the
> > old threads from the mail archive, just tell me and will help to find
> > them.
> >
> > For mmc we have decided to improve the situation by adding support for
> > aliases in DT. The support seems robust enough to me, but if you think
> > there are problems with it, please tell me and I am happy to help to
> > improve it.
>
> DT rule #1: DT is hardware description (device naming is software
> policy).

I do admit, the use of aliases in DT, for *all* cases including mmc is
a bit of a stretch. Anyway, it's there.

>
> The only generally accepted aliases are "serial0" (the first serial
> port, as such labeled on the board, to be used for the console;  there
> may be more labeled ports, and thus more "serialX" aliases), and
> "ethernet0" (the first Ethernet port, so U-Boot knows to which port to
> add an appropriate "local-mac-address" property, when booting over the
> network).  So yeah, you can claim the first SD card slot is labeled as
> such, and thus deserves an alias.  Then the issue is what you do with
> the remaining slots, which can be added either statically or
> dynamically.  And what if for some reason the labeled MMC slot is not
> probed first...

The probe order shouldn't matter. By pre-parsing all mmc aliases (for
all slots) existing in the DTB we can keep track of reserved ids. In
this way, for the non-alias case, we can dynamically select a
non-reserved id.

Although, perhaps more improvements can be made.

>
> The description of commit 7678f4c20fa7670f ("serial: sh-sci: Add support
> for dynamic instances") mentions some background and remaining
> issues w.r.t. aliases.
>
> > In regards to adding a new Kconfig option for "legacy probe", I am
> > open to this if that's what people think is needed. Although, as
> > pointed out earlier in this thread, it won't move us into a stable
> > situation. The only solution to get to that point, is either to use
> > udev/disklabel rules or the mmc aliases in DT.
>
> IMHO that will be a fragile solution, too: over time, it may become
> harder and harder to maintain the original probe order.
>
> And if it will be an option, it means there will be two code paths to
> maintain, increasing the burden.

Alright, sounds like we can conclude that the Kconfig option doesn't
seem like a good way forward.

Kind regards
Uffe