diff mbox series

[v5,01/18] Revert "soc: imx: gpcv2: move reset assert after requesting domain power up"

Message ID 20211002005954.1367653-2-l.stach@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series i.MX8MM GPC improvements and BLK_CTRL driver | expand

Commit Message

Lucas Stach Oct. 2, 2021, 12:59 a.m. UTC
This reverts commit a77ebdd9f553. It turns out that the VPU domain has no
different requirements, even though the downstream ATF implementation seems
to suggest otherwise. Powering on the domain with the reset asserted works
fine. As the changed sequence has caused sporadic issues with the GPU
domains, just revert the change to go back to the working sequence.

Cc: <stable@vger.kernel.org> # 5.14
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Acked-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Adam Ford Oct. 3, 2021, 10:43 a.m. UTC | #1
On Fri, Oct 1, 2021 at 8:00 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> This reverts commit a77ebdd9f553. It turns out that the VPU domain has no
> different requirements, even though the downstream ATF implementation seems
> to suggest otherwise. Powering on the domain with the reset asserted works
> fine. As the changed sequence has caused sporadic issues with the GPU
> domains, just revert the change to go back to the working sequence.
>
> Cc: <stable@vger.kernel.org> # 5.14
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Acked-by: Peng Fan <peng.fan@nxp.com>
> ---

Lucas,

I applied your series to the 5.14.y kernel to test with the
imx8mm-beacon board, but I found that it doesn't wake from sleep.
I'll experiment with other versions of ATF.  If nobody else has this
problem, I'll assume, it's an error on my part.

adam

>  drivers/soc/imx/gpcv2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 34a9ac1f2b9b..8b7a01773aec 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -244,6 +244,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>                 goto out_regulator_disable;
>         }
>
> +       reset_control_assert(domain->reset);
> +
>         if (domain->bits.pxx) {
>                 /* request the domain to power up */
>                 regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> @@ -266,8 +268,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>                                   GPC_PGC_CTRL_PCR);
>         }
>
> -       reset_control_assert(domain->reset);
> -
>         /* delay for reset to propagate */
>         udelay(5);
>
> --
> 2.30.2
>
Lucas Stach Oct. 3, 2021, 7:46 p.m. UTC | #2
Hi Adam,

Am Sonntag, dem 03.10.2021 um 05:43 -0500 schrieb Adam Ford:
> On Fri, Oct 1, 2021 at 8:00 PM Lucas Stach <l.stach@pengutronix.de> wrote:
> > 
> > This reverts commit a77ebdd9f553. It turns out that the VPU domain has no
> > different requirements, even though the downstream ATF implementation seems
> > to suggest otherwise. Powering on the domain with the reset asserted works
> > fine. As the changed sequence has caused sporadic issues with the GPU
> > domains, just revert the change to go back to the working sequence.
> > 
> > Cc: <stable@vger.kernel.org> # 5.14
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > Acked-by: Peng Fan <peng.fan@nxp.com>
> > ---
> 
> Lucas,
> 
> I applied your series to the 5.14.y kernel to test with the
> imx8mm-beacon board, but I found that it doesn't wake from sleep.
> I'll experiment with other versions of ATF.  If nobody else has this
> problem, I'll assume, it's an error on my part.

I've tested this series on the i.MX8MM-EVK and a custom (not yet
public) i.MX8MM board and both did work as expected with both system
suspend/resume and runtime power management for the display parts. I've
used the upstream TF-A release v2.5.

Regards,
Lucas

> 
> adam
> 
> >  drivers/soc/imx/gpcv2.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> > index 34a9ac1f2b9b..8b7a01773aec 100644
> > --- a/drivers/soc/imx/gpcv2.c
> > +++ b/drivers/soc/imx/gpcv2.c
> > @@ -244,6 +244,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> >                 goto out_regulator_disable;
> >         }
> > 
> > +       reset_control_assert(domain->reset);
> > +
> >         if (domain->bits.pxx) {
> >                 /* request the domain to power up */
> >                 regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> > @@ -266,8 +268,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> >                                   GPC_PGC_CTRL_PCR);
> >         }
> > 
> > -       reset_control_assert(domain->reset);
> > -
> >         /* delay for reset to propagate */
> >         udelay(5);
> > 
> > --
> > 2.30.2
> >
Adam Ford Oct. 4, 2021, 12:03 a.m. UTC | #3
On Sun, Oct 3, 2021 at 2:46 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Adam,
>
> Am Sonntag, dem 03.10.2021 um 05:43 -0500 schrieb Adam Ford:
> > On Fri, Oct 1, 2021 at 8:00 PM Lucas Stach <l.stach@pengutronix.de> wrote:
> > >
> > > This reverts commit a77ebdd9f553. It turns out that the VPU domain has no
> > > different requirements, even though the downstream ATF implementation seems
> > > to suggest otherwise. Powering on the domain with the reset asserted works
> > > fine. As the changed sequence has caused sporadic issues with the GPU
> > > domains, just revert the change to go back to the working sequence.
> > >
> > > Cc: <stable@vger.kernel.org> # 5.14
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > Acked-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> >
> > Lucas,
> >
> > I applied your series to the 5.14.y kernel to test with the
> > imx8mm-beacon board, but I found that it doesn't wake from sleep.
> > I'll experiment with other versions of ATF.  If nobody else has this
> > problem, I'll assume, it's an error on my part.
>
> I've tested this series on the i.MX8MM-EVK and a custom (not yet
> public) i.MX8MM board and both did work as expected with both system
> suspend/resume and runtime power management for the display parts. I've
> used the upstream TF-A release v2.5.

OK.  I have tested the USB otg1 and otg2 and they work. The etnativ
controllers enumerate, but without lcdif and dsim, it's not practical
to test.
If nobody else is having issues with suspend-resume, go ahead and mark this as:

Tested-by: Adam Ford <aford173@gmail.com> #imx8mm-beacon
>
> Regards,
> Lucas
>
> >
> > adam
> >
> > >  drivers/soc/imx/gpcv2.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> > > index 34a9ac1f2b9b..8b7a01773aec 100644
> > > --- a/drivers/soc/imx/gpcv2.c
> > > +++ b/drivers/soc/imx/gpcv2.c
> > > @@ -244,6 +244,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> > >                 goto out_regulator_disable;
> > >         }
> > >
> > > +       reset_control_assert(domain->reset);
> > > +
> > >         if (domain->bits.pxx) {
> > >                 /* request the domain to power up */
> > >                 regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> > > @@ -266,8 +268,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> > >                                   GPC_PGC_CTRL_PCR);
> > >         }
> > >
> > > -       reset_control_assert(domain->reset);
> > > -
> > >         /* delay for reset to propagate */
> > >         udelay(5);
> > >
> > > --
> > > 2.30.2
> > >
>
>
diff mbox series

Patch

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 34a9ac1f2b9b..8b7a01773aec 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -244,6 +244,8 @@  static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		goto out_regulator_disable;
 	}
 
+	reset_control_assert(domain->reset);
+
 	if (domain->bits.pxx) {
 		/* request the domain to power up */
 		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
@@ -266,8 +268,6 @@  static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 				  GPC_PGC_CTRL_PCR);
 	}
 
-	reset_control_assert(domain->reset);
-
 	/* delay for reset to propagate */
 	udelay(5);