[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCHv4 8/9] plat/common: Introduce fdt_get_interrupt helper



Hello Jia he,

Please find the comment inline

Thanks & Regards
Sharan

On 3/27/19 3:34 AM, Jia He wrote:
From: Wei Chen <wei.chen@xxxxxxx>
This helper will be used very frequently for devices to
get their interrupts.

Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
Signed-off-by: Jia He <justin.he@xxxxxxx>
---
  plat/common/fdt.c         | 29 +++++++++++++++++++++++++++++
  plat/common/include/fdt.h | 13 +++++++++++++
  2 files changed, 42 insertions(+)

diff --git a/plat/common/fdt.c b/plat/common/fdt.c
index 1707ef2..931c7de 100644
--- a/plat/common/fdt.c
+++ b/plat/common/fdt.c
@@ -342,3 +342,32 @@ int fdt_node_offset_by_compatible_list(const void *fdt, 
int startoffset,
return FDT_ERR_NOTFOUND;
  }
+

Is there a reason why the APIs for fdt_get_interrupt is different from fdt_get_address? Here we return the address as a pointer to address and NULL for error and in other case we return as a reference to the addr parameter and return error code. Can we make these function API consistent? I prefer the one with error code.
+const void *fdt_get_interrupt(const void *fdt, int nodeoffset,
+                       int index, int *size)
+{
+       int nintr, len, term_size;
+       const void *regs;
+
UK_ASSERT(size);

+       nintr = fdt_interrupt_cells(fdt, nodeoffset);
+       if (nintr < 0 || nintr >= FDT_MAX_NCELLS)
+               return NULL;
+
+       /* "interrupts-extended" is not supported */
+       regs = fdt_getprop(fdt, nodeoffset, "interrupts-extended", &len);
+       if (regs) {
+               uk_pr_warn("interrupts multiple parents is not supported\n");
+               return NULL;
+       }
+       /*
+        * Interrupt content must cover the index specific irq information.
+        */
+       regs = fdt_getprop(fdt, nodeoffset, "interrupts", &len);
+       term_size = sizeof(fdt32_t) * nintr;
+       if (regs == NULL || len < term_size * (index + 1))
+               return NULL;
+
+       *size = nintr;
+
+       return regs + term_size * index;
+}
diff --git a/plat/common/include/fdt.h b/plat/common/include/fdt.h
index be6b265..3490f71 100644
--- a/plat/common/include/fdt.h
+++ b/plat/common/include/fdt.h
@@ -154,4 +154,17 @@ int fdt_get_address(const void *fdt, int nodeoffset, int 
index,
  int fdt_node_offset_by_compatible_list(const void *fdt, int startoffset,
                                        const char *compatibles[]);
+/**
+ * fdt_get_interrupt - retrieve device interrupt of a given index
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node to find the address for
+ * @index: the index of interrupt we want to retrieve
+ * @size: interrupt cell size in fdt32_t
+ *
+ * returns:
+ *      NULL on failed, non-NULL on success
+ */
+const void *fdt_get_interrupt(const void *fdt, int nodeoffset,
+                               int index, int *size);
+
  #endif


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.