linux-kernel - Re: [PATCH v3 1/5] PM: sleep: Resume children after resuming the parent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJZ5v0jSr1t=bEkOO9HJKmxFVf4oY+ij77aHsBeKgL+NpDfmnQ@mail.gmail.com>
Date: Thu, 8 May 2025 20:06:30 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Jon Hunter <jonathanh@...dia.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, "Rafael J. Wysocki" <rjw@...ysocki.net>, 
	Linux PM <linux-pm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Alan Stern <stern@...land.harvard.edu>, Ulf Hansson <ulf.hansson@...aro.org>, 
	Johan Hovold <johan@...nel.org>, Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, 
	Saravana Kannan <saravanak@...gle.com>, 
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH v3 1/5] PM: sleep: Resume children after resuming the parent

On Thu, May 8, 2025 at 3:38 PM Jon Hunter <jonathanh@...dia.com> wrote:
>
>
> On 07/05/2025 17:43, Rafael J. Wysocki wrote:
>
> ...
>
> >> Setting all these to 'disabled' fixes the problem. However, also just
> >> setting the 'cypd4226' device to 'sync' fixes the problem (the ina3221
> >> devices seem to be fine being async). The 'cypd4226' device is
> >> interesting, because this one is a USB Type-C controller and there is a
> >> circular dependency between the Type-C and USB PHY (see
> >> arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts).
> >
> > Circular dependencies are problematic for suspend/resume in general.
> > I wonder if fw_devlink can resolve this?
>
> I booted with fw_devlink=on, but this did not resolve the problem :-(

I see, but thanks for checking.

> >> If I make the following change then this does fix it ...
> >>
> >> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> >> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> >> index f01e4ef6619d..e9a9df1431af 100644
> >> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> >> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> >> @@ -1483,6 +1483,8 @@ static int ucsi_ccg_probe(struct i2c_client *client)
> >>
> >>           i2c_set_clientdata(client, uc);
> >>
> >> +       device_disable_async_suspend(uc->dev);
> >> +
> >>           pm_runtime_set_active(uc->dev);
> >>           pm_runtime_enable(uc->dev);
> >>           pm_runtime_use_autosuspend(uc->dev);
> >>
> >> Is this the right fix for this?
> >
> > At least as a stop-gap, yes.
> >
> > In order to enable async suspend for a device, one needs to at least
> > assume with sufficiently high confidence that it will be safe to
> > reorder it with respect to any other device in the system except for
> > the devices having known dependencies on the device in question.
> > Those known dependencies either are parent-child connections or they
> > need to be represented by device links.
> >
> > In this particular case, it is painfully clear that the suspend of the
> > device in question cannot be reordered with respect to at least one
> > other device where the dependency is not known in the above sense.
> >
> > Thus the device in question should not be allowed to suspend asynchronously.
> >
> > Would it be better to represent the dependency in question via a
> > device link?  Yes, it would, but until that happens, disabling async
> > suspend is the right thing to do IMV.
>
> OK so it is not clear to me if the PM core should be handling this for
> us.

No, it can't because dependency information is missing.

> Is that what you are implying/suggesting?

No.

Async suspend needs to be disabled for this device for now I'm afraid
and I don't think that it will make suspend/resume take much more
time.

Thanks!