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] [thread-next>] [day] [month] [year] [list]
Message-ID: <0f7a3738-0b94-4361-a277-c3a90d87aebc@nvidia.com>
Date: Thu, 8 May 2025 14:38:19 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: "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 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 :-(

>> 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. Is that what you are implying/suggesting?

Thanks
Jon

-- 
nvpublic


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ