netdev - Re: [PATCH] cs89x0: add support for i.MX31ADS ARM board

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080914120358.GA26748@xi.wantstofly.org>
Date:	Sun, 14 Sep 2008 14:03:58 +0200
From:	Lennert Buytenhek <buytenh@...tstofly.org>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	Gilles Chanteperdrix <gilles.chanteperdrix@...omai.org>,
	netdev@...r.kernel.org, Russell King <rmk@....linux.org.uk>
Subject: Re: [PATCH] cs89x0: add support for i.MX31ADS ARM board

On Sat, Sep 13, 2008 at 08:29:55PM -0400, Jeff Garzik wrote:

> >Signed-off-by: Gilles Chanteperdrix <gilles.chanteperdrix@...omai.org>
> >---
> >diff --git a/drivers/net/cs89x0.c b/drivers/net/cs89x0.c
> >index ea6144a..875a43d 100644
> >--- a/drivers/net/cs89x0.c
> >+++ b/drivers/net/cs89x0.c
> >@@ -194,6 +194,12 @@ static unsigned int cs8900_irq_map[] = 
> >{IRQ_IXDP2X01_CS8900, 0, 0, 0};
> > #define CIRRUS_DEFAULT_IRQ	VH_INTC_INT_NUM_CASCADED_INTERRUPT_1 /* 
> > Event inputs bank 1 - ID 35/bit 3 */
> > static unsigned int netcard_portlist[] __used __initdata = 
> > {CIRRUS_DEFAULT_BASE, 0};
> > static unsigned int cs8900_irq_map[] = {CIRRUS_DEFAULT_IRQ, 0, 0, 0};
> >+#elif defined(CONFIG_MACH_MX31ADS)
> >+#include <mach/board-mx31ads.h>
> >+static unsigned int netcard_portlist[] __used __initdata = {
> >+	PBC_BASE_ADDRESS + PBC_CS8900A_IOBASE + 0x300, 0
> >+};
> >+static unsigned cs8900_irq_map[] = {EXPIO_INT_ENET_INT, 0, 0, 0};
> > #else
> > static unsigned int netcard_portlist[] __used __initdata =
> >    { 0x300, 0x320, 0x340, 0x360, 0x200, 0x220, 0x240, 0x260, 0x280, 
> >    0x2a0, 0x2c0, 0x2e0, 0};
> >@@ -802,7 +808,7 @@ cs89x0_probe1(struct net_device *dev, int ioaddr, int 
> >modular)
> > 	} else {
> > 		i = lp->isa_config & INT_NO_MASK;
> > 		if (lp->chip_type == CS8900) {
> >-#if defined(CONFIG_MACH_IXDP2351) || defined(CONFIG_ARCH_IXDP2X01) || 
> >defined(CONFIG_ARCH_PNX010X)
> >+#if defined(CONFIG_MACH_IXDP2351) || defined(CONFIG_ARCH_IXDP2X01) || 
> >defined(CONFIG_ARCH_PNX010X) || defined(CONFIG_MACH_MX31ADS)
> > 		        i = cs8900_irq_map[0];
> > #else
> > 			/* Translate the IRQ using the IRQ mapping table. */
> >@@ -1029,6 +1035,7 @@ skip_this_frame:
> > 
> > void  __init reset_chip(struct net_device *dev)
> > {
> >+#if !defined(CONFIG_MACH_MX31ADS)
> > #if !defined(CONFIG_MACH_IXDP2351) && !defined(CONFIG_ARCH_IXDP2X01)
> > 	struct net_local *lp = netdev_priv(dev);
> > 	int ioaddr = dev->base_addr;
> >@@ -1057,6 +1064,7 @@ void  __init reset_chip(struct net_device *dev)
> > 	reset_start_time = jiffies;
> > 	while( (readreg(dev, PP_SelfST) & INIT_DONE) == 0 && jiffies - 
> > 	reset_start_time < 2)
> > 		;
> >+#endif /* !CONFIG_MACH_MX31ADS */
> > }
> > 
> > 
> >@@ -1304,7 +1312,7 @@ net_open(struct net_device *dev)
> > 	else
> > #endif
> > 	{
> >-#if !defined(CONFIG_MACH_IXDP2351) && !defined(CONFIG_ARCH_IXDP2X01) && 
> >!defined(CONFIG_ARCH_PNX010X)
> >+#if !defined(CONFIG_MACH_IXDP2351) && !defined(CONFIG_ARCH_IXDP2X01) && 
> >!defined(CONFIG_ARCH_PNX010X) && !defined(CONFIG_MACH_MX31ADS)
> > 		if (((1 << dev->irq) & lp->irq_map) == 0) {
> > 			printk(KERN_ERR "%s: IRQ %d is not in our map of 
> > 			allowable IRQs, which is %x\n",
> 
> I don't know enough about the arch to be able to ACK or NAK...  pass 
> this through the appropriate arch maintainer tree please?

I don't have the original patch in my netdev box anymore, but what
seems to me should be done is to add a platform driver interface to
the cs89x0 driver, like how most other non-PCI non-other-autoconf-
supporting-bus net drivers do it.  (There aren't too many -- grep
for platform_driver_register in drivers/net/*)

Then the knowledge about I/O addresses and IRQ lines can live in the
platform code (where it belongs) instead of having it all in the driver
(where it doesn't belong).  And all the IXDP2351 and IXDP2X01 ifdefs
can disappear as well.  And the ->readword()/->writeword() hacks that
need to be done for IXDP2351 and IXDP2X01 and PNX can then just be an
(optional) set of platform data methods, with a fallback to the default
one if they aren't passed in.

The question is whether someone wants to step up and do that work.
cs89x0 has seen a lot of drive-by patching by people looking to satisfy
their needs, but noone has stepped up to give it the proper love and
care that it deserves.

Gilles, are you willing to do the platform driver conversion?  If not,
then I guess we should just merge this patch as-is (after you explain
why you need to stub out reset_chip() on your board, as no other board
seems to need this?).
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html