Re: [XEN v3 4/9] xen/arm: Introduce a wrapper for dt_device_get_address() to handle paddr_t

[Julien Grall]

Hi Ayan,

On 08/02/2023 12:05, Ayan Kumar Halder wrote:

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 6c9712ab7b..d0f19d5cfc 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -934,8 +934,9 @@ bail:
  }

/* dt_device_address - Translate device tree address and return it */

-int dt_device_get_address(const struct dt_device_node *dev, unsigned int index,
-                          u64 *addr, u64 *size)
+static int dt_device_get_address(const struct dt_device_node *dev,
+                                 unsigned int index,
+                                 u64 *addr, u64 *size)
  {
      const __be32 *addrp;
      unsigned int flags;
@@ -955,6 +956,37 @@ int dt_device_get_address(const struct dt_device_node 
*dev, unsigned int index,
      return 0;
  }

+int dt_device_get_paddr(const struct dt_device_node *dev, unsigned int index,

+                        paddr_t *addr, paddr_t *size)
+{
+    u64 dt_addr = 0, dt_size = 0;

Please use uint64_t for new code.

+    int ret;
+
+    ret = dt_device_get_address(dev, index, &dt_addr, &dt_size);
+
+    if ( addr )
+    {
+        if ( (dt_addr >> (PADDR_SHIFT - 1)) > 1 )
+        {
+            printk("Error: Physical address greater than max width 
supported\n");

I would print the width, address and the node name to help debugging.

+            return -EINVAL;

NIT: -EINVAL tends to be quite overloaded. How about using -ERANGE?

+        }
+
+        *addr = dt_addr;
+    }
+
+    if ( size )
+    {
+        if ( (dt_size >> (PADDR_SHIFT - 1)) > 1 )
+        {
+            printk("Error: Physical size greater than max width supported\n");

Same here for the message but with s/address/size/.

+            return -EINVAL;

Same here for the errno.

The rest looks fine so long Stefano's comment are addressed.

+        }
+        *size = dt_size;
+    }
+
+    return ret;
+}

Cheers,

--
Julien Grall