# HG changeset patch
# User Louis Opter <kalessin@kalessin.fr>
# Date 1390096353 28800
# Node ID 8bbdb488f6fe811a21982ec079f3352c8ee065d4
# Parent  8229a46ec65892dac50d15f1210d25a3d319a6c3
Wip, cleaning up the probe function

diff -r 8229a46ec658 -r 8bbdb488f6fe rathaxes_sample_e1000_rewrite_device_dependent_code.patch
--- a/rathaxes_sample_e1000_rewrite_device_dependent_code.patch	Sun Jan 12 18:33:09 2014 -0800
+++ b/rathaxes_sample_e1000_rewrite_device_dependent_code.patch	Sat Jan 18 17:52:33 2014 -0800
@@ -1,12 +1,12 @@
 # HG changeset patch
-# Parent 1229971cfa561a1c788a407620ce545eac7d0141
+# Parent d71e19169c392a9509bf816bf2ca7858212df878
 rathaxes: rewrite/refactor all the e1000 device dependent code
 
 diff --git a/notes.txt b/notes.txt
 new file mode 100644
 --- /dev/null
 +++ b/notes.txt
-@@ -0,0 +1,40 @@
+@@ -0,0 +1,47 @@
 +Remarks for David & Lionel:
 +
 +- Too much changes to not start over;
@@ -30,7 +30,9 @@
 +- Pointcuts are scoped by subsystems which is kinda weird because different
 +  subsystems are going to share the same pointcuts (i.e: the PCI and USB
 +  susystem are going to expose the same pointcuts that can be used by the
-+  Ethernet subsystem).
++  Ethernet subsystem);
++- The compiler adds a & when I try to expand e1000::Context.io_addr regardless
++  of whether the attribute is declared as Builtin::symbol.ref or .scalar.
 +
 +Todo/Totry:
 +
@@ -39,6 +41,11 @@
 +- Worry about the code being executed concurrently, e.g: what happens if the
 +  interrupt handler is called right before we disable it in the close path?
 +- Why the DMA sequences are taking an AbstractDMAHandle instead of a DMAHandle?
++- Right now the Ethernet subsystem hooks into the PCI subsystem via the
++  Ethernet::init sequence, maybe the content of this sequence should be moved
++  under the Ethernet::Device type definition (maybe we could solve the
++  free_netdev call in case of failure at the same time);
++- PCI::Device.device should be PCI::Device.k_device.
 +
 +Questions:
 +
@@ -682,7 +689,7 @@
          {
              E1000_TXD_DTYP_D        = 0x00100000, /* Data Descriptor */
              E1000_TXD_DTYP_C        = 0x00000000, /* Context Descriptor */
-@@ -701,326 +140,700 @@
+@@ -701,326 +140,701 @@
          }
      }
  
@@ -816,14 +823,15 @@
 +
 +            static unsigned int rtx_e1000_reg_read32(${e1000::Context} self, ${e1000::Register} reg)
 +            {
-+                return ioread32(${local.self.io_addr} + reg);
++                // XXX The compiler adds a & if I use ${local.self.io_addr}:
++                return ioread32(self->io_addr + reg);
 +            }
 +
 +            static void rtx_e1000_reg_write32(${e1000::Context} self,
 +                                              ${e1000::Register} reg,
 +                                              unsigned int value)
 +            {
-+                return iowrite32(value, ${local.self.io_addr} + reg);
++                return iowrite32(value, self->io_addr + reg);
 +            }
 +
 +            static void rtx_e1000_reg_set32(${e1000::Context} self,
@@ -832,7 +840,7 @@
 +            {
 +                return iowrite32(
 +                    rtx_e1000_reg_read32(${local.self}, reg) | value,
-+                    ${local.self.io_addr} + reg
++                    self->io_addr + reg
 +                );
 +            }
 +
@@ -842,7 +850,7 @@
 +            {
 +                return iowrite32(rtx_e1000_reg_read32(
 +                    ${local.self}, reg) & ~value,
-+                    ${local.self.io_addr} + reg
++                    self->io_addr + reg
 +                );
 +            }
          }
@@ -1711,7 +1719,7 @@
 +        // work since local.hw_ctx.rx_ring is just going to be a symbol).
 +        attribute   Builtin::symbol.scalar  rx_ring;
 +        attribute   Builtin::symbol.scalar  tx_ring;
-+        attribute   Builtin::symbol.scalar  io_addr;
++        attribute   Builtin::symbol.ref     io_addr;
 +        attribute   Ethernet::Device.ref    net_dev;
 +    }
 +
@@ -2153,8 +2161,23 @@
              {
                  .ndo_open = rtx_ethernet_open,
                  .ndo_stop = rtx_ethernet_close,
-@@ -298,64 +320,62 @@
-          */
+@@ -288,74 +310,65 @@
+             };
+         }
+ 
+-        /*
+-         * NOTE: for now, the error handling is leaking from PCI::probe(), but
+-         * it's better than doing it at all.
+-         *
+-         * XXX: the chunk argument isn't correctly expanded by the
+-         * compiler I have to use the same name as the actual C
+-         * variable:
+-         */
++        // NOTE: for now, the error handling is leaking from PCI::probe(), but
++        // it's better than not doing it at all.
++        //
++        // XXX: the chunk argument isn't correctly expanded by the compiler I
++        // have to use the same name as the actual C variable:
          chunk PCI::pci_probe_hook(PCI::Device rtx_pci_dev)
          {
 -            ${Ethernet::AbstractDevice.ref} rtx_net_dev;
@@ -2176,6 +2199,9 @@
 +                    error = -ENOMEM;
 +                    goto fail;
 +                }
++                // XXX Past this point we should have another label to do
++                // free_netdev(rtx_net_dev) in case of failure but we don't
++                // have a nice way of doing that.
 +                SET_NETDEV_DEV(${local.rtx_net_dev.k_net_dev}, ${rtx_pci_dev.device});
 +                strlcpy(${local.rtx_net_dev.k_net_dev}->name,
 +                        ${config.ifname},
@@ -2191,20 +2217,13 @@
 +                }
 +
 +                /* Initialize our context held by the net_device structure */
-+                /*
-+                 * XXX: the cast is here because the compiler resolve the
-+                 * type of rtx_pci_dev.pci_device to the type of
-+                 * rtx_pci_dev instead of the type of rtx_pci_dev.pci_device.
-+                 */
-+                ${PCI::AbstractDevice.ref} workaround = ${rtx_pci_dev.pci_device};
-+                ${cast local.workaround as PCI::AbstractDevice};
 +                { /* XXX: I end up with a placeholder if I don't open a scope */
-+                    ${local.rtx_ether_ctx.init(local.rtx_net_dev, local.workaround)};
++                    ${local.rtx_ether_ctx.init(local.rtx_net_dev, rtx_pci_dev.pci_device)};
 +                }
 +
 +                /* Register ourselves in the parent context: */
 +                /* ${rtx_pci_dev.set_context(local.rtx_ether_ctx)}; */
-+                ${rtx_pci_dev}->context = rtx_ether_ctx;
++                ${rtx_pci_dev.rtx_drv_context} = rtx_ether_ctx;
 +
 +                /*
 +                 * XXX: the asssignments/casts are here to circumvent
@@ -2270,7 +2289,7 @@
          chunk   ::CALL()
          {
          }
-@@ -369,15 +389,17 @@
+@@ -369,15 +382,17 @@
           */
          chunk   PCI::pci_remove_hook(PCI::Device rtx_pci_dev)
          {
@@ -2529,6 +2548,38 @@
 diff --git a/rathaxes/samples/e1000/e1000.rti b/rathaxes/samples/e1000/old_e1000.rti
 copy from rathaxes/samples/e1000/e1000.rti
 copy to rathaxes/samples/e1000/old_e1000.rti
+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
+@@ -73,9 +73,8 @@
+ 
+         method  init(PCI::AbstractDevice pdev)
+         {
+-            ${PCI::AbstractDevice.ref} workaround = (${PCI::AbstractDevice.ref})pdev;
+             ${self}->pdev = ${pdev};
+-            ${self}->bars = pci_select_bars(${local.workaround.k_pci_dev}, IORESOURCE_MEM);
++            ${self}->bars = pci_select_bars(${pdev.k_pci_dev}, IORESOURCE_MEM);
+             ${self}->ioaddr = NULL;
+             ${self}->context = NULL;
+         }
+@@ -98,7 +97,7 @@
+ 
+         method  set_rtx_drv_context(Builtin::symbol ctx)
+         {
+-            ${self}->context = ctx;
++            ${self.rtx_drv_context} = ${ctx};
+         }
+ 
+         map
+@@ -127,7 +126,7 @@
+                                                       const struct pci_device_id *pdev_id)
+             {
+                 int error;
+-                ${PCI::Device.ref} rtx_pci_dev;
++                ${PCI::Device.ref} rtx_pci_dev = NULL;
+                 ${PCI::AbstractDevice.ref} rtx_pdev = (${PCI::AbstractDevice.ref})pdev;
+ 
+                 rtx_pci_dev = kmalloc(sizeof(*rtx_pci_dev), GFP_KERNEL);
 diff --git a/rathaxes/samples/e1000/socket.blt b/rathaxes/samples/e1000/socket.blt
 --- a/rathaxes/samples/e1000/socket.blt
 +++ b/rathaxes/samples/e1000/socket.blt
diff -r 8229a46ec658 -r 8bbdb488f6fe series