Re: [PATCH] hack: soc: imx: gpcv2: avoid unbalanced powering off of one device

[Lucas Stach <l.stach@pengutronix.de>]

Hi Martin,

Am Mittwoch, dem 08.12.2021 um 14:47 +0100 schrieb Martin Kepplinger:
> Hi Lucas,
> 
> I've posted this hack with a report here a few days back:
> https://lore.kernel.org/linux-arm-kernel/20211122115145.177196-1-martin.kepplinger@puri.sm/
> 
> But now that I see these suspend/resume callback additions things
> again go wrong on my imx8mq system.
> 
> With a v5.16-rc4 based tree and printing on regulator enable/disable,
> system suspend + resume looks like so:
> 
> [   47.559681] imx-pgc imx-pgc-domain.5: enable
> [   47.584679] imx-pgc imx-pgc-domain.0: disable
> [   47.646592] imx-pgc imx-pgc-domain.5: disable
> [   47.823627] imx-pgc imx-pgc-domain.5: enable
> [   47.994805] imx-pgc imx-pgc-domain.5: disable
> [   48.664018] imx-pgc imx-pgc-domain.5: enable
> [   48.805828] imx-pgc imx-pgc-domain.5: disable
> [   49.909579] imx-pgc imx-pgc-domain.6: enable
> [   50.013079] imx-pgc imx-pgc-domain.6: failed to enable regulator: -110
> [   50.013686] imx-pgc imx-pgc-domain.5: enable
> [   50.120224] imx-pgc imx-pgc-domain.5: failed to enable regulator: -110
> [   50.120324] imx-pgc imx-pgc-domain.0: enable
> [   53.703468] imx-pgc imx-pgc-domain.0: disable
> [   53.746368] imx-pgc imx-pgc-domain.5: disable
> [   53.754452] imx-pgc imx-pgc-domain.5: failed to disable regulator: -5
> [   53.765045] imx-pgc imx-pgc-domain.6: disable
> [   53.822269] imx-pgc imx-pgc-domain.6: failed to disable regulator: -5
> 
> 
> But my main point is that the situation is a bit hard to understand
> right now. when transitioning to system suspend we expect (if disabled)
> enable+disable to happen, right? and after resuming: enable (+ runtime disable).

Right.

> Makes sense functinally, but I wonder if we could implement it a bit clearer?

Unfortunately, I don't think there is a way to do this in a much
cleaner way. 
> 
> Anyway I'm also not sure whether imx8mq might be different than your
> imx8mm system.

imx8mq, without the upcoming VPU blk-ctrl, has no nested power domains,
which are the main reason for the power domain runtime resume before
the system suspend. If they aren't resumed before the suspend the
nested domains will not be able to power up their parent domains on
resume, due to runtime PM being unavailable at this stage. All of
8mm/8mn/8mp have some sorts of nested power domains.

> 
> When I revert your one patch and add my hack below again, things
> work again and the system resumes without errors.
> 
> Can you imagine what might be missing here?
> 
I would like to understand why the runtime resume of the power domain
isn't working for you. Is this a i2c attached regulator? There might be
some RPM dependency handling (device link) missing to keep the i2c bus
alive until the power domains finished their suspend handling.

Regards,
Lucas

> thanks a lot for working on this!
> 
>                                martin
> ---
>  drivers/soc/imx/gpcv2.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 07610bf87854..898886c9d799 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -319,6 +319,9 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
>  	u32 reg_val, pgc;
>  	int ret;
>  
> +	if (pm_runtime_suspended(domain->dev))
> +		return 0;
> +
>  	/* Enable reset clocks for all devices in the domain */
>  	if (!domain->keep_clocks) {
>  		ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel