Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two

2012-04-06 Thread Geert Uytterhoeven
On Fri, Apr 6, 2012 at 00:10, Michael Schmitz schmitz...@googlemail.com wrote:
 Indeed, with a small modification (keep multi-platform kernels in mind):

 #ifdef CONFIG_ATARI
 #define EI_IRQ_FLAGS    (MACH_IS_ATARI ? IRQF_SHARED : 0)
 #else
 #define EI_IRQ_FLAGS    0
 #endif


 Right you are. Is any other m68k platform using ne.c directly, or do you
 plan to convert all other NE2000 based drivers to ne.c now?

Plain ne is also used on Q40.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two

2012-04-05 Thread Geert Uytterhoeven
On Wed, Apr 4, 2012 at 22:46, Paul Gortmaker
paul.gortma...@windriver.com wrote:
 On 12-04-01 04:49 AM, Michael Schmitz wrote:
 And on re-reading the comments in the other part of the patch, i.e.
 ...emulates the card interrupt via a timer  --perhaps the driver
 should be just fixed to support generic netpoll, instead of adding
 an arch specific thing that amounts to netpoll.  Then anyone can
 attempt to limp along and use one of these ancient relics w/o IRQ.

 Here's take two of my patch to convert the m68k Atari ROM port Ethernet
 driver to use mainstream ne.c, with minimal changes to the core NE2000
 code.

 In particular:

 Changes to core net code:
 * add a platform specific IRQ flag, so ne.c can share a hardware or
 timer interrupt with some other interrupt source.

 Changes to arch/m68k code:
 * register the 8390 platform device on Atari only if the hardware is present
 * retain the old driver (atari_ethernec.c in Geert's tree) under a
 different config option, to be removed soon.

 Regarding your suggestion that netpoll be used instead of a dedicated
 timer interrupt: no changes to ne.c or 8390p.c are required to use
 netpoll, it all works out of the box. All that is needed to use the
 driver with netpoll is setting the device interrupt to some source that
 can be registered, and enabling CONFIG_NETPOLL. Interrupt rate and hence
 throughput is lower with netpoll though, which is why I still prefer the
 dedicated timer option.

 How much lower?  Enough to matter?  Implicit in that question is
 the assumption that this is largely a hobbyist platform and nobody
 is using it in a closet to route gigabytes of traffic.

One other thing we could do is increase CONFIG_HZ to 250.

 Also, the only advantage to modifying ne.c is to allow dumping
 the old driver.  What is the remove soon plan?  Any reason
 for it to not be synchronous?  That would eliminate the Kconfig
 churn and the introduction of the _OLD option.  Modifying ne.c
 and then deciding to keep the old driver because it is faster
 would make this change pointless.

From my point of view, remove soon means it will never hit mainline.

 diff --git a/drivers/net/ethernet/8390/8390.h
 b/drivers/net/ethernet/8390/8390.h
 index ef325ff..9416245 100644
 --- a/drivers/net/ethernet/8390/8390.h
 +++ b/drivers/net/ethernet/8390/8390.h
 @@ -32,6 +32,14 @@ extern void ei_poll(struct net_device *dev);
  extern void eip_poll(struct net_device *dev);
  #endif

 +/* Some platforms may need special IRQ flags */
 +#if IS_ENABLED(CONFIG_ATARI_ETHERNEC)
 +#  define EI_IRQ_FLAGS    IRQF_SHARED
 +#endif
 +
 +#ifndef EI_IRQ_FLAGS
 +#  define EI_IRQ_FLAGS    0
 +#endif

 This seems more klunky than it needs to be.  If we assume that anyone
 building ne.c on atari is hence trying to drive an ethernec device
 than it can just be

 #ifdef CONFIG_ATARI
 #define EI_IRQ_FLAGS    IRQF_SHARED
 #else
 #define EI_IRQ_FLAGS    0
 #endif

Indeed, with a small modification (keep multi-platform kernels in mind):

#ifdef CONFIG_ATARI
#define EI_IRQ_FLAGS    (MACH_IS_ATARI ? IRQF_SHARED : 0)
#else
#define EI_IRQ_FLAGS    0
#endif

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two

2012-04-05 Thread Michael Schmitz

Hi Paul,

(Apologies to all for botching the patch format ...)

Regarding your suggestion that netpoll be used instead of a dedicated 
timer interrupt: no changes to ne.c or 8390p.c are required to use 
netpoll, it all works out of the box. All that is needed to use the 
driver with netpoll is setting the device interrupt to some source that  
can be registered, and enabling CONFIG_NETPOLL. Interrupt rate and hence 
throughput is lower with netpoll though, which is why I still prefer the 
dedicated timer option.



How much lower?  Enough to matter?  Implicit in that question is
the assumption that this is largely a hobbyist platform and nobody
is using it in a closet to route gigabytes of traffic.
  
I'd say about at least double latency. I can try and measure bulk data 
rates if it matters. My gut feeling is latency limits data rates even 
when say behind a DSL modem for downloads. It sure did when my Falcon 
was still hooked up to a university network, uploading and downloading 
source and binary packages for Debian/68k.


Of course you're not routing gigabytes of traffic with this (where to - 
a PPP connection? :). Whoever wants minimum latency better reach for the 
soldering iron and wire up the interrupt line to some suitable input.

Also, the only advantage to modifying ne.c is to allow dumping
the old driver.  What is the remove soon plan?  Any reason
for it to not be synchronous?  That would eliminate the Kconfig
churn and the introduction of the _OLD option.  Modifying ne.c
and then deciding to keep the old driver because it is faster
would make this change pointless.
  
As soon as eventual changes to ne.c get accepted. If you want us to drop 
the old driver in the same patch, fine by me.
diff --git a/drivers/net/ethernet/8390/8390.h 
b/drivers/net/ethernet/8390/8390.h

index ef325ff..9416245 100644
--- a/drivers/net/ethernet/8390/8390.h
+++ b/drivers/net/ethernet/8390/8390.h
@@ -32,6 +32,14 @@ extern void ei_poll(struct net_device *dev);
 extern void eip_poll(struct net_device *dev);
 #endif
 
+/* Some platforms may need special IRQ flags */

+#if IS_ENABLED(CONFIG_ATARI_ETHERNEC)
+#  define EI_IRQ_FLAGSIRQF_SHARED
+#endif
+
+#ifndef EI_IRQ_FLAGS
+#  define EI_IRQ_FLAGS0
+#endif



This seems more klunky than it needs to be.  If we assume that anyone
building ne.c on atari is hence trying to drive an ethernec device
than it can just be

#ifdef CONFIG_ATARI
#define EI_IRQ_FLAGSIRQF_SHARED
#else
#define EI_IRQ_FLAGS0
#endif

  
Pretty safe assumption - if we further assume no other arch has reason 
to resort to such a kludge, we can simplify it this way.

--- a/drivers/net/ethernet/8390/ne.c
+++ b/drivers/net/ethernet/8390/ne.c
@@ -165,7 +165,8 @@ bad_clone_list[] __initdata = {
 #if defined(CONFIG_PLAT_MAPPI)
 #  define DCR_VAL 0x4b
 #elif defined(CONFIG_PLAT_OAKS32R)  || \
-   defined(CONFIG_MACH_TX49XX)
+   defined(CONFIG_MACH_TX49XX) || \
+   IS_ENABLED(CONFIG_ATARI_ETHERNEC)



Rather than use IS_ENABLED on a driver setting, you can follow
the surrounding context and use defined(CONFIG_ATARI) -- i.e.
work off a platform setting.
  
True as well, point taken. Is the patch acceptable with these changes? 
If so, would you be OK with this going through Geert's tree?


Cheers,

 Michael

--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two

2012-04-05 Thread Paul Gortmaker
On 12-04-05 05:28 AM, Geert Uytterhoeven wrote:
 On Wed, Apr 4, 2012 at 22:46, Paul Gortmaker
 paul.gortma...@windriver.com wrote:
 On 12-04-01 04:49 AM, Michael Schmitz wrote:
 And on re-reading the comments in the other part of the patch, i.e.
 ...emulates the card interrupt via a timer  --perhaps the driver
 should be just fixed to support generic netpoll, instead of adding
 an arch specific thing that amounts to netpoll.  Then anyone can
 attempt to limp along and use one of these ancient relics w/o IRQ.

 Here's take two of my patch to convert the m68k Atari ROM port Ethernet
 driver to use mainstream ne.c, with minimal changes to the core NE2000
 code.

 In particular:

 Changes to core net code:
 * add a platform specific IRQ flag, so ne.c can share a hardware or
 timer interrupt with some other interrupt source.

 Changes to arch/m68k code:
 * register the 8390 platform device on Atari only if the hardware is present
 * retain the old driver (atari_ethernec.c in Geert's tree) under a
 different config option, to be removed soon.

 Regarding your suggestion that netpoll be used instead of a dedicated
 timer interrupt: no changes to ne.c or 8390p.c are required to use
 netpoll, it all works out of the box. All that is needed to use the
 driver with netpoll is setting the device interrupt to some source that
 can be registered, and enabling CONFIG_NETPOLL. Interrupt rate and hence
 throughput is lower with netpoll though, which is why I still prefer the
 dedicated timer option.

 How much lower?  Enough to matter?  Implicit in that question is
 the assumption that this is largely a hobbyist platform and nobody
 is using it in a closet to route gigabytes of traffic.
 
 One other thing we could do is increase CONFIG_HZ to 250.
 
 Also, the only advantage to modifying ne.c is to allow dumping
 the old driver.  What is the remove soon plan?  Any reason
 for it to not be synchronous?  That would eliminate the Kconfig
 churn and the introduction of the _OLD option.  Modifying ne.c
 and then deciding to keep the old driver because it is faster
 would make this change pointless.
 
 From my point of view, remove soon means it will never hit mainline.

Can you clarify what it is?   It isn't clear to me if you
mean the _removal_ will never hit mainline, or the transient
renamed old driver will never hit mainline.

If the former, then there is no point pursuing this any further
as I said above.

If the latter, then the commit sent out for review should have
no instances of this renaming to old related changes.

Thanks,
Paul.

 
 diff --git a/drivers/net/ethernet/8390/8390.h
 b/drivers/net/ethernet/8390/8390.h
 index ef325ff..9416245 100644
 --- a/drivers/net/ethernet/8390/8390.h
 +++ b/drivers/net/ethernet/8390/8390.h
 @@ -32,6 +32,14 @@ extern void ei_poll(struct net_device *dev);
  extern void eip_poll(struct net_device *dev);
  #endif

 +/* Some platforms may need special IRQ flags */
 +#if IS_ENABLED(CONFIG_ATARI_ETHERNEC)
 +#  define EI_IRQ_FLAGSIRQF_SHARED
 +#endif
 +
 +#ifndef EI_IRQ_FLAGS
 +#  define EI_IRQ_FLAGS0
 +#endif

 This seems more klunky than it needs to be.  If we assume that anyone
 building ne.c on atari is hence trying to drive an ethernec device
 than it can just be

 #ifdef CONFIG_ATARI
 #define EI_IRQ_FLAGSIRQF_SHARED
 #else
 #define EI_IRQ_FLAGS0
 #endif
 
 Indeed, with a small modification (keep multi-platform kernels in mind):
 
 #ifdef CONFIG_ATARI
 #define EI_IRQ_FLAGS(MACH_IS_ATARI ? IRQF_SHARED : 0)
 #else
 #define EI_IRQ_FLAGS0
 #endif
 
 Gr{oetje,eeting}s,
 
 Geert
 
 --
 Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
 ge...@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
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two

2012-04-03 Thread David Miller
From: Michael Schmitz schmitz...@googlemail.com
Date: Sun, 01 Apr 2012 20:49:52 +1200

 Hi Paul, Geert,
 And on re-reading the comments in the other part of the patch, i.e.
 ...emulates the card interrupt via a timer  --perhaps the driver
 should be just fixed to support generic netpoll, instead of adding
 an arch specific thing that amounts to netpoll.  Then anyone can
 attempt to limp along and use one of these ancient relics w/o IRQ.
   
 Here's take two of my patch to convert the m68k Atari ROM port
 Ethernet driver to use mainstream ne.c, with minimal changes to the
 core NE2000 code.

Please fix your email client, it corrupts your outgoing patches
by breaking up long lines with newlines amongst other things.
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two

2012-04-01 Thread Michael Schmitz

Hi Paul, Geert,

And on re-reading the comments in the other part of the patch, i.e.
...emulates the card interrupt via a timer  --perhaps the driver
should be just fixed to support generic netpoll, instead of adding
an arch specific thing that amounts to netpoll.  Then anyone can
attempt to limp along and use one of these ancient relics w/o IRQ.
  
Here's take two of my patch to convert the m68k Atari ROM port Ethernet 
driver to use mainstream ne.c, with minimal changes to the core NE2000 
code.


In particular:

Changes to core net code:
* add a platform specific IRQ flag, so ne.c can share a hardware or 
timer interrupt with some other interrupt source.


Changes to arch/m68k code:
* register the 8390 platform device on Atari only if the hardware is present
* retain the old driver (atari_ethernec.c in Geert's tree) under a 
different config option, to be removed soon.


Regarding your suggestion that netpoll be used instead of a dedicated 
timer interrupt: no changes to ne.c or 8390p.c are required to use 
netpoll, it all works out of the box. All that is needed to use the 
driver with netpoll is setting the device interrupt to some source that  
can be registered, and enabling CONFIG_NETPOLL. Interrupt rate and hence 
throughput is lower with netpoll though, which is why I still prefer the 
dedicated timer option.


Comments?

Cheers,

 Michael Schmitz

Signed-off-by: Michael Schmitz schm...@debian.org

--
arch/m68k/atari/config.c   |   41 
+---

drivers/net/Space.c|2 +-
drivers/net/ethernet/8390/8390.h   |8 +
drivers/net/ethernet/8390/Kconfig  |   18 +++-
drivers/net/ethernet/8390/Makefile |3 +-
drivers/net/ethernet/8390/atari_ethernec.c |6 ++--
drivers/net/ethernet/8390/ne.c |5 ++-
7 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/arch/m68k/atari/config.c b/arch/m68k/atari/config.c
index 22375e0..6cff09f 100644
--- a/arch/m68k/atari/config.c
+++ b/arch/m68k/atari/config.c
@@ -664,6 +664,35 @@ static void atari_get_hardware_list(struct seq_file *m)
 */

#define ATARI_ETHERNEC_PHYS_ADDR0xfffa
+#define ATARI_ETHERNEC_BASE0x300
+#define ATARI_ETHERNEC_IRQIRQ_MFP_TIMD
+
+static struct resource rtl8019_resources[] = {
+[0] = {
+.name= rtl9019-regs,
+.start= ATARI_ETHERNEC_BASE,
+.end= ATARI_ETHERNEC_BASE + 0x20 - 1,
+.flags= IORESOURCE_IO,
+},
+[1] = {
+.name= rtl9019-irq,
+.start= ATARI_ETHERNEC_IRQ,
+.end= ATARI_ETHERNEC_IRQ,
+.flags= IORESOURCE_IRQ,
+},
+};
+
+static struct platform_device rtl8019_device = {
+.name= ne,
+.id= -1,
+.num_resources= ARRAY_SIZE(rtl8019_resources),
+.resource= rtl8019_resources,
+};
+
+static struct platform_device *atari_ethernec_devices[] __initdata = {
+rtl8019_device
+};
+

#define ATARI_ETHERNAT_PHYS_ADDR0x8000
#define ATARI_ETHERNAT_IRQ196
@@ -690,6 +719,7 @@ static struct platform_device smc91x_device = {
.resource= smc91x_resources,
};

+
#define ATARI_USB_PHYS_ADDR0x8010
#define ATARI_USB_IRQ195

@@ -753,7 +783,8 @@ static struct platform_device 
*atari_ethernat_devices[] __initdata = {

isp1160_device
};

-#if IS_ENABLED(CONFIG_ATARI_ETHERNEC) || IS_ENABLED(CONFIG_ATARI_ETHERNAT)
+#if IS_ENABLED(CONFIG_ATARI_ETHERNEC) || 
IS_ENABLED(CONFIG_ATARI_ETHERNAT) \

+ || IS_ENABLED(CONFIG_ATARI_ETHERNEC_OLD)
irqreturn_t atari_timerd_interrupt(int irq, void *dev_id)
{
return IRQ_HANDLED;
@@ -762,16 +793,17 @@ irqreturn_t atari_timerd_interrupt(int irq, void 
*dev_id)


int __init atari_platform_init(void)
{
-int rv = -ENODEV, ret, need_timer = 0;
+int rv = -ENODEV, rv1 = -ENODEV, need_timer = 0;
unsigned char *enatc_virt, *enec_virt;

if (!MACH_IS_ATARI)
return -ENODEV;

-#if IS_ENABLED(CONFIG_ATARI_ETHERNEC)
+#if IS_ENABLED(CONFIG_ATARI_ETHERNEC) || 
IS_ENABLED(CONFIG_ATARI_ETHERNEC_OLD)

enec_virt = (unsigned char *)ioremap((ATARI_ETHERNEC_PHYS_ADDR), 0xf);
if (hwreg_present(enec_virt)) {
need_timer = 1;
+rv1 = platform_add_devices(atari_ethernec_devices, 
ARRAY_SIZE(atari_ethernec_devices));

}
iounmap(enec_virt);
#endif
@@ -788,6 +820,7 @@ int __init atari_platform_init(void)
#endif

if (need_timer) {
+int ret;
const char *name = Timer D dummy interrupt;

/* timer routine set up in atari_ethernec_probe() */
@@ -802,7 +835,7 @@ int __init atari_platform_init(void)
}
}

-if (ret) return ret;
+if (rv1) return rv1;
return rv;   
}


diff --git a/drivers/net/Space.c b/drivers/net/Space.c
index d5e8882..548b73b 100644
--- a/drivers/net/Space.c
+++ b/drivers/net/Space.c
@@ -243,7 +243,7 @@ static struct devprobe2 m68k_probes[] __initdata = {
#ifdef CONFIG_ATARILANCE/* Lance-based Atari ethernet