Re: [patch 10/15] m68k: Some network drivers do not check the platform

Re: [patch 10/15] m68k: Some network drivers do not check the platform

[Brad Boyer]

On Thu, Apr 24, 2008 at 08:51:17PM +0200, Geert Uytterhoeven wrote:
> On Thu, 24 Apr 2008, Christoph Hellwig wrote:
> > On Thu, Apr 24, 2008 at 08:29:50PM +0200, Geert Uytterhoeven wrote:
> > > +++ b/drivers/net/macmace.c
> > > @@ -781,6 +781,9 @@ static int __init mac_mace_init_module(v
> > >  {
> > >  	int err;
> > >  
> > > +	if (!MACH_IS_MAC)
> > > +		return -ENODEV;
> > > +
> > >  	if ((err = platform_driver_register(&mac_mace_driver))) {
> > >  		printk(KERN_ERR "Driver registration failed\n");
> > >  		return err;
> > 
> 
> All these drivers predate the platform device framework and have been converted
> to this framework a while ago, without separating the actual platform drivers
> and platform devices.

At some point I intend to merge macmace.c into mace.c by using the macio
bus code on m68k macs. This would avoid the need for these checks, but
it's not something I have working yet. My first driver to merge with ppc
would be serial since we don't need any DMA support for that. It's on my
list of things to do once I get macio working on m68k. I'll be sure to
send a patch to the m68k list as soon as I have something that runs.
After that I'll copy it to the ppc list to make sure I didn't break that,
and submit it for inclusion once people are satisfied.

	Brad Boyer
	flar@allandria.com

[patch 10/15] m68k: Some network drivers do not check the platform

[Geert Uytterhoeven]

[-- Attachment #1: m68k-net-drivers-break-multiplatform.diff --]
[-- Type: text/plain, Size: 2756 bytes --]

From: Geert Uytterhoeven <geert@linux-m68k.org>

Some network drivers do not check whether they're actually running on the
correct platform, causing multi-platform kernels to crash if they are not.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/82596.c     |    7 +++++++
 drivers/net/apne.c      |    3 +++
 drivers/net/mac89x0.c   |    3 +++
 drivers/net/macmace.c   |    3 +++
 drivers/net/sun3lance.c |    3 +++
 5 files changed, 19 insertions(+)

--- a/drivers/net/82596.c
+++ b/drivers/net/82596.c
@@ -1162,6 +1162,7 @@ struct net_device * __init i82596_probe(
 		memcpy(eth_addr, (void *) 0xfffc1f2c, 6);	/* YUCK! Get addr from NOVRAM */
 		dev->base_addr = MVME_I596_BASE;
 		dev->irq = (unsigned) MVME16x_IRQ_I596;
+		goto found;
 	}
 #endif
 #ifdef ENABLE_BVME6000_NET
@@ -1176,6 +1177,7 @@ struct net_device * __init i82596_probe(
 		rtc[3] = msr;
 		dev->base_addr = BVME_I596_BASE;
 		dev->irq = (unsigned) BVME_IRQ_I596;
+		goto found;
 	}
 #endif
 #ifdef ENABLE_APRICOT
@@ -1212,8 +1214,13 @@ struct net_device * __init i82596_probe(
 		}
 
 		dev->irq = 10;
+		goto found;
 	}
 #endif
+	err = -ENODEV;
+	goto out;
+
+found:
 	dev->mem_start = (int)__get_free_pages(GFP_ATOMIC, 0);
 	if (!dev->mem_start) {
 		err = -ENOMEM;
--- a/drivers/net/apne.c
+++ b/drivers/net/apne.c
@@ -127,6 +127,9 @@ struct net_device * __init apne_probe(in
 #endif
 	int err;
 
+	if (!MACH_IS_AMIGA)
+		return ERR_PTR(-ENODEV);
+
 	if (apne_owned)
 		return ERR_PTR(-ENODEV);
 
--- a/drivers/net/mac89x0.c
+++ b/drivers/net/mac89x0.c
@@ -183,6 +183,9 @@ struct net_device * __init mac89x0_probe
 	int err = -ENODEV;
 	DECLARE_MAC_BUF(mac);
 
+	if (!MACH_IS_MAC)
+		return ERR_PTR(-ENODEV);
+
 	dev = alloc_etherdev(sizeof(struct net_local));
 	if (!dev)
 		return ERR_PTR(-ENOMEM);
--- a/drivers/net/macmace.c
+++ b/drivers/net/macmace.c
@@ -781,6 +781,9 @@ static int __init mac_mace_init_module(v
 {
 	int err;
 
+	if (!MACH_IS_MAC)
+		return -ENODEV;
+
 	if ((err = platform_driver_register(&mac_mace_driver))) {
 		printk(KERN_ERR "Driver registration failed\n");
 		return err;
--- a/drivers/net/sun3lance.c
+++ b/drivers/net/sun3lance.c
@@ -250,6 +250,9 @@ struct net_device * __init sun3lance_pro
 	static int found;
 	int err = -ENODEV;
 
+	if (!MACH_IS_SUN3 && !MACH_IS_SUN3X)
+		return ERR_PTR(-ENODEV);
+
 	/* check that this machine has an onboard lance */
 	switch(idprom->id_machtype) {
 	case SM_SUN3|SM_3_50:

-- 
Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [patch 10/15] m68k: Some network drivers do not check the platform

[Christoph Hellwig]

On Thu, Apr 24, 2008 at 08:29:50PM +0200, Geert Uytterhoeven wrote:
> +++ b/drivers/net/macmace.c
> @@ -781,6 +781,9 @@ static int __init mac_mace_init_module(v
>  {
>  	int err;
>  
> +	if (!MACH_IS_MAC)
> +		return -ENODEV;
> +
>  	if ((err = platform_driver_register(&mac_mace_driver))) {
>  		printk(KERN_ERR "Driver registration failed\n");
>  		return err;

This is rather odd.  The whole point of the platform_driver
infrastructure is that the arch code only registers it if it's present
and then the driver can match for it.  So the right fix here is to move
the platform_driver_register(&mac_mace_driver) into the mac arch setup
code.  A quick grep shows this is a quite common pattern for m68k
drivers.

Re: [patch 10/15] m68k: Some network drivers do not check the platform

[Geert Uytterhoeven]

On Thu, 24 Apr 2008, Christoph Hellwig wrote:
> On Thu, Apr 24, 2008 at 08:29:50PM +0200, Geert Uytterhoeven wrote:
> > +++ b/drivers/net/macmace.c
> > @@ -781,6 +781,9 @@ static int __init mac_mace_init_module(v
> >  {
> >  	int err;
> >  
> > +	if (!MACH_IS_MAC)
> > +		return -ENODEV;
> > +
> >  	if ((err = platform_driver_register(&mac_mace_driver))) {
> >  		printk(KERN_ERR "Driver registration failed\n");
> >  		return err;
> 
> This is rather odd.  The whole point of the platform_driver
> infrastructure is that the arch code only registers it if it's present
> and then the driver can match for it.  So the right fix here is to move
> the platform_driver_register(&mac_mace_driver) into the mac arch setup

I suppose you mean platform_*device*_register(), not
platform_*driver*_register()?

> code.  A quick grep shows this is a quite common pattern for m68k
> drivers.

I know. That's work in progress.

All these drivers predate the platform device framework and have been converted
to this framework a while ago, without separating the actual platform drivers
and platform devices.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [patch 10/15] m68k: Some network drivers do not check the platform

[Christoph Hellwig]

On Thu, Apr 24, 2008 at 08:51:17PM +0200, Geert Uytterhoeven wrote:
> I suppose you mean platform_*device*_register(), not
> platform_*driver*_register()?

Yes, sorry.

---end quoted text---