All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pinctrl: rockchip: Fix Data Abort exception in SPL
@ 2023-06-08 10:59 Jonas Karlman
  2023-06-29 10:39 ` Kever Yang
  0 siblings, 1 reply; 2+ messages in thread
From: Jonas Karlman @ 2023-06-08 10:59 UTC (permalink / raw)
  To: Simon Glass, Philipp Tomsich, Kever Yang, Heiko Stuebner, David Wu
  Cc: Jagan Teki, Eugen Hristev, Jonas Karlman, Philipp Tomsich, u-boot

Using CONFIG_ARMV8_SPL_EXCEPTION_VECTORS=y and CONFIG_OF_LIVE=y triggers
a Data Abort exception from unaligned memory access when the pinctrl
driver iterate node properties, e.g. for UART2 on RK3568.

  setting mux of GPIO0-24 to 1
  setting mux of GPIO0-24 to 1
  "Synchronous Abort" handler, esr 0x96000021
  elr: 000000000000e554 lr : 000000000000e54c
  x 0: 0000000000000a5c x 1: 0000000000000a5c
  x 2: 0000000000000007 x 3: 0000000000000065
  x 4: 0000000000000007 x 5: 0000000000022d4e
  x 6: 0000000000000a7c x 7: 00000000000227a4
  x 8: 0000000000021cf0 x 9: 0000000000000a7c
  x10: 0000000000021cf0 x11: 0000000000021cf0
  x12: 00000000003fda1c x13: 0000000000000007
  x14: 00000000003fd9ec x15: 000000000001c0ff
  x16: 0000000007000000 x17: 00000000fdccd028
  x18: 00000000003fde20 x19: 0000000000000018
  x20: 0000000000020670 x21: 0000000000000000
  x22: 00000000003fdb00 x23: 00000000003fef90
  x24: 0000000000020688 x25: 0000000000000000
  x26: 0000000000000001 x27: 00000000003ffc50
  x28: 0000000000000000 x29: 00000000003fda60

  Code: b94083e1 97ffd508 93407c01 37f81260 (f9401038)
  Resetting CPU ...

Fix this by replacing the loop to access node properties with use of
ofnode_for_each_prop instead of the current ifdef.

Also continue to next prop instead of aborting at first sign of an
unknown property.

This fixes the Data Abort exception and also pinconf of e.g. pull and
drive in SPL, e.g. for UART2 on RK3568.

  setting mux of GPIO0-24 to 1
  setting mux of GPIO0-24 to 1
  setting pull of GPIO0-24 to 5
  setting mux of GPIO0-25 to 1
  setting mux of GPIO0-25 to 1
  setting pull of GPIO0-25 to 5

Fixes: e7ae4cf27a6d ("pinctrl: rockchip: Add common rockchip pinctrl driver")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 .../pinctrl/rockchip/pinctrl-rockchip-core.c  | 28 ++++---------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
index d9d61fdb726a..8ef089994f46 100644
--- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
+++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
@@ -12,7 +12,6 @@
 #include <fdtdec.h>
 #include <linux/bitops.h>
 #include <linux/libfdt.h>
-#include <asm/global_data.h>
 
 #include "pinctrl-rockchip.h"
 
@@ -433,13 +432,7 @@ static int rockchip_pinctrl_set_state(struct udevice *dev,
 	int prop_len, param;
 	const u32 *data;
 	ofnode node;
-#ifdef CONFIG_OF_LIVE
-	const struct device_node *np;
-	struct property *pp;
-#else
-	int property_offset, pcfg_node;
-	const void *blob = gd->fdt_blob;
-#endif
+	struct ofprop prop;
 	data = dev_read_prop(config, "rockchip,pins", &count);
 	if (count < 0) {
 		debug("%s: bad array size %d\n", __func__, count);
@@ -473,24 +466,15 @@ static int rockchip_pinctrl_set_state(struct udevice *dev,
 		node = ofnode_get_by_phandle(conf);
 		if (!ofnode_valid(node))
 			return -ENODEV;
-#ifdef CONFIG_OF_LIVE
-		np = ofnode_to_np(node);
-		for (pp = np->properties; pp; pp = pp->next) {
-			prop_name = pp->name;
-			prop_len = pp->length;
-			value = pp->value;
-#else
-		pcfg_node = ofnode_to_offset(node);
-		fdt_for_each_property_offset(property_offset, blob, pcfg_node) {
-			value = fdt_getprop_by_offset(blob, property_offset,
-						      &prop_name, &prop_len);
+		ofnode_for_each_prop(prop, node) {
+			value = ofprop_get_property(&prop, &prop_name, &prop_len);
 			if (!value)
-				return -ENOENT;
-#endif
+				continue;
+
 			param = rockchip_pinconf_prop_name_to_param(prop_name,
 								    &default_val);
 			if (param < 0)
-				break;
+				continue;
 
 			if (prop_len >= sizeof(fdt32_t))
 				arg = fdt32_to_cpu(*(fdt32_t *)value);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH] pinctrl: rockchip: Fix Data Abort exception in SPL
  2023-06-08 10:59 [PATCH] pinctrl: rockchip: Fix Data Abort exception in SPL Jonas Karlman
@ 2023-06-29 10:39 ` Kever Yang
  0 siblings, 0 replies; 2+ messages in thread
From: Kever Yang @ 2023-06-29 10:39 UTC (permalink / raw)
  To: Jonas Karlman, Simon Glass, Philipp Tomsich, Heiko Stuebner, David Wu
  Cc: Jagan Teki, Eugen Hristev, Philipp Tomsich, u-boot


On 2023/6/8 18:59, Jonas Karlman wrote:
> Using CONFIG_ARMV8_SPL_EXCEPTION_VECTORS=y and CONFIG_OF_LIVE=y triggers
> a Data Abort exception from unaligned memory access when the pinctrl
> driver iterate node properties, e.g. for UART2 on RK3568.
>
>    setting mux of GPIO0-24 to 1
>    setting mux of GPIO0-24 to 1
>    "Synchronous Abort" handler, esr 0x96000021
>    elr: 000000000000e554 lr : 000000000000e54c
>    x 0: 0000000000000a5c x 1: 0000000000000a5c
>    x 2: 0000000000000007 x 3: 0000000000000065
>    x 4: 0000000000000007 x 5: 0000000000022d4e
>    x 6: 0000000000000a7c x 7: 00000000000227a4
>    x 8: 0000000000021cf0 x 9: 0000000000000a7c
>    x10: 0000000000021cf0 x11: 0000000000021cf0
>    x12: 00000000003fda1c x13: 0000000000000007
>    x14: 00000000003fd9ec x15: 000000000001c0ff
>    x16: 0000000007000000 x17: 00000000fdccd028
>    x18: 00000000003fde20 x19: 0000000000000018
>    x20: 0000000000020670 x21: 0000000000000000
>    x22: 00000000003fdb00 x23: 00000000003fef90
>    x24: 0000000000020688 x25: 0000000000000000
>    x26: 0000000000000001 x27: 00000000003ffc50
>    x28: 0000000000000000 x29: 00000000003fda60
>
>    Code: b94083e1 97ffd508 93407c01 37f81260 (f9401038)
>    Resetting CPU ...
>
> Fix this by replacing the loop to access node properties with use of
> ofnode_for_each_prop instead of the current ifdef.
>
> Also continue to next prop instead of aborting at first sign of an
> unknown property.
>
> This fixes the Data Abort exception and also pinconf of e.g. pull and
> drive in SPL, e.g. for UART2 on RK3568.
>
>    setting mux of GPIO0-24 to 1
>    setting mux of GPIO0-24 to 1
>    setting pull of GPIO0-24 to 5
>    setting mux of GPIO0-25 to 1
>    setting mux of GPIO0-25 to 1
>    setting pull of GPIO0-25 to 5
>
> Fixes: e7ae4cf27a6d ("pinctrl: rockchip: Add common rockchip pinctrl driver")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   .../pinctrl/rockchip/pinctrl-rockchip-core.c  | 28 ++++---------------
>   1 file changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> index d9d61fdb726a..8ef089994f46 100644
> --- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> @@ -12,7 +12,6 @@
>   #include <fdtdec.h>
>   #include <linux/bitops.h>
>   #include <linux/libfdt.h>
> -#include <asm/global_data.h>
>   
>   #include "pinctrl-rockchip.h"
>   
> @@ -433,13 +432,7 @@ static int rockchip_pinctrl_set_state(struct udevice *dev,
>   	int prop_len, param;
>   	const u32 *data;
>   	ofnode node;
> -#ifdef CONFIG_OF_LIVE
> -	const struct device_node *np;
> -	struct property *pp;
> -#else
> -	int property_offset, pcfg_node;
> -	const void *blob = gd->fdt_blob;
> -#endif
> +	struct ofprop prop;
>   	data = dev_read_prop(config, "rockchip,pins", &count);
>   	if (count < 0) {
>   		debug("%s: bad array size %d\n", __func__, count);
> @@ -473,24 +466,15 @@ static int rockchip_pinctrl_set_state(struct udevice *dev,
>   		node = ofnode_get_by_phandle(conf);
>   		if (!ofnode_valid(node))
>   			return -ENODEV;
> -#ifdef CONFIG_OF_LIVE
> -		np = ofnode_to_np(node);
> -		for (pp = np->properties; pp; pp = pp->next) {
> -			prop_name = pp->name;
> -			prop_len = pp->length;
> -			value = pp->value;
> -#else
> -		pcfg_node = ofnode_to_offset(node);
> -		fdt_for_each_property_offset(property_offset, blob, pcfg_node) {
> -			value = fdt_getprop_by_offset(blob, property_offset,
> -						      &prop_name, &prop_len);
> +		ofnode_for_each_prop(prop, node) {
> +			value = ofprop_get_property(&prop, &prop_name, &prop_len);
>   			if (!value)
> -				return -ENOENT;
> -#endif
> +				continue;
> +
>   			param = rockchip_pinconf_prop_name_to_param(prop_name,
>   								    &default_val);
>   			if (param < 0)
> -				break;
> +				continue;
>   
>   			if (prop_len >= sizeof(fdt32_t))
>   				arg = fdt32_to_cpu(*(fdt32_t *)value);

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-06-29 10:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08 10:59 [PATCH] pinctrl: rockchip: Fix Data Abort exception in SPL Jonas Karlman
2023-06-29 10:39 ` Kever Yang

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.