# HG changeset patch
# Parent e24ae3e49b73a2b799765d863f69f0c956ea2c54
# User Louis Opter <louis@lse.epitech.net>, David Pineau <dav.pineau@gmail.com>
rathaxes: correctly use chunk and template sequences parameters in the e1000 sample

diff --git a/rathaxes/samples/e1000/e1000.blt b/rathaxes/samples/e1000/e1000.blt
--- a/rathaxes/samples/e1000/e1000.blt
+++ b/rathaxes/samples/e1000/e1000.blt
@@ -15,18 +15,20 @@
 
         chunk   ::decl()
         {
-            struct rtx_e1000_ctx
+            /*
+             * Yes, this typedef looks ugly but read the remark about
+             * Ethernet::Device in ethernet.blt.
+             */
+            typedef struct rtx_e1000_ctx
             {
                 int                             bars;
                 unsigned char /* __iomem */     *ioaddr;
                 int                             irq;
-            };
+            } *rtx_e1000_ctx_p;
         }
 
-        chunk   ::init(bars, ioaddr)
+        chunk   ::init()
         {
-            ${self}.bars = ${bars};
-            ${self}.ioaddr = ${ioaddr};
         }
 
         map
@@ -136,58 +138,74 @@
 
     template sequence   e1000::create_device()
     {
-        chunk Ethernet::create_device()
+        chunk Ethernet::create_device(/* PCI::Device */ pdev, /* Ethernet::Device */ rtx_ether_ctx)
         {
-            rtx_ether_ctx->hw_ctx.irq = pdev->irq;
-            rtx_ether_ctx->hw_ctx.bars = pci_select_bars(pdev, IORESOURCE_MEM);
-            if (pci_enable_device_mem(pdev))
+            /*
+             * PCI init stuff:
+             *
+             * Some of that code should be moved in the PCI blts, also at some
+             * point maybe we could do that completely automatically in the PCI
+             * blts.
+             */
+
+            /*
+             * We could have used an init function here but since we can't init
+             * all the fields at once (see, ioaddr) and cannot call a C
+             * function within a placeholder (${}), it wasn't really worth it.
+             */
+            ${rtx_ether_ctx}->hw_ctx.bars = pci_select_bars(${pdev}, IORESOURCE_MEM);
+            ${rtx_ether_ctx}->hw_ctx.irq = ${pdev}->irq;
+
+            if (pci_enable_device_mem(${pdev}))
             {
                 ${Log::info("e1000::create: pci_enable_device_mem failed")};
             }
-
-            if (pci_request_selected_regions(pdev, rtx_ether_ctx->hw_ctx.bars, ${config.name}))
+            if (pci_request_selected_regions(${pdev}, ${rtx_ether_ctx}->hw_ctx.bars, ${config.name}))
             {
                 ${Log::info("e1000::create: pci_request_selected_regions failed")};
             }
-
             if (${config.set_master})
             {
-                pci_set_master(pdev);
+                pci_set_master(${pdev});
             }
 
             /* 0 here is for BAR_0: */
-            rtx_ether_ctx->hw_ctx.ioaddr = pci_ioremap_bar(pdev, 0);
-            if (!rtx_ether_ctx->hw_ctx.ioaddr)
+            ${rtx_ether_ctx}->hw_ctx.ioaddr = pci_ioremap_bar(${pdev}, 0);
+            if (!${rtx_ether_ctx}->hw_ctx.ioaddr)
             {
                 ${Log::info("e1000::create: pci_ioremap_bar failed")};
             }
 
+            /*
+             * The really device specific algorithm starts here (so it should
+             * certainly be written in the frontend):
+             */
+
             /* Reset the card */
-            rtx_e1000_register_write32(&rtx_ether_ctx->hw_ctx, E1000_CTRL, E1000_CMD_RST);
+            rtx_e1000_register_write32(&${rtx_ether_ctx}->hw_ctx, E1000_CTRL, E1000_CMD_RST);
             udelay(10);
 
-            /* Now we can load its mac address */
+            /* Now we can load its mac address (thanks minix code) */
             int i = 0;
-            for (i = 0 /* < this is not generated! */; i < 3; ++i)
+            for (i = 0 /* < this is not generated! (cnorm bug) */; i < 3; ++i)
             {
-                rtx_e1000_register_write32(&rtx_ether_ctx->hw_ctx, E1000_EEPROM_READ, (i << 8) | 1);
+                rtx_e1000_register_write32(&${rtx_ether_ctx}->hw_ctx, E1000_EEPROM_READ, (i << 8) | 1);
 
                 int value;
-                /* Should be a do { } while(); but the compiler doesn't do { } while(); yet. */
-                value = rtx_e1000_register_read32(&rtx_ether_ctx->hw_ctx, E1000_EEPROM_READ);
-                while ((value & (1 << 4)) == 0)
-                    value = rtx_e1000_register_read32(&rtx_ether_ctx->hw_ctx, E1000_EEPROM_READ);
+                do
+                    value = rtx_e1000_register_read32(&${rtx_ether_ctx}->hw_ctx, E1000_EEPROM_READ);
+                while ((value & (1 << 4)) == 0);
                 value >>= 16;
 
-                rtx_ether_ctx->net_dev->dev_addr[i * 2] = value & 0xff;
-                rtx_ether_ctx->net_dev->dev_addr[i * 2 + 1] = (value >> 8) & 0xff;
+                ${rtx_ether_ctx}->net_dev->dev_addr[i * 2] = value & 0xff;
+                ${rtx_ether_ctx}->net_dev->dev_addr[i * 2 + 1] = (value >> 8) & 0xff;
             }
 
-            memcpy(rtx_ether_ctx->net_dev->perm_addr,
-                   rtx_ether_ctx->net_dev->dev_addr,
-                   rtx_ether_ctx->net_dev->addr_len);
+            memcpy(${rtx_ether_ctx}->net_dev->perm_addr,
+                   ${rtx_ether_ctx}->net_dev->dev_addr,
+                   ${rtx_ether_ctx}->net_dev->addr_len);
 
-            { /* < mais lol. */
+            { /* < See #10 */
                 ${Log::info("e1000::create: mac address loaded from the EEPROM")};
             }
         }
@@ -199,15 +217,14 @@
 
     template sequence   e1000::destroy_device()
     {
-        chunk   Ethernet::destroy_device
+        chunk   Ethernet::destroy_device(/* PCI::Device */ pdev, /* Ethernet::Device */ rtx_ether_ctx)
         {
             /*
              * Here, we should have some checks to avoid to free resources that
              * haven't been allocated. (e.g: in case of previous errors).
              */
-            struct rtx_ethernet_dev* rtx_ether_ctx = netdev_priv(net_dev);
-            iounmap(rtx_ether_ctx->hw_ctx.ioaddr);
-            pci_release_selected_regions(pdev, rtx_ether_ctx->hw_ctx.bars);
+            iounmap(${rtx_ether_ctx}->hw_ctx.ioaddr);
+            pci_release_selected_regions(${pdev}, ${rtx_ether_ctx}->hw_ctx.bars);
         }
 
         chunk   ::CALL
@@ -275,7 +292,7 @@
 
         chunk   ::CALL()
         {
-            rtx_e1000_register_read32(&${ctx}, ${reg_offset});
+            rtx_e1000_register_read32(${ctx}, ${reg_offset});
         }
     }
 
@@ -296,7 +313,7 @@
 
         chunk   ::CALL()
         {
-            rtx_e1000_register_write32(&${ctx}, ${reg_offset});
+            rtx_e1000_register_write32(${ctx}, ${reg_offset});
         }
     }
 
@@ -320,7 +337,7 @@
         }
     }
 
-    template sequence   e1000::setup_interrupt_handler()
+    template sequence   e1000::setup_interrupt_handler(Ethernet::Device ctx)
     {
         chunk   LKM::includes()
         {
@@ -352,13 +369,13 @@
             }
         }
 
-        chunk   ::CALL()
+        chunk   ::CALL
         {
             // this is an hack for the scope
             (void)1;
             {
                 int error;
-                error = e1000_setup_interrupt_handler(rtx_ether_dev);
+                error = e1000_setup_interrupt_handler(${ctx});
                 if (error)
                 {
                     return error;
diff --git a/rathaxes/samples/e1000/e1000.rti b/rathaxes/samples/e1000/e1000.rti
--- a/rathaxes/samples/e1000/e1000.rti
+++ b/rathaxes/samples/e1000/e1000.rti
@@ -1,25 +1,22 @@
 interface e1000 : Socket, Ethernet, PCI, LKM
 {
     provided type   e1000::Context;
+    /*
+     * These two types should actually be registers definitions in the frontend:
+     */
     provided type   e1000::Register;
     provided type   e1000::Commands;
 
-    /*
-     * This sequence should receive an argument like Ethernet::Device, but it is
-     * unclear about how this argument should be bound to a variable/argument in
-     * the instrumented C code.
-     *
-     * Here again, we rely on the fact that *we* wrote the parent context and
-     * named the C variables we need/use with the same name everywhere.
-     */
     provided sequence   e1000::create_device()
     {
+        /* should take PCI::Device and Ethernet::Device args: */
         provided chunk  Ethernet::create_device;
         provided chunk  ::CALL;
     }
 
     provided sequence   e1000::destroy_device()
     {
+        /* should take PCI::Device and Ethernet::Device args: */
         provided chunk  Ethernet::destroy_device;
         provided chunk  ::CALL;
     }
@@ -35,9 +32,9 @@
         provided chunk  ::CALL;
     }
 
-    provided sequence   e1000::setup_interrupt_handler()
+    provided sequence   e1000::setup_interrupt_handler(Ethernet::Device)
     {
-        provided chunk  LKM::includes; // work without this one
+        provided chunk  LKM::includes; // works without this one
         provided chunk  LKM::prototypes;
         provided chunk  LKM::code;
         provided chunk  ::CALL;
diff --git a/rathaxes/samples/e1000/ethernet.blt b/rathaxes/samples/e1000/ethernet.blt
--- a/rathaxes/samples/e1000/ethernet.blt
+++ b/rathaxes/samples/e1000/ethernet.blt
@@ -1,5 +1,10 @@
 with Ethernet, PCI, LKM, Log
 {
+    /*
+     * Unlike PCI::Device, Ethernet::Device doesn't match the struct net_device
+     * from Linux. Ethernet::Device is the type that we use in the private
+     * field of the struct net_device.
+     */
     template type   Ethernet::Device()
     {
         chunk LKM::includes()
@@ -10,7 +15,14 @@
 
         chunk ::decl()
         {
-            struct rtx_ethernet_dev
+            /*
+             * So, at first sight, it sucks to typedef it as pointer but (for
+             * now) it makes sense for two reasons:
+             * - This structure will always be used through a pointer;
+             * - This remove the ambiguity of pointer/not-pointer in the
+             * ::init() chunk.
+             */
+            typedef struct rtx_ethernet_dev
             {
                 /*
                  * I think it's useless to use the ${PCI::Device} "abstraction"
@@ -21,12 +33,18 @@
 
                 /* while waiting on issue #8 */
                 struct rtx_e1000_ctx    hw_ctx;
-            };
+            } *rtx_ethernet_dev_p;
         }
 
-        chunk ::init(net_dev)
+        chunk ::init(net_dev, pci_dev)
         {
-            ${self} = ${net_dev};
+            ${self} = netdev_priv(${net_dev});
+            /*
+             * We can use -> because we know that ${self} will be always a
+             * pointer ("thanks" to the typedef)
+             */
+            ${self}->pci_dev = ${pci_dev};
+            ${self}->net_dev = ${net_dev};
         }
 
         map
@@ -45,10 +63,10 @@
         {
             static int  rtx_ethernet_open(struct net_device *dev)
             {
-                struct rtx_ethernet_dev* rtx_ether_dev =  netdev_priv(dev);
+                struct rtx_ethernet_dev* rtx_ether_dev = netdev_priv(dev);
                 struct rtx_e1000_ctx* ctx = &rtx_ether_dev->hw_ctx;
 
-                ${pointcut ::IMPLEMENTATION};
+                ${pointcut ::IMPLEMENTATION(local.rtx_ethernet_dev)};
 
                 return 0;
             }
@@ -121,7 +139,7 @@
         }
     }
 
-    template sequence   Ethernet::init(PCI::Device pdev)
+    template sequence   Ethernet::init()
     {
         chunk LKM::data()
         {
@@ -133,18 +151,20 @@
             };
         }
 
-        chunk PCI::pci_probe_hook()
+        /* For now the type is not handled, so we just omit it (see #17) */
+        chunk PCI::pci_probe_hook(/* PCI::Device */ pdev)
         {
             /*
              * This typedef is needed to workaround a bug in CNorm __std__
              * dialect.
              */
             typedef int ${Ethernet::Device};
-            ${Ethernet::Device} *rtx_ether_ctx;
-            struct net_device   *net_dev;
+
+            ${Ethernet::Device} rtx_ether_ctx;
+            struct net_device *net_dev;
             int error;
 
-            error = 0;
+            /* Initialize the net_device structure */
             net_dev = alloc_etherdev(sizeof(*rtx_ether_ctx));
             if (net_dev == 0)
             {
@@ -152,64 +172,54 @@
                 /*
                  * Again, the error should be "raised" in the parent context.
                  *
-                 * Here we know that we should return ENOMEM because *we* wrote
+                 * Here we know that we can return ENOMEM because *we* wrote
                  * the parent context.
                  */
                 return -ENOMEM;
             }
+            SET_NETDEV_DEV(net_dev, &${pdev}->dev);
             strlcpy(net_dev->name, ${config.ifname}, sizeof(net_dev->name));
-            net_dev->irq = pdev->irq;
-            // Maybe we should try ${rtx_ether_ctx.init()} here:
-            rtx_ether_ctx = netdev_priv(net_dev);
-            //rtx_ether_ctx->pci_dev = ${pdev};
-            rtx_ether_ctx->pci_dev = pdev; // In the meantime do it directly
-            rtx_ether_ctx->net_dev = net_dev;
+            net_dev->irq = ${pdev}->irq;
+            net_dev->netdev_ops = &rtx_ether_ops;
 
-            /*
-             * The substitution of ${pdev} fails here. I also tried to add a
-             * "substitute method" to the PCI::Device that was just doing
-             * "${self}" but it didn't work either (it was subsituted by a
-             * placeholder, e.g: _1).
-             *
-             * That's why we cheated a bit and named all the arguments pdev.
-             */
-            //SET_NETDEV_DEV(net_dev, &${pdev}->dev);
-            SET_NETDEV_DEV(net_dev, &pdev->dev);
-            net_dev->netdev_ops = &rtx_ether_ops;
-            if ((error = register_netdev(net_dev)))
+            error = register_netdev(net_dev);
+            if (error)
             {
                 ${Log::info("Cannot register the driver")};
                 return error;
             }
 
-            /* same problem as above with ${pdev} */
-            //pci_set_drvdata(${pdev}, net_dev);
-            pci_set_drvdata(pdev, net_dev);
+            /* Initialize our context held by the net_device structure */
+            ${rtx_ether_ctx.init(local.net_dev, pdev)};
 
-            ${pointcut Ethernet::create_device};
+            pci_set_drvdata(${pdev}, net_dev);
+
+            ${pointcut Ethernet::create_device(pdev, local.rtx_ether_ctx)};
         }
 
+        /* This chunk should be removed (see #26) */
         chunk   ::CALL
         {
         }
     }
 
-    template sequence   Ethernet::exit(PCI::Device pdev)
+    template sequence   Ethernet::exit()
     {
-        chunk   PCI::pci_remove_hook()
+        chunk   PCI::pci_remove_hook(/* PCI::Device */ pdev)
         {
-            struct net_device *net_dev = pci_get_drvdata(pdev);
+            /* workaround for cnorm unstrict */
+            typedef int ${Ethernet::Device};
 
-            ${pointcut Ethernet::destroy_device};
+            struct net_device *net_dev = pci_get_drvdata(${pdev});
+            ${Ethernet::Device} rtx_ether_ctx = netdev_priv(net_dev);
+
+            ${pointcut Ethernet::destroy_device(pdev, local.rtx_ether_ctx)};
 
             unregister_netdev(net_dev);
-            /*
-             * If we had some cleanup todo with struct rtx_ether_ctx we would
-             * do a netdev_priv(net_dev) here and do it.
-             */
             free_netdev(net_dev);
         }
 
+        /* This chunk should be removed (see #26) */
         chunk ::CALL
         {
         }
diff --git a/rathaxes/samples/e1000/ethernet.rti b/rathaxes/samples/e1000/ethernet.rti
--- a/rathaxes/samples/e1000/ethernet.rti
+++ b/rathaxes/samples/e1000/ethernet.rti
@@ -28,19 +28,17 @@
         provided chunk  LKM::code;
     }
 
-    provided sequence   Ethernet::init(PCI::Device)
+    provided sequence   Ethernet::init()
     {
         provided chunk  LKM::data;
-        provided chunk  PCI::pci_probe_hook;
-        provided chunk  ::CALL;
+        provided chunk  PCI::pci_probe_hook; /* should take a PCI::Device arg */
 
         provided pointcut   Ethernet::create_device;
     }
 
-    provided sequence   Ethernet::exit(PCI::Device)
+    provided sequence   Ethernet::exit()
     {
-        provided chunk  ::CALL;
-        provided chunk  PCI::pci_remove_hook;
+        provided chunk  PCI::pci_remove_hook; /* should take a PCI::Device arg */
 
         provided pointcut   Ethernet::destroy_device;
     }
diff --git a/rathaxes/samples/e1000/lkm.rtx b/rathaxes/samples/e1000/lkm.rtx
--- a/rathaxes/samples/e1000/lkm.rtx
+++ b/rathaxes/samples/e1000/lkm.rtx
@@ -3,7 +3,7 @@
     Ethernet::open(Ethernet::Device dev)
     {
         Log::info("Open the device");
-        e1000::setup_interrupt_handler();
+        e1000::setup_interrupt_handler(dev);
         Log::info("Interrupt handler installed");
         e1000::set_up_device();
         e1000::activate_device_interruption();
diff --git a/rathaxes/samples/e1000/pci.blt b/rathaxes/samples/e1000/pci.blt
--- a/rathaxes/samples/e1000/pci.blt
+++ b/rathaxes/samples/e1000/pci.blt
@@ -22,7 +22,7 @@
         }
     }
 
-    template sequence   PCI::probe(PCI::Device pdev)
+    template sequence   PCI::probe()
     {
         chunk LKM::prototypes()
         {
@@ -41,7 +41,8 @@
                 if (err < 0)
                     goto fail;
 
-                ${pointcut PCI::pci_probe_hook};
+                /* Use local. to reference a local C variable: */
+                ${pointcut PCI::pci_probe_hook(local.pdev)};
 
                 return 0;
 
@@ -50,12 +51,13 @@
             }
         }
 
+        /* This chunk should be remove (see #26) */
         chunk   ::CALL
         {
         }
     }
 
-    template sequence   PCI::remove(PCI::Device pdev)
+    template sequence   PCI::remove()
     {
         chunk LKM::prototypes()
         {
@@ -66,12 +68,13 @@
         {
             static void rtx_pci_remove(struct pci_dev *pdev)
             {
-                ${pointcut PCI::pci_remove_hook};
+                ${pointcut PCI::pci_remove_hook(local.pdev)};
 
                 pci_disable_device(pdev);
             }
         }
 
+        /* This chunk should be remove (see #26) */
         chunk   ::CALL()
         {
         }
@@ -125,6 +128,8 @@
              * This sequence is just "intermediate" code that will just inject
              * itself in the hook LKM::init_bus_hook for which this sequence
              * has a chunk (see above chunk).
+             *
+             * -> Should be removed see #26
              */
         }
     }
@@ -136,6 +141,7 @@
             pci_unregister_driver(&rtx_pci_driver);
         }
 
+        /* This chunk should be removed */
         chunk ::CALL
         {
         }
diff --git a/rathaxes/samples/e1000/pci.rti b/rathaxes/samples/e1000/pci.rti
--- a/rathaxes/samples/e1000/pci.rti
+++ b/rathaxes/samples/e1000/pci.rti
@@ -18,7 +18,7 @@
         provided chunk  LKM::deinit_bus_hook;
     }
 
-    provided sequence   PCI::probe(PCI::Device)
+    provided sequence   PCI::probe()
     {
         provided chunk  LKM::prototypes;
         provided chunk  LKM::code;
@@ -26,11 +26,11 @@
         provided pointcut   PCI::pci_probe_hook;
     }
 
-    provided sequence   PCI::remove(PCI::Device)
+    provided sequence   PCI::remove()
     {
         provided chunk  LKM::prototypes;
         provided chunk  LKM::code;
 
-        provided pointcut   PCI::pci_remove_hook;
+        provided pointcut   PCI::pci_remove_hook; /* Should take a PCI::Device arg */
     }
 }