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

[Rafael J. Wysocki]

On Wed, May 7, 2025 at 5:40 PM Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>
>
>
> On 07/05/2025 15:56, Rafael J. Wysocki wrote:
>
> ...
>
> > So apparently one of the children has not been suspended yet when this
> > happens.  That's fine because it should be suspended at one point and
> > the parent suspend should be unblocked, so it looks like the child
> > suspend doesn't complete for some reason.
> >
> >> I will enable the PM_ADVANCED_DEBUG and confirm that making the I2C
> >> itself non-async works.
> >
> > What probably happens is that after the "PM: sleep: Suspend async
> > parents after suspending children" , the i2c clients are suspended
> > upfront (because they have no children) and when one of them has
> > suspended, it triggers a parent suspend.  The parent suspend then
> > waits for the other client to complete suspending, but that cannot
> > make progress for some reason.
> >
> > Before that patch, the i2c clients would have suspended only after all
> > of the "sync" devices following them in dpm_list had been suspended
> > (the list is processed in the reverse order during suspend), so it
> > looks like there is a hidden dependency between one of the i2c clients
> > and a "sync" device.
> >
> > If the above supposition is right, flagging the i2c client as "sync"
> > will make the problem go away.
>
> So all the I2C controllers are 'sync' devices ...
>
> $ cat /sys/class/i2c-dev/i2c-*/power/async
> disabled
> disabled
> disabled
> disabled
> disabled
> disabled
> disabled
>
> The I2C clients on the problematic I2C controller are all 'async'
> devices ...
>
> $ cat /sys/class/i2c-dev/i2c-2/device/2-*/power/async
> enabled
> enabled
> enabled
>
> 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).
>
> 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);
>

I'm going to pick up this patch for 6.16 and add a changelog to it, so
please consider providing a Signed-off-by: tag for this change.