Mercurial > archived > louis > epitech > mq > rathaxes
changeset 137:8bbdb488f6fe
Wip, cleaning up the probe function
| author | Louis Opter <kalessin@kalessin.fr> |
|---|---|
| date | Sat, 18 Jan 2014 17:52:33 -0800 |
| parents | 8229a46ec658 |
| children | 6527e078252c |
| files | rathaxes_sample_e1000_rewrite_device_dependent_code.patch series |
| diffstat | 1 files changed, 72 insertions(+), 21 deletions(-) [+] |
line wrap: on
line diff
--- 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