[PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Atsushi Nemoto]

This patch lets RBTX49XX boards use generic platform_driver interface
for the ne driver.

* Use platform_device to pass ioaddr and irq to the ne driver.
* Remove unnecessary ifdefs for RBTX49XX from the ne driver.
* Make the ne driver selectable on these boards regardless of CONFIG_ISA
* Add an ifdef for netcard_portlist[] to avoid unnecessary auto-probe.
  (I'm not sure M32R needs auto-probe but it is current behavior)

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
 .../toshiba_rbtx4927/toshiba_rbtx4927_setup.c      |   19 +++++++++++++++++++
 arch/mips/tx4938/toshiba_rbtx4938/setup.c          |   20 ++++++++++++++++++++
 drivers/net/Kconfig                                |    2 +-
 drivers/net/ne.c                                   |   13 ++++---------
 4 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c b/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
index 0f7576d..7d2c9d0 100644
--- a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
+++ b/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
@@ -1049,3 +1049,22 @@ static int __init toshiba_rbtx4927_rtc_init(void)
 	return IS_ERR(dev) ? PTR_ERR(dev) : 0;
 }
 device_initcall(toshiba_rbtx4927_rtc_init);
+
+static int __init rbtx4927_ne_init(void)
+{
+	struct resource res[] = {
+		{
+			.start	= RBTX4927_RTL_8019_BASE,
+			.end	= RBTX4927_RTL_8019_BASE + 0x20 - 1,
+			.flags	= IORESOURCE_IO,
+		}, {
+			.start	= RBTX4927_RTL_8019_IRQ,
+			.flags	= IORESOURCE_IRQ,
+		}
+	};
+	struct platform_device *dev =
+		platform_device_register_simple("ne", -1,
+						res, ARRAY_SIZE(res));
+	return IS_ERR(dev) ? PTR_ERR(dev) : 0;
+}
+device_initcall(rbtx4927_ne_init);
diff --git a/arch/mips/tx4938/toshiba_rbtx4938/setup.c b/arch/mips/tx4938/toshiba_rbtx4938/setup.c
index 66163ba..f5d1ce7 100644
--- a/arch/mips/tx4938/toshiba_rbtx4938/setup.c
+++ b/arch/mips/tx4938/toshiba_rbtx4938/setup.c
@@ -20,6 +20,7 @@
 #include <linux/console.h>
 #include <linux/pci.h>
 #include <linux/pm.h>
+#include <linux/platform_device.h>
 
 #include <asm/wbflush.h>
 #include <asm/reboot.h>
@@ -1037,3 +1038,22 @@ static int __init tx4938_spi_proc_setup(void)
 
 __initcall(tx4938_spi_proc_setup);
 #endif
+
+static int __init rbtx4938_ne_init(void)
+{
+	struct resource res[] = {
+		{
+			.start	= RBTX4938_RTL_8019_BASE,
+			.end	= RBTX4938_RTL_8019_BASE + 0x20 - 1,
+			.flags	= IORESOURCE_IO,
+		}, {
+			.start	= RBTX4938_RTL_8019_IRQ,
+			.flags	= IORESOURCE_IRQ,
+		}
+	};
+	struct platform_device *dev =
+		platform_device_register_simple("ne", -1,
+						res, ARRAY_SIZE(res));
+	return IS_ERR(dev) ? PTR_ERR(dev) : 0;
+}
+device_initcall(rbtx4938_ne_init);
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index aba0d39..a80e8ce 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -1088,7 +1088,7 @@ config ETH16I
 
 config NE2000
 	tristate "NE2000/NE1000 support"
-	depends on NET_ISA || (Q40 && m) || M32R
+	depends on NET_ISA || (Q40 && m) || M32R || TOSHIBA_RBTX4927 || TOSHIBA_RBTX4938
 	select CRC32
 	---help---
 	  If you have a network (Ethernet) card of this type, say Y and read
diff --git a/drivers/net/ne.c b/drivers/net/ne.c
index b8a181f..4e99e7a 100644
--- a/drivers/net/ne.c
+++ b/drivers/net/ne.c
@@ -56,10 +56,6 @@ static const char version2[] =
 #include <asm/system.h>
 #include <asm/io.h>
 
-#if defined(CONFIG_TOSHIBA_RBTX4927) || defined(CONFIG_TOSHIBA_RBTX4938)
-#include <asm/tx4938/rbtx4938.h>
-#endif
-
 #include "8390.h"
 
 #define DRV_NAME "ne"
@@ -81,7 +77,10 @@ static const char version2[] =
 /* A zero-terminated list of I/O addresses to be probed at boot. */
 #ifndef MODULE
 static unsigned int netcard_portlist[] __initdata = {
-	0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0
+#if defined(CONFIG_ISA) || defined(CONFIG_M32R)
+	0x300, 0x280, 0x320, 0x340, 0x360, 0x380,
+#endif
+	0
 };
 #endif
 
@@ -227,10 +226,6 @@ struct net_device * __init ne_probe(int unit)
 	sprintf(dev->name, "eth%d", unit);
 	netdev_boot_setup_check(dev);
 
-#ifdef CONFIG_TOSHIBA_RBTX4938
-	dev->base_addr = RBTX4938_RTL_8019_BASE;
-	dev->irq = RBTX4938_RTL_8019_IRQ;
-#endif
 	err = do_ne_probe(dev);
 	if (err)
 		goto out;

Re: [PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Atsushi Nemoto]

On Wed, 25 Apr 2007 01:55:49 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> This patch lets RBTX49XX boards use generic platform_driver interface
> for the ne driver.

This patch obsolates a patch I send on 1 Mar.

> Subject: [PATCH] Fix broken RBTX4927 support in ne.c
> Message-Id: <20070301.012223.129448787.anemo@mba.ocn.ne.jp>
> Date: 	Thu, 01 Mar 2007 01:22:23 +0900 (JST)

I revoke this old patch if new patch was acceptable.

---
Atsushi Nemoto

Re: [PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Andrew Morton]

On Wed, 25 Apr 2007 01:55:49 +0900 (JST) Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:

> +static int __init rbtx4938_ne_init(void)
> +{
> +	struct resource res[] = {
> +		{
> +			.start	= RBTX4938_RTL_8019_BASE,
> +			.end	= RBTX4938_RTL_8019_BASE + 0x20 - 1,
> +			.flags	= IORESOURCE_IO,
> +		}, {
> +			.start	= RBTX4938_RTL_8019_IRQ,
> +			.flags	= IORESOURCE_IRQ,
> +		}
> +	};
> +	struct platform_device *dev =
> +		platform_device_register_simple("ne", -1,
> +						res, ARRAY_SIZE(res));
> +	return IS_ERR(dev) ? PTR_ERR(dev) : 0;
> +}

platform_device_register_simple() copies *res by value, so I believe we can
make res[] static __initdata.  This way we don't need to evaluate the array
on the stack at runtime, and the data gets discarded after initcalls have
run.

Can you please review and test the below?  I had a go but wasn't able to
fumble my way to a suitable config (I hope):

<stdin>:1176:2: warning: #warning syscall fadvise64_64 not implemented
arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c: In function `toshiba_rbtx4927_time_init':
arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c:1026: error: `tx4927_cpu_clock' undeclared (first use in this function)
arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c:1026: error: (Each undeclared identifier is reported only once
arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c:1026: error: for each function it appears in.)
make[1]: *** [arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.o] Error 1
make: *** [arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.o] Error 2
make: *** Waiting for unfinished jobs....
arch/mips/tx4938/toshiba_rbtx4938/setup.c:41: warning: 'tx4938_report_pcic_status1' declared `static' but never defined
arch/mips/tx4938/toshiba_rbtx4938/setup.c:56: warning: 'tx4938_pcic_trdyto' defined but not used
arch/mips/tx4938/toshiba_rbtx4938/setup.c:57: warning: 'tx4938_pcic_retryto' defined but not used



From: Andrew Morton <akpm@linux-foundation.org>

Cc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c~ne-mips-use-platform_driver-for-ne-on-rbtx49xx-fix arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
--- a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c~ne-mips-use-platform_driver-for-ne-on-rbtx49xx-fix
+++ a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
@@ -1039,7 +1039,7 @@ void __init toshiba_rbtx4927_timer_setup
 
 static int __init toshiba_rbtx4927_rtc_init(void)
 {
-	struct resource res = {
+	static struct resource __initdata res = {
 		.start	= 0x1c010000,
 		.end	= 0x1c010000 + 0x800 - 1,
 		.flags	= IORESOURCE_MEM,
@@ -1052,7 +1052,7 @@ device_initcall(toshiba_rbtx4927_rtc_ini
 
 static int __init rbtx4927_ne_init(void)
 {
-	struct resource res[] = {
+	static struct resource __initdata res[] = {
 		{
 			.start	= RBTX4927_RTL_8019_BASE,
 			.end	= RBTX4927_RTL_8019_BASE + 0x20 - 1,
_

Re: [PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Jeff Garzik]

Atsushi Nemoto wrote:
> On Wed, 25 Apr 2007 01:55:49 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
>> This patch lets RBTX49XX boards use generic platform_driver interface
>> for the ne driver.
> 
> This patch obsolates a patch I send on 1 Mar.
> 
>> Subject: [PATCH] Fix broken RBTX4927 support in ne.c
>> Message-Id: <20070301.012223.129448787.anemo@mba.ocn.ne.jp>
>> Date: 	Thu, 01 Mar 2007 01:22:23 +0900 (JST)
> 
> I revoke this old patch if new patch was acceptable.

I do not see the old patch in my queue (my apologies!), so all is well.

	Jeff

Re: [PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Jeff Garzik]

Atsushi Nemoto wrote:
> This patch lets RBTX49XX boards use generic platform_driver interface
> for the ne driver.
> 
> * Use platform_device to pass ioaddr and irq to the ne driver.
> * Remove unnecessary ifdefs for RBTX49XX from the ne driver.
> * Make the ne driver selectable on these boards regardless of CONFIG_ISA
> * Add an ifdef for netcard_portlist[] to avoid unnecessary auto-probe.
>   (I'm not sure M32R needs auto-probe but it is current behavior)
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> ---
>  .../toshiba_rbtx4927/toshiba_rbtx4927_setup.c      |   19 +++++++++++++++++++
>  arch/mips/tx4938/toshiba_rbtx4938/setup.c          |   20 ++++++++++++++++++++
>  drivers/net/Kconfig                                |    2 +-
>  drivers/net/ne.c                                   |   13 ++++---------
>  4 files changed, 44 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c b/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
> index 0f7576d..7d2c9d0 100644
> --- a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
> +++ b/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
> @@ -1049,3 +1049,22 @@ static int __init toshiba_rbtx4927_rtc_init(void)
>  	return IS_ERR(dev) ? PTR_ERR(dev) : 0;
>  }
>  device_initcall(toshiba_rbtx4927_rtc_init);
> +
> +static int __init rbtx4927_ne_init(void)
> +{
> +	struct resource res[] = {
> +		{
> +			.start	= RBTX4927_RTL_8019_BASE,
> +			.end	= RBTX4927_RTL_8019_BASE + 0x20 - 1,
> +			.flags	= IORESOURCE_IO,
> +		}, {
> +			.start	= RBTX4927_RTL_8019_IRQ,
> +			.flags	= IORESOURCE_IRQ,
> +		}
> +	};
> +	struct platform_device *dev =
> +		platform_device_register_simple("ne", -1,
> +						res, ARRAY_SIZE(res));
> +	return IS_ERR(dev) ? PTR_ERR(dev) : 0;
> +}
> +device_initcall(rbtx4927_ne_init);
> diff --git a/arch/mips/tx4938/toshiba_rbtx4938/setup.c b/arch/mips/tx4938/toshiba_rbtx4938/setup.c
> index 66163ba..f5d1ce7 100644
> --- a/arch/mips/tx4938/toshiba_rbtx4938/setup.c
> +++ b/arch/mips/tx4938/toshiba_rbtx4938/setup.c
> @@ -20,6 +20,7 @@
>  #include <linux/console.h>
>  #include <linux/pci.h>
>  #include <linux/pm.h>
> +#include <linux/platform_device.h>
>  
>  #include <asm/wbflush.h>
>  #include <asm/reboot.h>
> @@ -1037,3 +1038,22 @@ static int __init tx4938_spi_proc_setup(void)
>  
>  __initcall(tx4938_spi_proc_setup);
>  #endif
> +
> +static int __init rbtx4938_ne_init(void)
> +{
> +	struct resource res[] = {
> +		{
> +			.start	= RBTX4938_RTL_8019_BASE,
> +			.end	= RBTX4938_RTL_8019_BASE + 0x20 - 1,
> +			.flags	= IORESOURCE_IO,
> +		}, {
> +			.start	= RBTX4938_RTL_8019_IRQ,
> +			.flags	= IORESOURCE_IRQ,
> +		}
> +	};
> +	struct platform_device *dev =
> +		platform_device_register_simple("ne", -1,
> +						res, ARRAY_SIZE(res));
> +	return IS_ERR(dev) ? PTR_ERR(dev) : 0;
> +}
> +device_initcall(rbtx4938_ne_init);
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index aba0d39..a80e8ce 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -1088,7 +1088,7 @@ config ETH16I
>  
>  config NE2000
>  	tristate "NE2000/NE1000 support"
> -	depends on NET_ISA || (Q40 && m) || M32R
> +	depends on NET_ISA || (Q40 && m) || M32R || TOSHIBA_RBTX4927 || TOSHIBA_RBTX4938
>  	select CRC32
>  	---help---
>  	  If you have a network (Ethernet) card of this type, say Y and read
> diff --git a/drivers/net/ne.c b/drivers/net/ne.c
> index b8a181f..4e99e7a 100644
> --- a/drivers/net/ne.c
> +++ b/drivers/net/ne.c
> @@ -56,10 +56,6 @@ static const char version2[] =
>  #include <asm/system.h>
>  #include <asm/io.h>
>  
> -#if defined(CONFIG_TOSHIBA_RBTX4927) || defined(CONFIG_TOSHIBA_RBTX4938)
> -#include <asm/tx4938/rbtx4938.h>
> -#endif
> -
>  #include "8390.h"
>  
>  #define DRV_NAME "ne"
> @@ -81,7 +77,10 @@ static const char version2[] =
>  /* A zero-terminated list of I/O addresses to be probed at boot. */
>  #ifndef MODULE
>  static unsigned int netcard_portlist[] __initdata = {
> -	0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0
> +#if defined(CONFIG_ISA) || defined(CONFIG_M32R)
> +	0x300, 0x280, 0x320, 0x340, 0x360, 0x380,
> +#endif
> +	0

This looks a bit strange, and perhaps more difficult to maintain long term.

I would suggest creating a NEEDS_PORTLIST cpp macro at the top of ne.c, 
to be defined or undefined based on CONFIG_xxx symbols.  Then, down in 
the code itself, conditionally include or exclude all portlist related 
data tables and code.

Sound sane?

	Jeff

Re: [PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Atsushi Nemoto]

On Sat, 28 Apr 2007 01:04:14 -0700, Andrew Morton <akpm@linux-foundation.org> wrote:
> platform_device_register_simple() copies *res by value, so I believe we can
> make res[] static __initdata.  This way we don't need to evaluate the array
> on the stack at runtime, and the data gets discarded after initcalls have
> run.
> 
> Can you please review and test the below?  I had a go but wasn't able to
> fumble my way to a suitable config (I hope):

It looks OK.  I will merge your fix to updated patch.  Thank you.

And the lack of its defconfig should be fixed anyway, but it should be
another story ...

---
Atsushi Nemoto

Re: [PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Atsushi Nemoto]

On Sat, 28 Apr 2007 11:10:37 -0400, Jeff Garzik <jeff@garzik.org> wrote:
> >  static unsigned int netcard_portlist[] __initdata = {
> > -	0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0
> > +#if defined(CONFIG_ISA) || defined(CONFIG_M32R)
> > +	0x300, 0x280, 0x320, 0x340, 0x360, 0x380,
> > +#endif
> > +	0
> 
> This looks a bit strange, and perhaps more difficult to maintain long term.
> 
> I would suggest creating a NEEDS_PORTLIST cpp macro at the top of ne.c, 
> to be defined or undefined based on CONFIG_xxx symbols.  Then, down in 
> the code itself, conditionally include or exclude all portlist related 
> data tables and code.
> 
> Sound sane?

Sure.  Do you mean something like this?


#if !defined(MODULE) && (defined(CONFIG_ISA) || defined(CONFIG_M32R))
#define NEEDS_PORTLIST
#endif
...
#ifdef NEEDS_PORTLIST
static unsigned int netcard_portlist[] __initdata = {
	0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0
};
#endif
...
#ifdef NEEDS_PORTLIST
	int orig_irq = dev->irq;
#endif
...
#ifdef NEEDS_PORTLIST
	/* Last resort. The semi-risky ISA auto-probe. */
	for (base_addr = 0; netcard_portlist[base_addr] != 0; base_addr++) {
		int ioaddr = netcard_portlist[base_addr];
		dev->irq = orig_irq;
		if (ne_probe1(dev, ioaddr) == 0)
			return 0;
	}
#endif

---
Atsushi Nemoto

Re: [PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Jeff Garzik]

Atsushi Nemoto wrote:
> On Sat, 28 Apr 2007 11:10:37 -0400, Jeff Garzik <jeff@garzik.org> wrote:
>>>  static unsigned int netcard_portlist[] __initdata = {
>>> -	0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0
>>> +#if defined(CONFIG_ISA) || defined(CONFIG_M32R)
>>> +	0x300, 0x280, 0x320, 0x340, 0x360, 0x380,
>>> +#endif
>>> +	0
>> This looks a bit strange, and perhaps more difficult to maintain long term.
>>
>> I would suggest creating a NEEDS_PORTLIST cpp macro at the top of ne.c, 
>> to be defined or undefined based on CONFIG_xxx symbols.  Then, down in 
>> the code itself, conditionally include or exclude all portlist related 
>> data tables and code.
>>
>> Sound sane?
> 
> Sure.  Do you mean something like this?

Correct.

	Jeff

Re: [PATCH 2/3] ne: MIPS: Use platform_driver for ne on RBTX49XX

[Atsushi Nemoto]

On Sun, 29 Apr 2007 02:10:37 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> > platform_device_register_simple() copies *res by value, so I believe we can
> > make res[] static __initdata.  This way we don't need to evaluate the array
> > on the stack at runtime, and the data gets discarded after initcalls have
> > run.
> > 
> > Can you please review and test the below?  I had a go but wasn't able to
> > fumble my way to a suitable config (I hope):
> 
> It looks OK.  I will merge your fix to updated patch.  Thank you.

Unfortunately, the RBTX4938 part can not be converted to static
__initdata since RBTX4938_RTL_8019_BASE is not a constant.

And I think the change for toshiba_rbtx4927_rtc_init() should be an
another patch.  So I will add the "static __initdata" only to
rbtx4927_ne_init().

---
Atsushi Nemoto