All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss
@ 2021-09-30  8:01 ` Tony Lindgren
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2021-09-30  8:01 UTC (permalink / raw)
  To: linux-omap
  Cc: Grygorii Strashko, Tero Kristo, Dave Gerlach,
	H. Nikolaus Schaller, Matti Vaittinen, linux-kernel, Faiz Abbas,
	Yongqin Liu, Santosh Shilimkar, Robert Nelson, Drew Fustini,
	linux-arm-kernel

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")
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);
 
 	/* 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);
+	}
 
-exit:
-	if (reset->clkdm) {
-		/* At least dra7 iva needs a delay before clkdm idle */
-		if (has_rstst)
-			udelay(1);
+	if (reset->clkdm)
 		pdata->clkdm_allow_idle(reset->clkdm);
-	}
 
 	return ret;
 }
-- 
2.33.0

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss
@ 2021-09-30  8:01 ` Tony Lindgren
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2021-09-30  8:01 UTC (permalink / raw)
  To: linux-omap
  Cc: Dave Gerlach, Faiz Abbas, Santosh Shilimkar, Suman Anna,
	Tero Kristo, linux-kernel, linux-arm-kernel, Drew Fustini,
	Grygorii Strashko, H. Nikolaus Schaller, Robert Nelson,
	Yongqin Liu, Matti Vaittinen

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")
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);
 
 	/* 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);
+	}
 
-exit:
-	if (reset->clkdm) {
-		/* At least dra7 iva needs a delay before clkdm idle */
-		if (has_rstst)
-			udelay(1);
+	if (reset->clkdm)
 		pdata->clkdm_allow_idle(reset->clkdm);
-	}
 
 	return ret;
 }
-- 
2.33.0

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss
  2021-09-30  8:01 ` Tony Lindgren
@ 2021-09-30 11:19   ` Matti Vaittinen
  -1 siblings, 0 replies; 6+ messages in thread
From: Matti Vaittinen @ 2021-09-30 11:19 UTC (permalink / raw)
  To: Tony Lindgren, linux-omap
  Cc: Dave Gerlach, Faiz Abbas, Santosh Shilimkar, Suman Anna,
	Tero Kristo, linux-kernel, linux-arm-kernel, Drew Fustini,
	Grygorii Strashko, H. Nikolaus Schaller, Robert Nelson,
	Yongqin Liu

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



^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss
@ 2021-09-30 11:19   ` Matti Vaittinen
  0 siblings, 0 replies; 6+ messages in thread
From: Matti Vaittinen @ 2021-09-30 11:19 UTC (permalink / raw)
  To: Tony Lindgren, linux-omap
  Cc: Grygorii Strashko, Tero Kristo, Dave Gerlach,
	H. Nikolaus Schaller, linux-kernel, Faiz Abbas, Yongqin Liu,
	Santosh Shilimkar, Robert Nelson, Drew Fustini, linux-arm-kernel

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



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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss
  2021-09-30 11:19   ` Matti Vaittinen
@ 2021-10-06  5:07     ` Tony Lindgren
  -1 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2021-10-06  5:07 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: linux-omap, Dave Gerlach, Faiz Abbas, Santosh Shilimkar,
	Suman Anna, Tero Kristo, linux-kernel, linux-arm-kernel,
	Drew Fustini, Grygorii Strashko, H. Nikolaus Schaller,
	Robert Nelson, Yongqin Liu

* Matti Vaittinen <mazziesaccount@gmail.com> [210930 11:20]:
> Thanks Tony!
> 
> This was _much_ appreciated :)

Thanks for testing, applying this into fixes.

Regards,

Tony

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss
@ 2021-10-06  5:07     ` Tony Lindgren
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2021-10-06  5:07 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: linux-omap, Tero Kristo, Dave Gerlach, H. Nikolaus Schaller,
	linux-kernel, Faiz Abbas, Yongqin Liu, Santosh Shilimkar,
	Grygorii Strashko, Robert Nelson, Drew Fustini, linux-arm-kernel

* Matti Vaittinen <mazziesaccount@gmail.com> [210930 11:20]:
> Thanks Tony!
> 
> This was _much_ appreciated :)

Thanks for testing, applying this into fixes.

Regards,

Tony

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-06  5:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-30  8:01 [PATCH] soc: ti: omap-prm: Fix external abort for am335x pruss Tony Lindgren
2021-09-30  8:01 ` Tony Lindgren
2021-09-30 11:19 ` Matti Vaittinen
2021-09-30 11:19   ` Matti Vaittinen
2021-10-06  5:07   ` Tony Lindgren
2021-10-06  5:07     ` Tony Lindgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.