Messages in this thread | | | Date | Wed, 18 Oct 2023 09:56:14 +0200 | Subject | Re: [PATCH v1 1/1] Revert "pinctrl: avoid unsafe code pattern in find_pinctrl()" | From | Ferry Toth <> |
| |
Hi,
(resend due to html reject)
On 17-10-2023 23:43, Dmitry Torokhov wrote: > Hi Andy, > > On Tue, Oct 17, 2023 at 10:45:39PM +0300, Andy Shevchenko wrote: >> On Tue, Oct 17, 2023 at 08:59:05PM +0200, Linus Walleij wrote: >>> On Tue, Oct 17, 2023 at 8:34 PM Andy Shevchenko >>> <andriy.shevchenko@linux.intel.com> wrote: >>>> On Tue, Oct 17, 2023 at 08:18:23PM +0200, Linus Walleij wrote: >>> >>>>> 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. >>> >>> Actually, I don't agree with that, just relaying it. I would prefer that we >>> solve exactly the problem that we are facing here: some random unrelated >>> code or similar affecting enumeration order of mmc devices. > > Sorry, but the era of static configuration where one has a well defined > order in which things are probed and numbered has long gone. The right > answer is either device aliases that provides stable numbering on a > board that is not dependent on scheduler behavior, mutexes > implementation (how they deal with writer starvation, etc), > kernel/driver/subsystem linking order and myriad other things, or > mounting by UUID. The kernel does not provide any guarantees on the > stability of device probe and instantiation order. > > If you think about it it is the same issue as legacy GPIO numbering. > It was convenient some time ago, but now it is no longer suitable or > sufficient and could change when kernel is uprevved. > >>> >>> It's not the first time it happens to me, I have several devices that change >>> this enumeration order depending on whether an SD card is plugged >>> in or not, and in a *BIG* way: the boot partition on the soldered eMMC >>> changes enumeration depending on whether an SD card is inserted >>> or not, and that has never been fixed (because above). >> >> This is not the problem I have. I haven't added any SD card, hardware >> configuration is the same. The solely difference in the whole setup is >> this revert applied or not. > > Yes, I guess there is a contention on this mutex and the fact that we > are now taking it once and not twice makes difference in which probes > happen. If you look at the logs, you will see that even before the patch > controllers did not enumerate on the order of PCI functions: > > [ 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
You are referring to the order printed in dmesg. But actually
mmc0 = 0000:00:01.0 mmc1 = 0000:00:01.2 mmc2 = 0000:00:01.3
And this has been so for like 8 years. See f.i. https://github.com/edison-fw/meta-intel-edison/issues/135 (this is with Yocto, so using systemd, the issue discussed there is not related to this but to card detection iirc)
> So you have mmc2 instantiated before mmc1 even before the patch. This > happens because we now have > > .probe_type = PROBE_PREFER_ASYNCHRONOUS, > > in sdhci_driver structure in drivers/mmc/host/sdhci-pci-core.c. It just > happened that even with asynchronous probing your storage did end up on > mmc0 originally and you were happy. > > I wonder, could you please post entire dmesg for your system? > >> >>>>> 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. > > Rob might not want them, but they are the reality and are present for > multiple classes of devices and I believe are here to stay. > >>>> >>>>> 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? >>> >>> Because device properties are not device tree, they are just some >>> Linux thing so we can do whatever we want. Just checking if >>> Dmitry has some idea that would solve this for good, he usually >>> replies quickly. >> >> OK. > > I think the right answer is "fix the userspace" really in this case. We > could also try extend of_alias_get_id() to see if we could pass some > preferred numbering on x86. But this will again be fragile if the > knowledge resides in the driver and is not tied to a particular board > (as it is in DT case): there could be multiple controllers, things will > be shifting board to board... > > Thanks. >
| |