Re: [PATCH 3/4] libxl: Allocate MMIO params for I2c device and update DT

[Oleksandr]

On 05.05.22 10:33, Viresh Kumar wrote:

Hello Viresh


This patch allocates Virtio MMIO params (IRQ and memory region) and pass
them to the backend, also update Guest device-tree.

Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
---
  tools/libs/light/libxl_arm.c | 57 ++++++++++++++++++++++++++++++++++--
  1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 8132a47a72b6..ea633d6f91df 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -115,6 +115,26 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
          }
      }
  
+    for (i = 0; i < d_config->num_i2cs; i++) {
+        libxl_device_i2c *i2c = &d_config->i2cs[i];
+
+        i2c->base = alloc_virtio_mmio_base(gc);
+        if (!i2c->base)
+            return ERROR_FAIL;
+
+        i2c->irq = alloc_virtio_mmio_irq(gc);
+        if (!i2c->irq)
+            return ERROR_FAIL;
+
+        if (virtio_irq < i2c->irq)
+            virtio_irq = i2c->irq;
+
+        virtio_enabled = true;
+
+        LOG(DEBUG, "Allocate Virtio MMIO params for I2C: IRQ %u BASE 
0x%"PRIx64,
+            i2c->irq, i2c->base);
+    }
+
      if (virtio_enabled)
          nr_spis += (virtio_irq - 32) + 1;
  
@@ -874,7 +894,7 @@ static int make_vpci_node(libxl__gc *gc, void *fdt,
  }
  
  
-static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
+static int _make_virtio_mmio_node(libxl__gc *gc, void *fdt,
                                   uint64_t base, uint32_t irq)


NIT: I am not sure the leading underscore in the name would be welcome. 
I would probably rename it.

How about s/_make_virtio_mmio_node/make_virtio_mmio_node_common here and ...



  {
      int res;
@@ -897,7 +917,35 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
      res = fdt_property_interrupts(gc, fdt, &intr, 1);
      if (res) return res;
  
-    res = fdt_property(fdt, "dma-coherent", NULL, 0);
+    return fdt_property(fdt, "dma-coherent", NULL, 0);
+}
+
+static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
+                                 uint64_t base, uint32_t irq)

... s/make_virtio_mmio_node/make_virtio_mmio_node_simple here or 
something like that?


+{
+    int res;
+
+    res = _make_virtio_mmio_node(gc, fdt, base, irq);
+    if (res) return res;
+
+    return fdt_end_node(fdt);
+}
+
+static int make_virtio_mmio_node_i2c(libxl__gc *gc, void *fdt,
+                                 uint64_t base, uint32_t irq)
+{
+    int res;
+
+    res = _make_virtio_mmio_node(gc, fdt, base, irq);
+    if (res) return res;
+
+    res = fdt_begin_node(fdt, "i2c");
+    if (res) return res;
+
+    res = fdt_property_compat(gc, fdt, 1, "virtio,device22");

Please provide a link (in the commit description) to the corresponding 
Linux device tree binding. I assume it is:

Documentation/devicetree/bindings/i2c/i2c-virtio.yaml


+    if (res) return res;
+
+    res = fdt_end_node(fdt);
      if (res) return res;
  
      res = fdt_end_node(fdt);
@@ -1221,6 +1269,11 @@ static int libxl__prepare_dtb(libxl__gc *gc, 
libxl_domain_config *d_config,
                  FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) );
          }
  
+        for (i = 0; i < d_config->num_i2cs; i++) {
+            libxl_device_i2c *i2c = &d_config->i2cs[i];
+            FDT( make_virtio_mmio_node_i2c(gc, fdt, i2c->base, i2c->irq) );
+        }
+
          if (pfdt)
              FDT( copy_partial_fdt(gc, fdt, pfdt) );
  

--
Regards,

Oleksandr Tyshchenko