Messages in this thread | | | Date | Thu, 30 Sep 2021 14:19:53 +0300 | From | Matti Vaittinen <> | Subject | Re: [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss |
| |
Thanks Tony!
This was _much_ appreciated :)
On 9/30/21 11:01, Tony Lindgren wrote: > Starting with v5.15-rc1, we may now see some am335x beaglebone black > device produce the following error on pruss probe: > > Unhandled fault: external abort on non-linefetch (0x1008) at 0xe0326000 > > This has started with the enabling of pruss for am335x in the dts files. > > Turns out the is caused by the PRM reset handling not waiting for the > reset bit to clear. To fix the issue, let's always wait for the reset > bit to clear, even if there is a separate reset status register. > > We attempted to fix a similar issue for dra7 iva with a udelay() in > commit effe89e40037 ("soc: ti: omap-prm: Fix occasional abort on reset > deassert for dra7 iva"). There is no longer a need for the udelay() > for dra7 iva reset either with the check added for reset bit clearing. > > Cc: Drew Fustini <pdp7pdp7@gmail.com> > Cc: Grygorii Strashko <grygorii.strashko@ti.com> > Cc: "H. Nikolaus Schaller" <hns@goldelico.com> > Cc: Robert Nelson <robertcnelson@gmail.com> > Cc: Yongqin Liu <yongqin.liu@linaro.org> > Reported-by: Matti Vaittinen <mazziesaccount@gmail.com> > Fixes: effe89e40037 ("soc: ti: omap-prm: Fix occasional abort on reset deassert for dra7 iva")
Tested-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/soc/ti/omap_prm.c | 27 +++++++++++++++------------ > 1 file changed, 15 insertions(+), 12 deletions(-) > > diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c > --- a/drivers/soc/ti/omap_prm.c > +++ b/drivers/soc/ti/omap_prm.c > @@ -825,25 +825,28 @@ static int omap_reset_deassert(struct reset_controller_dev *rcdev, > writel_relaxed(v, reset->prm->base + reset->prm->data->rstctrl); > spin_unlock_irqrestore(&reset->lock, flags); > > - if (!has_rstst) > - goto exit; > + /* wait for the reset bit to clear */ > + ret = readl_relaxed_poll_timeout_atomic(reset->prm->base + > + reset->prm->data->rstctrl, > + v, !(v & BIT(id)), 1, > + OMAP_RESET_MAX_WAIT); > + if (ret) > + pr_err("%s: timedout waiting for %s:%lu\n", __func__, > + reset->prm->data->name, id);
If I was writing this I might drop the __func__. AFAIR dyndbg allows enabling the functipn names to be printed by +f. This is just a 'nit' though - I am happy if this fix gets in no matter how this print eventually looks like. I just thought I mention this as the __func__ catched my eye.
> > /* wait for the status to be set */ > - ret = readl_relaxed_poll_timeout_atomic(reset->prm->base + > + if (has_rstst) { > + ret = readl_relaxed_poll_timeout_atomic(reset->prm->base + > reset->prm->data->rstst, > v, v & BIT(st_bit), 1, > OMAP_RESET_MAX_WAIT); > - if (ret) > - pr_err("%s: timedout waiting for %s:%lu\n", __func__, > - reset->prm->data->name, id); > + if (ret) > + pr_err("%s: timedout waiting for %s:%lu\n", __func__, > + reset->prm->data->name, id);
Same here (although that would be unrelated change as the print exists prior this patch).
I tested this patch on v5.15-rc3 using my BBB Rev C - it seems to fix the boot issue on my board! Thanks a bunch!
Best Regards --Matti Vaittinen
| |