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 <jo...@kwiboo.se>
Reviewed-by: Kever Yang <kever.y...@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);

Reply via email to