[...] > > > >>>> > > > >>>> However, if the serial port's clock or PM Domain is shared with another > > > >>>> device, and that other device is runtime-suspended before the full > > > >>>> serial driver has probed, the serial port's clock and/or PM Domain will > > > >>>> be disabled inadvertently. Any subsequent serial console output will > > > >>>> cause a crash or system lock-up. E.g. on R/SH-Mobile SoCs, the serial > > > >>>> ports share their PM Domain with several other I/O devices. After the > > > >>>> use of pwm (Armadillo-800-EVA) or i2c (KZM-A9-GT) during early boot, > > > >>>> before the full serial driver takes over, the PM Domain containing the > > > >>>> early serial port is powered down, causing a lock-up when booted with > > > >>>> "earlycon". > > Let's call this "Case B". > > > > >>> > > > >>> Thanks for the detailed description of the problem! As pointed out in > > > >>> regards to another similar recent patch [1], this is indeed a generic > > > >>> problem, not limited to the serial console handling. > > > >>> > > > >>> At Linaro Connect a few weeks ago I followed up with Saravana from the > > > >>> earlier discussions at LPC last fall. We now have a generic solution > > > >>> for genpd drafted on plain paper, based on fw_devlink and the > > > >>> ->sync_state() callback. I am currently working on the genpd series, > > > >>> while Saravana will re-spin the series (can't find the link to the > > > >>> last version) for the clock framework. Ideally, we want these things > > > >>> to work in a very similar way. > > > >>> > > > >>> That said, allow me to post the series for genpd in a week or two to > > > >>> see if it can solve your problem too, for the serial console. I managed to hit the vacation period before I was able to post the series. I will pick it up this week and hopefully should be able to post something next week. > > > >> > > > >> Both the genpd and the clock solutions will make suppliers depend on all > > > >> their consumers to be probed, right? > > > >> > > > >> I think it is a solution, and should be worked on, but it has the > > > >> drawback that suppliers that have consumers that will possibly never be > > > >> probed, will also never be able to turn off unused resources. > > > >> > > > >> This was specifically the case with the TI ti-sci pmdomain case I was > > > >> looking at: the genpd driver (ti_sci_pm_domains.c) provides a lot of > > > >> genpds for totally unrelated devices, and so if, e.g., you don't have or > > > >> don't want to load a driver for the GPU, all PDs are affected. > > > >> > > > >> Even here the solutions you mention will help: instead of things getting > > > >> broken because genpds get turned off while they are actually in use, the > > > >> genpds will be kept enabled, thus fixing the breakage. Unfortunately, > > > >> they'll be kept enabled forever. > > > >> > > > >> I've been ill for quite a while so I haven't had the chance to look at > > > >> this more, but before that I was hacking around a bit with something I > > > >> named .partial_sync_state(). .sync_state() gets called when all the > > > >> consumers have probed, but .partial_sync_state() gets called when _a_ > > > >> consumer has been probed. > > > >> > > > >> For the .sync_state() things are easy for the driver, as it knows > > > >> everything related has been probed, but for .partial_sync_state() the > > > >> driver needs to track resources internally. .partial_sync_state() will > > > >> tell the driver that a consumer device has probed, the driver can then > > > >> find out which specific resources (genpds in my case) that consumer > > > >> refers to, and then... Well, that's how far I got with my hacks =). > > > >> > > > >> So, I don't know if this .partial_sync_state() can even work, but I > > > >> think we do need something more on top of the .sync_state(). > > > > > > > > Thanks for the update! > > > > > > > > You certainly have a point, but rather than implementing some platform > > > > specific method, I think we should be able enforce the call to > > > > ->sync_state(), based upon some condition/timeout - and even if all > > > > consumers haven't been probed. > > > > > > Hmm, I think that was already implemented in some of the serieses out > > > there (or even in mainline already?), as I remember doing some > > > experiments with it. I don't like it much, though. > > > > > > With a simple timeout, it'll always be just a bit too early for some > > > user (nfs mount took a bit more time than expected -> board frozen). > > > > > > The only condition I can see that would somewhat work is a manual > > > trigger from the userspace. The boot scripts could then signal the > > > kernel when all the modules have been loaded and probably a suitable, > > > platform/use case specific amount of time has passed to allow the > > > drivers to probe. > > > > This is also already supported in mainline. > > > > Devices with sync_state() implementations (once Ulf adds it) will have > > a state_synced file in sysfs. It shows where it has been called yet or > > not. But you can also echo 1 into it to force the sync_state() > > callback (only if it hasn't been called already). So, yeah, all > > methods of handling this are available if you implement the > > sync_state() callback. > > > > By default it's all strict (wait till all consumers probe > > successfully). But you can set it to timeout (fw_devlink.sync_state). > > And you also have the option I mentioned above that you can use with > > both cases. > > So the idea is to disable unused genpds and clocks from the genpd > resp. clock's driver .sync_state() callback, instead of from a late > initcall? That would indeed solve issues related to "Case A". > > However, how to solve "Case B"? Ignore disabling genpds or clocks > before .sync_state() callback() has been called? > That would cause issues for cases where the clock must be disabled, > cfr. > "[PATCH RFC 0/3] Add clk_disable_unprepare_sync()" > https://lore.kernel.org/all/20240131160947.96171-1-biju.das.jz@xxxxxxxxxxxxxx/ > "[PATCH v3 0/3] Add clk_poll_disable_unprepare()" > https://lore.kernel.org/linux-renesas-soc/20240318110842.41956-1-biju.das.jz@xxxxxxxxxxxxxx/ > For genpd, the plan is to check the initial state of the PM domain. It can be powered-on or powered-off and if it's powered-on, we should not allow it to be powered-off until after ->sync_state() have been called. The similar approach is what Saravanna is trying to implement for clocks, I think. In the end, we simply need to try out these approaches to see if they really work. Although, based on previous discussions (LKML + F2F-conferences), I think there should be a good chance for us. [...] Kind regards Uffe
