+ Saravana On Mon, 23 Nov 2020 at 14:09, Jon Hunter <jonathanh@xxxxxxxxxx> wrote: > > Hi Ulf, > > On 19/11/2020 10:15, Ulf Hansson wrote: > > On Mon, 16 Nov 2020 at 17:17, Jon Hunter <jonathanh@xxxxxxxxxx> wrote: > >> > >> Hi all, > >> > >> We recently ran into a problem on Tegra186 where it was failing to > >> resume from suspend. It turned out that a driver, the Tegra ACONNECT > >> (drivers/bus/tegra-aconnect.c), was being resumed before the PM domain > >> provider, the BPMP (drivers/firmware/tegra/bpmp.c), and the Tegra > >> ACONNECT was trying to enable the PM domain before the provider had been > >> resumed. > >> > >> According to commit 4d23a5e84806 it states that 'genpd powers on the PM > >> domain unconditionally in the system PM resume "noirq" phase'. However, > >> what I don't see is anything that guarantees that the provider is > >> resumed before any device that requires power domains. Unless there is > >> something that I am missing? > > > > The genpd provider's ->power_on() callback should be invoked as soon > > as an attached device gets resumed via the ->resume_noirq() callback > > (genpd_resume_noirq). Have you verified that this is working as > > expected for you? > > Yes this is working as expected. The problem is that the ->power_on > callback for a device is occurring before the provider itself has been > resumed. I see. > > > Note that, if there is no device attached to the genpd, the > > ->power_on() callback may not be invoked - unless there is a child > > domain being powered on. > > > > From the genpd provider driver point of view - why do you need to > > implement system suspend/resume callbacks at all? Do you have some > > additional operations to run, besides those executed from the > > ->power_on|off() callbacks? > > The provider in this case is an embedded controller, the BPMP, and it > needs to be resumed [0] prior to calling the provider callbacks. I am > wondering if any other providers have this requirement? It seems like it should be a rather common requirement for a genpd provider - at least for those providers that need to run some additional operations at system suspend/resume. I guess the reason for this problem is that the order of how the devices end up in the dpm_list, doesn't fit well for your case. Normally, a provider should be registered prior and the consumer, as that would probably lead to that the provider becomes resumed first. In any case, to make sure the order becomes correct, a device link should be created between the genpd domain provider and the consumer device. As a matter of fact, this is done "automagically" during boot for DT based platforms, see of_link_property() in drivers/of/property.c. Currently these device links are created with DL_FLAG_SYNC_STATE_ONLY, unless the "fw_devlink" kernel command line specifies a different option (on == DL_FLAG_AUTOPROBE_CONSUMER). I would try to play with that and see how that turns out. > > Thanks > Jon > > [0] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/firmware/tegra/bpmp.c#n797 Kind regards Uffe
