comparison rathaxes_sample_e1000_rewrite_device_dependent_code.patch @ 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
comparison
equal deleted inserted replaced
136:8229a46ec658 137:8bbdb488f6fe
1 # HG changeset patch 1 # HG changeset patch
2 # Parent 1229971cfa561a1c788a407620ce545eac7d0141 2 # Parent d71e19169c392a9509bf816bf2ca7858212df878
3 rathaxes: rewrite/refactor all the e1000 device dependent code 3 rathaxes: rewrite/refactor all the e1000 device dependent code
4 4
5 diff --git a/notes.txt b/notes.txt 5 diff --git a/notes.txt b/notes.txt
6 new file mode 100644 6 new file mode 100644
7 --- /dev/null 7 --- /dev/null
8 +++ b/notes.txt 8 +++ b/notes.txt
9 @@ -0,0 +1,40 @@ 9 @@ -0,0 +1,47 @@
10 +Remarks for David & Lionel: 10 +Remarks for David & Lionel:
11 + 11 +
12 +- Too much changes to not start over; 12 +- Too much changes to not start over;
13 +- Lack of methods is extremely annoying and requires a lot of workarounds (e.g: 13 +- Lack of methods is extremely annoying and requires a lot of workarounds (e.g:
14 + see the register read/write/set/unset methods on e1000::Context); 14 + see the register read/write/set/unset methods on e1000::Context);
28 +- Not being able to use a Rathaxes type in the attributes is really annoying 28 +- Not being able to use a Rathaxes type in the attributes is really annoying
29 + and forces me to hardcode a lot of stuff; 29 + and forces me to hardcode a lot of stuff;
30 +- Pointcuts are scoped by subsystems which is kinda weird because different 30 +- Pointcuts are scoped by subsystems which is kinda weird because different
31 + subsystems are going to share the same pointcuts (i.e: the PCI and USB 31 + subsystems are going to share the same pointcuts (i.e: the PCI and USB
32 + susystem are going to expose the same pointcuts that can be used by the 32 + susystem are going to expose the same pointcuts that can be used by the
33 + Ethernet subsystem). 33 + Ethernet subsystem);
34 +- The compiler adds a & when I try to expand e1000::Context.io_addr regardless
35 + of whether the attribute is declared as Builtin::symbol.ref or .scalar.
34 + 36 +
35 +Todo/Totry: 37 +Todo/Totry:
36 + 38 +
37 +- Use the rtx_ether_ctx attribute on AbstractDevice to initialize Device 39 +- Use the rtx_ether_ctx attribute on AbstractDevice to initialize Device
38 + objects in ethernet.bl; 40 + objects in ethernet.bl;
39 +- Worry about the code being executed concurrently, e.g: what happens if the 41 +- Worry about the code being executed concurrently, e.g: what happens if the
40 + interrupt handler is called right before we disable it in the close path? 42 + interrupt handler is called right before we disable it in the close path?
41 +- Why the DMA sequences are taking an AbstractDMAHandle instead of a DMAHandle? 43 +- Why the DMA sequences are taking an AbstractDMAHandle instead of a DMAHandle?
44 +- Right now the Ethernet subsystem hooks into the PCI subsystem via the
45 + Ethernet::init sequence, maybe the content of this sequence should be moved
46 + under the Ethernet::Device type definition (maybe we could solve the
47 + free_netdev call in case of failure at the same time);
48 +- PCI::Device.device should be PCI::Device.k_device.
42 + 49 +
43 +Questions: 50 +Questions:
44 + 51 +
45 +Compiler bugs: 52 +Compiler bugs:
46 + 53 +
680 - decl data_types() 687 - decl data_types()
681 + decl data_types() 688 + decl data_types()
682 { 689 {
683 E1000_TXD_DTYP_D = 0x00100000, /* Data Descriptor */ 690 E1000_TXD_DTYP_D = 0x00100000, /* Data Descriptor */
684 E1000_TXD_DTYP_C = 0x00000000, /* Context Descriptor */ 691 E1000_TXD_DTYP_C = 0x00000000, /* Context Descriptor */
685 @@ -701,326 +140,700 @@ 692 @@ -701,326 +140,701 @@
686 } 693 }
687 } 694 }
688 695
689 - /* TODO: make that a method of e1000::Context */ 696 - /* TODO: make that a method of e1000::Context */
690 - template sequence e1000::print_status(Ethernet::Device rtx_ether_ctx) 697 - template sequence e1000::print_status(Ethernet::Device rtx_ether_ctx)
814 pr_info("\tInterface: %s\n", (status & 3) == 3 ? "Up" : "Down"); 821 pr_info("\tInterface: %s\n", (status & 3) == 3 ? "Up" : "Down");
815 } 822 }
816 + 823 +
817 + static unsigned int rtx_e1000_reg_read32(${e1000::Context} self, ${e1000::Register} reg) 824 + static unsigned int rtx_e1000_reg_read32(${e1000::Context} self, ${e1000::Register} reg)
818 + { 825 + {
819 + return ioread32(${local.self.io_addr} + reg); 826 + // XXX The compiler adds a & if I use ${local.self.io_addr}:
827 + return ioread32(self->io_addr + reg);
820 + } 828 + }
821 + 829 +
822 + static void rtx_e1000_reg_write32(${e1000::Context} self, 830 + static void rtx_e1000_reg_write32(${e1000::Context} self,
823 + ${e1000::Register} reg, 831 + ${e1000::Register} reg,
824 + unsigned int value) 832 + unsigned int value)
825 + { 833 + {
826 + return iowrite32(value, ${local.self.io_addr} + reg); 834 + return iowrite32(value, self->io_addr + reg);
827 + } 835 + }
828 + 836 +
829 + static void rtx_e1000_reg_set32(${e1000::Context} self, 837 + static void rtx_e1000_reg_set32(${e1000::Context} self,
830 + ${e1000::Register} reg, 838 + ${e1000::Register} reg,
831 + unsigned int value) 839 + unsigned int value)
832 + { 840 + {
833 + return iowrite32( 841 + return iowrite32(
834 + rtx_e1000_reg_read32(${local.self}, reg) | value, 842 + rtx_e1000_reg_read32(${local.self}, reg) | value,
835 + ${local.self.io_addr} + reg 843 + self->io_addr + reg
836 + ); 844 + );
837 + } 845 + }
838 + 846 +
839 + static void rtx_e1000_reg_unset32(${e1000::Context} self, 847 + static void rtx_e1000_reg_unset32(${e1000::Context} self,
840 + ${e1000::Register} reg, 848 + ${e1000::Register} reg,
841 + unsigned int value) 849 + unsigned int value)
842 + { 850 + {
843 + return iowrite32(rtx_e1000_reg_read32( 851 + return iowrite32(rtx_e1000_reg_read32(
844 + ${local.self}, reg) & ~value, 852 + ${local.self}, reg) & ~value,
845 + ${local.self.io_addr} + reg 853 + self->io_addr + reg
846 + ); 854 + );
847 + } 855 + }
848 } 856 }
849 857
850 - chunk ::CALL() 858 - chunk ::CALL()
1709 + // hardcode generated types in the definition of the type, and they 1717 + // hardcode generated types in the definition of the type, and they
1710 + // can't be "dereferenced" (e.g: local.hw_ctx.rx_ring.desc_size can't 1718 + // can't be "dereferenced" (e.g: local.hw_ctx.rx_ring.desc_size can't
1711 + // work since local.hw_ctx.rx_ring is just going to be a symbol). 1719 + // work since local.hw_ctx.rx_ring is just going to be a symbol).
1712 + attribute Builtin::symbol.scalar rx_ring; 1720 + attribute Builtin::symbol.scalar rx_ring;
1713 + attribute Builtin::symbol.scalar tx_ring; 1721 + attribute Builtin::symbol.scalar tx_ring;
1714 + attribute Builtin::symbol.scalar io_addr; 1722 + attribute Builtin::symbol.ref io_addr;
1715 + attribute Ethernet::Device.ref net_dev; 1723 + attribute Ethernet::Device.ref net_dev;
1716 + } 1724 + }
1717 + 1725 +
1718 + provided type Ring 1726 + provided type Ring
1719 + { 1727 + {
2151 - static const struct net_device_ops rtx_ether_ops = 2159 - static const struct net_device_ops rtx_ether_ops =
2152 + static const struct net_device_ops rtx_ether_ops = 2160 + static const struct net_device_ops rtx_ether_ops =
2153 { 2161 {
2154 .ndo_open = rtx_ethernet_open, 2162 .ndo_open = rtx_ethernet_open,
2155 .ndo_stop = rtx_ethernet_close, 2163 .ndo_stop = rtx_ethernet_close,
2156 @@ -298,64 +320,62 @@ 2164 @@ -288,74 +310,65 @@
2157 */ 2165 };
2166 }
2167
2168 - /*
2169 - * NOTE: for now, the error handling is leaking from PCI::probe(), but
2170 - * it's better than doing it at all.
2171 - *
2172 - * XXX: the chunk argument isn't correctly expanded by the
2173 - * compiler I have to use the same name as the actual C
2174 - * variable:
2175 - */
2176 + // NOTE: for now, the error handling is leaking from PCI::probe(), but
2177 + // it's better than not doing it at all.
2178 + //
2179 + // XXX: the chunk argument isn't correctly expanded by the compiler I
2180 + // have to use the same name as the actual C variable:
2158 chunk PCI::pci_probe_hook(PCI::Device rtx_pci_dev) 2181 chunk PCI::pci_probe_hook(PCI::Device rtx_pci_dev)
2159 { 2182 {
2160 - ${Ethernet::AbstractDevice.ref} rtx_net_dev; 2183 - ${Ethernet::AbstractDevice.ref} rtx_net_dev;
2161 - ${Ethernet::Device.ref} rtx_ether_ctx; 2184 - ${Ethernet::Device.ref} rtx_ether_ctx;
2162 + { 2185 + {
2174 + { 2197 + {
2175 + ${Log::info("cannot allocate the ethernet device context")}; 2198 + ${Log::info("cannot allocate the ethernet device context")};
2176 + error = -ENOMEM; 2199 + error = -ENOMEM;
2177 + goto fail; 2200 + goto fail;
2178 + } 2201 + }
2202 + // XXX Past this point we should have another label to do
2203 + // free_netdev(rtx_net_dev) in case of failure but we don't
2204 + // have a nice way of doing that.
2179 + SET_NETDEV_DEV(${local.rtx_net_dev.k_net_dev}, ${rtx_pci_dev.device}); 2205 + SET_NETDEV_DEV(${local.rtx_net_dev.k_net_dev}, ${rtx_pci_dev.device});
2180 + strlcpy(${local.rtx_net_dev.k_net_dev}->name, 2206 + strlcpy(${local.rtx_net_dev.k_net_dev}->name,
2181 + ${config.ifname}, 2207 + ${config.ifname},
2182 + sizeof(${local.rtx_net_dev.k_net_dev}->name)); 2208 + sizeof(${local.rtx_net_dev.k_net_dev}->name));
2183 + ${local.rtx_net_dev.k_net_dev}->irq = ${rtx_pci_dev.irq}; 2209 + ${local.rtx_net_dev.k_net_dev}->irq = ${rtx_pci_dev.irq};
2189 + ${Log::info("cannot register the driver in the net subsystem")}; 2215 + ${Log::info("cannot register the driver in the net subsystem")};
2190 + goto fail; 2216 + goto fail;
2191 + } 2217 + }
2192 + 2218 +
2193 + /* Initialize our context held by the net_device structure */ 2219 + /* Initialize our context held by the net_device structure */
2194 + /*
2195 + * XXX: the cast is here because the compiler resolve the
2196 + * type of rtx_pci_dev.pci_device to the type of
2197 + * rtx_pci_dev instead of the type of rtx_pci_dev.pci_device.
2198 + */
2199 + ${PCI::AbstractDevice.ref} workaround = ${rtx_pci_dev.pci_device};
2200 + ${cast local.workaround as PCI::AbstractDevice};
2201 + { /* XXX: I end up with a placeholder if I don't open a scope */ 2220 + { /* XXX: I end up with a placeholder if I don't open a scope */
2202 + ${local.rtx_ether_ctx.init(local.rtx_net_dev, local.workaround)}; 2221 + ${local.rtx_ether_ctx.init(local.rtx_net_dev, rtx_pci_dev.pci_device)};
2203 + } 2222 + }
2204 + 2223 +
2205 + /* Register ourselves in the parent context: */ 2224 + /* Register ourselves in the parent context: */
2206 + /* ${rtx_pci_dev.set_context(local.rtx_ether_ctx)}; */ 2225 + /* ${rtx_pci_dev.set_context(local.rtx_ether_ctx)}; */
2207 + ${rtx_pci_dev}->context = rtx_ether_ctx; 2226 + ${rtx_pci_dev.rtx_drv_context} = rtx_ether_ctx;
2208 + 2227 +
2209 + /* 2228 + /*
2210 + * XXX: the asssignments/casts are here to circumvent 2229 + * XXX: the asssignments/casts are here to circumvent
2211 + * typing issues in the compiler (see previous XXX). 2230 + * typing issues in the compiler (see previous XXX).
2212 + */ 2231 + */
2268 - /* This chunk should be removed (see #26) */ 2287 - /* This chunk should be removed (see #26) */
2269 + /* XXX This chunk should be removed (see #26) */ 2288 + /* XXX This chunk should be removed (see #26) */
2270 chunk ::CALL() 2289 chunk ::CALL()
2271 { 2290 {
2272 } 2291 }
2273 @@ -369,15 +389,17 @@ 2292 @@ -369,15 +382,17 @@
2274 */ 2293 */
2275 chunk PCI::pci_remove_hook(PCI::Device rtx_pci_dev) 2294 chunk PCI::pci_remove_hook(PCI::Device rtx_pci_dev)
2276 { 2295 {
2277 - ${Ethernet::Device.ref} rtx_ether_ctx = ${rtx_pci_dev.rtx_drv_context}; 2296 - ${Ethernet::Device.ref} rtx_ether_ctx = ${rtx_pci_dev.rtx_drv_context};
2278 - BUG_ON(!rtx_ether_ctx); 2297 - BUG_ON(!rtx_ether_ctx);
2527 ${Log::info("adapter_init_rx: cannot dma-map a skbuff for the rx ring")}; 2546 ${Log::info("adapter_init_rx: cannot dma-map a skbuff for the rx ring")};
2528 goto err_skbuffs_map; 2547 goto err_skbuffs_map;
2529 diff --git a/rathaxes/samples/e1000/e1000.rti b/rathaxes/samples/e1000/old_e1000.rti 2548 diff --git a/rathaxes/samples/e1000/e1000.rti b/rathaxes/samples/e1000/old_e1000.rti
2530 copy from rathaxes/samples/e1000/e1000.rti 2549 copy from rathaxes/samples/e1000/e1000.rti
2531 copy to rathaxes/samples/e1000/old_e1000.rti 2550 copy to rathaxes/samples/e1000/old_e1000.rti
2551 diff --git a/rathaxes/samples/e1000/pci.blt b/rathaxes/samples/e1000/pci.blt
2552 --- a/rathaxes/samples/e1000/pci.blt
2553 +++ b/rathaxes/samples/e1000/pci.blt
2554 @@ -73,9 +73,8 @@
2555
2556 method init(PCI::AbstractDevice pdev)
2557 {
2558 - ${PCI::AbstractDevice.ref} workaround = (${PCI::AbstractDevice.ref})pdev;
2559 ${self}->pdev = ${pdev};
2560 - ${self}->bars = pci_select_bars(${local.workaround.k_pci_dev}, IORESOURCE_MEM);
2561 + ${self}->bars = pci_select_bars(${pdev.k_pci_dev}, IORESOURCE_MEM);
2562 ${self}->ioaddr = NULL;
2563 ${self}->context = NULL;
2564 }
2565 @@ -98,7 +97,7 @@
2566
2567 method set_rtx_drv_context(Builtin::symbol ctx)
2568 {
2569 - ${self}->context = ctx;
2570 + ${self.rtx_drv_context} = ${ctx};
2571 }
2572
2573 map
2574 @@ -127,7 +126,7 @@
2575 const struct pci_device_id *pdev_id)
2576 {
2577 int error;
2578 - ${PCI::Device.ref} rtx_pci_dev;
2579 + ${PCI::Device.ref} rtx_pci_dev = NULL;
2580 ${PCI::AbstractDevice.ref} rtx_pdev = (${PCI::AbstractDevice.ref})pdev;
2581
2582 rtx_pci_dev = kmalloc(sizeof(*rtx_pci_dev), GFP_KERNEL);
2532 diff --git a/rathaxes/samples/e1000/socket.blt b/rathaxes/samples/e1000/socket.blt 2583 diff --git a/rathaxes/samples/e1000/socket.blt b/rathaxes/samples/e1000/socket.blt
2533 --- a/rathaxes/samples/e1000/socket.blt 2584 --- a/rathaxes/samples/e1000/socket.blt
2534 +++ b/rathaxes/samples/e1000/socket.blt 2585 +++ b/rathaxes/samples/e1000/socket.blt
2535 @@ -22,16 +22,12 @@ 2586 @@ -22,16 +22,12 @@
2536 { 2587 {