Re: [2.6.30-rc3] powerpc: compilation error of mace module

Re: [2.6.30-rc3] powerpc: compilation error of mace module

[David Miller]

From: Rogério Brito <rbrito@ime.usp.br>
Date: Sun, 26 Apr 2009 12:57:09 -0300

> I am attempting to compile a new kernel for my OldWorld ppc box and I'm
> having some problems. The first one that appears to happen is the following:

Turn on CONFIG_COMPAT_NET_DEV_OPS in your kernel config.

There was no reason for you to turn that off, and you had
to have done it explicitly I think :-)

Or, if you're bored, feel free to convert the mace driver over
to netdev_ops :-)

[PATCH] powerpc: convert mace to netdev_ops (was: Re: [2.6.30-rc3] powerpc: compilation error of mace module)

[Rogério Brito]

Hi, Dave.

On Apr 26 2009, David Miller wrote:
> Or, if you're bored, feel free to convert the mace driver over
> to netdev_ops :-)

Is this anything close to what needs to be done? It's not without
failures, because the function mace_set_timeout receives a pointer to a
struct net_device, but is marked inline and is used by mace_tx_timeout,
which receives an unsigned long (which calls mace_set_timeout).

Perhaps it would be a case of removing the inline hint to the compiler?
I guess that BenH or Paul could comment here better...


Signed-off-by: Rogério Brito <rbrito@ime.usp.br>

---
--- a/drivers/net/mace.c	2008-12-29 15:25:15.000000000 -0200
+++ b/drivers/net/mace.c	2009-04-27 08:54:16.000000000 -0300
@@ -89,6 +89,16 @@ static inline void dbdma_reset(volatile 
 static inline void mace_clean_rings(struct mace_data *mp);
 static void __mace_set_address(struct net_device *dev, void *addr);
 
+/* Conversion to netdev_ops. */
+static const struct net_device_ops mace_netdev_ops = {
+	.ndo_open		= mace_open,
+	.ndo_stop		= mace_close,
+	.ndo_start_xmit		= mace_xmit_start,
+	.ndo_tx_timeout		= mace_set_timeout,
+	.ndo_set_multicast_list	= mace_set_multicast,
+	.ndo_set_mac_address	= mace_set_address,
+};
+
 /*
  * If we can't get a skbuff when we need it, we use this area for DMA.
  */
@@ -208,11 +217,7 @@ static int __devinit mace_probe(struct m
 		}
 	}
 
-	dev->open = mace_open;
-	dev->stop = mace_close;
-	dev->hard_start_xmit = mace_xmit_start;
-	dev->set_multicast_list = mace_set_multicast;
-	dev->set_mac_address = mace_set_address;
+	dev->netdev_ops = &mace_netdev_ops;
 
 	/*
 	 * Most of what is below could be moved to mace_open()

-- 
Rogério Brito : rbrito@{mackenzie,ime.usp}.br : GPG key 1024D/7C2CAEB8
http://www.ime.usp.br/~rbrito : http://meusite.mackenzie.com.br/rbrito
Projects: algorithms.berlios.de : lame.sf.net : vrms.alioth.debian.org
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: convert mace to netdev_ops

[David Miller]

From: Rogério Brito <rbrito@ime.usp.br>
Date: Mon, 27 Apr 2009 09:16:33 -0300

> Is this anything close to what needs to be done? It's not without
> failures, because the function mace_set_timeout receives a pointer to a
> struct net_device, but is marked inline and is used by mace_tx_timeout,
> which receives an unsigned long (which calls mace_set_timeout).
> 
> Perhaps it would be a case of removing the inline hint to the compiler?
> I guess that BenH or Paul could comment here better...
> 
> Signed-off-by: Rogério Brito <rbrito@ime.usp.br>

You can fix the mace_set_timeout() function arguments by having
a helper function that simply wraps around it and provides the
second expection of argument types.

Your patch is also wrong, it's missing a lot of netdev_ops
entries that are implicitly obtained via alloc_etherdev(),
namely:

	.ndo_change_mtu		= eth_change_mtu,
	.ndo_set_mac_address 	= eth_mac_addr,
	.ndo_validate_addr	= eth_validate_addr,

Re: [PATCH] powerpc: convert mace to netdev_ops

[Rogério Brito]

Hi, Dave.

On Apr 27 2009, David Miller wrote:
> You can fix the mace_set_timeout() function arguments by having a
> helper function that simply wraps around it and provides the second
> expection of argument types.

Hummm, this means that I'm not that bad... The wrapper function was the
first thing that came to my mind, but I just wanted to be as least
disruptive as possible.

> Your patch is also wrong, it's missing a lot of netdev_ops
> entries that are implicitly obtained via alloc_etherdev(),
> namely:

Thanks for pointing those out. I didn't find the documentation about
netdev_ops under Documentation (a simple grep didn't turn any results).

> 	.ndo_change_mtu		= eth_change_mtu,
> 	.ndo_set_mac_address 	= eth_mac_addr,
> 	.ndo_validate_addr	= eth_validate_addr,

Nice. I will incorporate such things.


Thanks, Rogério Brito.

-- 
Rogério Brito : rbrito@{mackenzie,ime.usp}.br : GPG key 1024D/7C2CAEB8
http://www.ime.usp.br/~rbrito : http://meusite.mackenzie.com.br/rbrito
Projects: algorithms.berlios.de : lame.sf.net : vrms.alioth.debian.org
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] powerpc: convert mace to netdev_ops

[Rogério Brito]

Hi, Dave.

On Apr 27 2009, David Miller wrote:
> You can fix the mace_set_timeout() function arguments by having
> a helper function that simply wraps around it and provides the
> second expection of argument types.

I hope that this version is a slightly better fix to convert mace to
netdev_ops.

This is against this morning's net-2.6 tree.


Signed-off-by: Rogério Brito <rbrito@ime.usp.br>
---
diff --git a/drivers/net/mace.c b/drivers/net/mace.c
index feebbd9..03a179d 100644
--- a/drivers/net/mace.c
+++ b/drivers/net/mace.c
@@ -89,6 +89,19 @@ static inline void dbdma_reset(volatile struct dbdma_regs __iomem *dma);
 static inline void mace_clean_rings(struct mace_data *mp);
 static void __mace_set_address(struct net_device *dev, void *addr);
 
+/* Conversion to netdev_ops. */
+static const struct net_device_ops mace_netdev_ops = {
+	.ndo_open		= mace_open,
+	.ndo_stop		= mace_close,
+	.ndo_start_xmit		= mace_xmit_start,
+	.ndo_tx_timeout		= mace_tx_timeout,
+	.ndo_set_multicast_list	= mace_set_multicast,
+	.ndo_set_mac_address	= mace_set_address,
+	.ndo_change_mtu		= eth_change_mtu,
+	.ndo_set_mac_address	= eth_mac_addr,
+	.ndo_validate_addr	= eth_validate_addr,
+};
+
 /*
  * If we can't get a skbuff when we need it, we use this area for DMA.
  */
@@ -207,11 +220,7 @@ static int __devinit mace_probe(struct macio_dev *mdev, const struct of_device_i
 		}
 	}
 
-	dev->open = mace_open;
-	dev->stop = mace_close;
-	dev->hard_start_xmit = mace_xmit_start;
-	dev->set_multicast_list = mace_set_multicast;
-	dev->set_mac_address = mace_set_address;
+	dev->netdev_ops = &mace_netdev_ops;
 
 	/*
 	 * Most of what is below could be moved to mace_open()
@@ -798,6 +807,13 @@ static irqreturn_t mace_interrupt(int irq, void *dev_id)
     return IRQ_HANDLED;
 }
 
+/*
+ * In the following, the parameter "data" is treated like a pointer,
+ * which is probably OK for 32 bit arches, but not for 64.
+ *
+ * (Are mace's found on any newer machines??) -- rbrito
+ *
+ */
 static void mace_tx_timeout(unsigned long data)
 {
     struct net_device *dev = (struct net_device *) data;


-- 
Rogério Brito : rbrito@{mackenzie,ime.usp}.br : GPG key 1024D/7C2CAEB8
http://www.ime.usp.br/~rbrito : http://meusite.mackenzie.com.br/rbrito
Projects: algorithms.berlios.de : lame.sf.net : vrms.alioth.debian.org

Re: [PATCH] powerpc: convert mace to netdev_ops

[David Miller]

From: Rogério Brito <rbrito@ime.usp.br>
Date: Sun, 3 May 2009 08:48:20 -0300

> I hope that this version is a slightly better fix to convert mace to
> netdev_ops.
> 
> This is against this morning's net-2.6 tree.
> 
> Signed-off-by: Rogério Brito <rbrito@ime.usp.br>

I'll apply this, thanks!  I made one change however.

> @@ -798,6 +807,13 @@ static irqreturn_t mace_interrupt(int irq, void *dev_id)
>      return IRQ_HANDLED;
>  }
>  
> +/*
> + * In the following, the parameter "data" is treated like a pointer,
> + * which is probably OK for 32 bit arches, but not for 64.
> + *
> + * (Are mace's found on any newer machines??) -- rbrito
> + *
> + */
>  static void mace_tx_timeout(unsigned long data)
>  {
>      struct net_device *dev = (struct net_device *) data;

I left this new comment out, as this is a common idiom (passing opaque
data as an 'unsigned long' argument to a callback) and casting it to a
pointer.

It also works perfectly fine on all 32-bit and 64-bit platforms.

Re: [PATCH] powerpc: convert mace to netdev_ops

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Sun, 03 May 2009 14:15:24 -0700 (PDT)

> From: Rogério Brito <rbrito@ime.usp.br>
> Date: Sun, 3 May 2009 08:48:20 -0300
> 
>> I hope that this version is a slightly better fix to convert mace to
>> netdev_ops.
>> 
>> This is against this morning's net-2.6 tree.
>> 
>> Signed-off-by: Rogério Brito <rbrito@ime.usp.br>
> 
> I'll apply this, thanks!  I made one change however.

Oh, nevermind, it seems that MACE was already converted
to netdev_ops some time ago in the net-next-2.6 tree.