Re: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock

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

Hi Marek,

Am Samstag, dem 17.07.2021 um 14:07 +0200 schrieb Marek Vasut:
> On 7/17/21 11:07 AM, Lucas Stach wrote:
> > Am Samstag, dem 17.07.2021 um 02:55 +0200 schrieb Marek Vasut:
> > > On 7/17/21 1:32 AM, Lucas Stach wrote:
> > > > Hi Marek,
> > > 
> > > Hi,
> > > 
> > > > Am Donnerstag, dem 01.07.2021 um 00:59 +0200 schrieb Marek Vasut:
> > > > > In case the power domain clock are ungated before the reset is asserted,
> > > > > the system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
> > > > > domains require different reset deassertion timing, and incorrect reset
> > > > > deassertion timing also leads to hang.
> > > > > 
> > > > > Add per-domain reset_{,de}assert_early flags which allow fine-grained
> > > > > control of the reset assertion and deassertion sequence. Currently, on
> > > > > MX8MM, the behavior is as follows and aligned with NXP downstream ATF
> > > > > fork:
> > > > > - VPUMIX: reset assert, reset deassert, domain power up
> > > > > - GPUMIX: reset assert, domain power on, reset deassert
> > > > > 
> > > > This patch should now be necessary, as my testing over the last few
> > > > days showed that the VPUMIX isn't actually different and copes just
> > > > fine with the reset being asserted early, just like the GPUMIX domain.
> > > 
> > > Yes, this patch is absolutely essential, otherwise the system hangs at
> > > random, as explained in the commit message.
> > 
> > And I was tired. This should have read *not* be necessary. Please take
> > a look at the series I posted, where I just reverted the patch which
> > changed the reset order to a late reset. With this the GPC now once
> > again uses the reset order as required by the GPU, without any
> > additional complexity.
> 
> During my extensive testing in the last few months, I've noticed random 
> hangs of the platform and the reset/clock enablement order does matter, 
> and it is different for different domains. The code in NXP ATF fork 
> seems to confirm that.
> 
> Why do you think that is not the case , is there some documentation 
> which confirms your hypothesis ?

Hahaha, documentation.

I've spent more than a week trying to make sense out of all the HW
requirements, the bits of docs that are actually in the RM and a lot of
testing of different flows. The ATF implementation was more of a
distraction than helpful in this quest. There are two things that my
testing seems to confirm:

a) The common sense assumption that the reset should be asserted
_before_  a power domain is going through the power-up flow and thus
the devices in the PD are going through undefined state seems to hold.

b) The i.MX8MM VPUMIX domain just tolerates the late reset
(reset assertion after power up), as implemented in the ATF, as there
are no devices in the MIX domain aside from the ADB, which would be
able to hang the system due to undefined state. I found no evidence
that the late reset is the preferred or even required sequence. The
VPUMIX domain powers up just fine with the reset already asserted.

This means that the domains do in fact have no differing requirements.
All domains require the reset to be asserted _before_ power-up and
require the clocks to be running for the reset to actually propagate
through. The sequence as implemented before "soc: imx: gpcv2: move
reset assert after requesting domain power up" is correct for all *MIX
domains.

In fact even the nested domains require the same sequence, the only
difference is that for the *MIX domains and non-nested peripheral
domains for reset and clocks the driver only has to deal with SRC and
CCM. For the nested domains clocks need to be enabled at the CCM level
and the BLK_CTRL level and resets aren't coming from the SRC, but from
the BLK_CTRL.

Regards,
Lucas


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