[U-Boot] [U-Boot,V2] imx: mx7: psci: add system reset support

[Peng Fan]

> -----Original Message-----
> From: Trent Piepho [mailto:tpiepho at impinj.com]
> Sent: 2018年5月10日 3:22
> To: Peng Fan <peng.fan at nxp.com>; christian.gmeiner at gmail.com; Anson
> Huang <anson.huang at nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; sbabic at denx.de
> Cc: Fabio Estevam <fabio.estevam at nxp.com>
> Subject: Re: [U-Boot,V2] imx: mx7: psci: add system reset support
> 
> On Wed, 2018-05-09 at 01:13 +0000, Peng Fan wrote:
> > > +0800, Anson Huang wrote:
> > > > > > Add i.MX7 PSCI system reset support, linux kernel now can use
> "reboot"
> > > > > > command to reset system.
> > > > >
> > > > >
> > > > > > +__secure void imx_system_reset(void) {
> > > > > > +	writew(1 << 2, WDOG1_BASE_ADDR); }
> > > > >
> > > > > This does not work properly on our board.
> > > >
> > > > You could try write 0x14 to WDOG1_BASE_ADDR to see whether it works.
> > > > Here using (1 << 2) triggers both SRS and WDOG_B which seems wrong.
> > >
> > > It works when the Linux driver does that and appears to work
> > > identically when uboot does it that way.
> > >
> > > Triggering both SRS and WDOG_B does not work as it causes a very
> > > short wdog_b assertion.
> >
> > Yes.
> 
> In case it wasn't clear, this code is in the new 2018.05 u-boot release and I think
> will cause a problem for any imx7d board using the typical reset erratum option
> of connecting wdog_b to a pfuze3000 pmic's pwrwon pin.
> 
> An external watchdog monitor chip, I think as used on sabreD for instance,
> might still respond to the short pulse and allow it work.

I understand this. 

> 
> > > There is a little difficulty here in just changing the code to use
> > > WCR_WDE|WCR_SRS, as that would be wrong if the board does not use an
> > > external signal.  The Linux driver uses the device tree to determine
> > > what to do, but this will not work for PSCI.
> >
> > U-Boot need parse the device tree and configure the WDOG accordingly.
> 
> Yes, but consider the difficulties:
> 
> There could be multiple Linux device trees, e.g. in a FIT image and chosen based
> on board revision.  Should the PSCI code use the u-boot device tree?  Which
> would then need to also have this board revision support.  Or should it use the
> Linux DT, which it would need to get information from when the kernel DT is
> prepared before boot?
> 
> Of the five external watchdog signals from the iMX7d, which one should be used?
> I believe each imx watchdog configured in Linux will register a reset handler
> that are called in turn, so Linux should handle this ok.  The uboot PSCI currently
> assumes wdog1 is the correct one to use.
> 
> The Linux driver interfaces with Linux pinctl to iomux the watchdog signal.  The
> PSCI code is assuming the wdog signal is correctly muxed.
> Default iomux values doesn't have any wdog signal routed to a pin.
> 
> Not impossible, but it is not so simple as the original three lines of code might
> have made it seem at first glance.

The imx_system_reset might could be moved to per board specific. Then the board
owner could have their own imeplementation.

Stefano, what do you think?

> 
> If one has the Linux imx watchdog driver working, to use as watchdog, then one
> gets a wdog system reset for free with all the above taken care of.  Which
> makes me wonder if disabling the PSCI reset might not be a better option in that
> situration.

Just wonder, since you are using psci, you have the power related code ready?

-Peng.