# HG changeset patch # User Louis Opter # Date 1329488128 -3600 # Node ID b43bed449cc2ad135664e482a721f7b64104a6a5 # Parent ebe4df228703fa8239d1f904a3d90b0969c8080b Start a series on the e1000 sample diff -r ebe4df228703 -r b43bed449cc2 rathaxes_correctly_use_chunk_and_template_sequences_parameters_in_e1000.patch --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/rathaxes_correctly_use_chunk_and_template_sequences_parameters_in_e1000.patch Fri Feb 17 15:15:28 2012 +0100 @@ -0,0 +1,290 @@ +# HG changeset patch +# Parent d759597ad67d463fa77467b2443120315980c70b +# User Louis Opter , David Pineau +rathaxes: correctly use chunk and template sequences parameters in the e1000 sample + +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,7 +63,7 @@ + { + 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}; +@@ -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,53 +172,42 @@ + /* + * 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)}; ++ ++ pci_set_drvdata(${pdev}, net_dev); + + ${pointcut Ethernet::create_device}; + } + ++ /* This chunk should be remove (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); ++ struct net_device *net_dev = pci_get_drvdata(${pdev}); + + ${pointcut Ethernet::destroy_device}; + +@@ -210,6 +219,7 @@ + free_netdev(net_dev); + } + ++ /* This chunk should be remove (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,18 +28,16 @@ + 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 pointcut Ethernet::create_device; + } + +- provided sequence Ethernet::exit(PCI::Device) ++ provided sequence Ethernet::exit() + { +- provided chunk ::CALL; + provided chunk PCI::pci_remove_hook; + + provided pointcut Ethernet::destroy_device; +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,7 +26,7 @@ + provided pointcut PCI::pci_probe_hook; + } + +- provided sequence PCI::remove(PCI::Device) ++ provided sequence PCI::remove() + { + provided chunk LKM::prototypes; + provided chunk LKM::code; diff -r ebe4df228703 -r b43bed449cc2 series --- a/series Fri Feb 17 15:12:31 2012 +0100 +++ b/series Fri Feb 17 15:15:28 2012 +0100 @@ -0,0 +1,1 @@ +rathaxes_correctly_use_chunk_and_template_sequences_parameters_in_e1000.patch