linux-kernel - Re: [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b7d29bf2-f7b6-1d53-2d59-9c1bb5ee4d84@gmail.com>
Date:   Thu, 30 Sep 2021 14:19:53 +0300
From:   Matti Vaittinen <mazziesaccount@...il.com>
To:     Tony Lindgren <tony@...mide.com>, linux-omap@...r.kernel.org
Cc:     Dave Gerlach <d-gerlach@...com>, Faiz Abbas <faiz_abbas@...com>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Suman Anna <s-anna@...com>, Tero Kristo <kristo@...nel.org>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Drew Fustini <pdp7pdp7@...il.com>,
        Grygorii Strashko <grygorii.strashko@...com>,
        "H. Nikolaus Schaller" <hns@...delico.com>,
        Robert Nelson <robertcnelson@...il.com>,
        Yongqin Liu <yongqin.liu@...aro.org>
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@...il.com>
> Cc: Grygorii Strashko <grygorii.strashko@...com>
> Cc: "H. Nikolaus Schaller" <hns@...delico.com>
> Cc: Robert Nelson <robertcnelson@...il.com>
> Cc: Yongqin Liu <yongqin.liu@...aro.org>
> Reported-by: Matti Vaittinen <mazziesaccount@...il.com>
> Fixes: effe89e40037 ("soc: ti: omap-prm: Fix occasional abort on reset deassert for dra7 iva")

Tested-by: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>

> Signed-off-by: Tony Lindgren <tony@...mide.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