Re: [PATCH v1 1/1] Revert "pinctrl: avoid unsafe code pattern in find_pinctrl()"

[Andy Shevchenko]

On Tue, Oct 17, 2023 at 08:18:23PM +0200, Linus Walleij wrote:
> On Tue, Oct 17, 2023 at 4:18 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> 
> > The commit breaks MMC enumeration on the Intel Merrifield
> > plaform.
> 
> The enumeration works, just that the probe order is different, right?
> 
> > Before:
> > [   36.439057] mmc0: SDHCI controller on PCI [0000:00:01.0] using ADMA
> > [   36.450924] mmc2: SDHCI controller on PCI [0000:00:01.3] using ADMA
> > [   36.459355] mmc1: SDHCI controller on PCI [0000:00:01.2] using ADMA
> > [   36.706399] mmc0: new DDR MMC card at address 0001
> > [   37.058972] mmc2: new ultra high speed DDR50 SDIO card at address 0001
> > [   37.278977] mmcblk0: mmc0:0001 H4G1d 3.64 GiB
> > [   37.297300]  mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10
> >
> > After:
> > [   36.436704] mmc2: SDHCI controller on PCI [0000:00:01.3] using ADMA
> > [   36.436720] mmc1: SDHCI controller on PCI [0000:00:01.0] using ADMA
> > [   36.463685] mmc0: SDHCI controller on PCI [0000:00:01.2] using ADMA
> > [   36.720627] mmc1: new DDR MMC card at address 0001
> > [   37.068181] mmc2: new ultra high speed DDR50 SDIO card at address 0001
> > [   37.279998] mmcblk1: mmc1:0001 H4G1d 3.64 GiB
> > [   37.302670]  mmcblk1: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10
> >
> > This reverts commit c153a4edff6ab01370fcac8e46f9c89cca1060c2.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> 
> Relying on this probe order or whatever it is causing one or the other
> to be enumerated first seems very fragile, I think this condition can be
> caused by other much more random things in the probe path as well,
> so it would be great if we could just hammer this down for good, as
> it is apparently ABI.
> 
> In the past some file system developers have told us (Ulf will know)
> that we can't rely on the block device enumeration to identify
> devices, and requires that we use things such as sysfs or the
> UUID volume label in ext4 to identify storage.

While I technically might agree with you, this was working for everybody
since day 1 of support of Intel Merrifield added (circa v4.8), now _user
space_ is broken.

Note, I'm having _simple_ setup, no fancy UDEV or DBUS there, and I want
my scripts simply continue working. As I mentioned, this is Buildroot
+ Busybox which I haven't touched in the area of how they treat MMC
devices in _user space_.

Since we are at rc6 I prefer to get this reverted first and next cycle we can
discuss better solutions. I'm all for testing any.

> That said, device trees are full of stuff like this:
> 
>         aliases {
>                 serial0 = &uart_AO;
>                 mmc0 = &sd_card_slot;
>                 mmc1 = &sdhc;
>         };

And Rob, AFAIU, is against aliases.

> Notice how this enumeration gets defined by the aliases.
> 
> Can you do the same with device properties? (If anyone can
> answer that question it's Dmitry!)

No, and why should we?

-- 
With Best Regards,
Andy Shevchenko