Thread (5 messages) flat view 5 messages, 2 authors, 2012-01-04
DORMANTno replies

[PATCH 1/4] CS89x0 : add platform driver support

From: s.hauer@pengutronix.de (Sascha Hauer)
Date: 2012-01-04 09:20:00
Also in: netdev

Hi Jaccon,

Thanks for continuing this. Some comments inline.

On Tue, Jan 03, 2012 at 11:12:09PM +0100, Jaccon Bastiaansen wrote:
quoted hunk ↗ jump to hunk
@@ -151,6 +153,9 @@
 #include <asm/system.h>
 #include <asm/io.h>
 #include <asm/irq.h>
+#ifdef CONFIG_CS89x0_PLATFORM
+#include <linux/atomic.h>
+#endif
No need to ifdef includes.
quoted hunk ↗ jump to hunk
 #if ALLOW_DMA
 #include <asm/dma.h>
 #endif
@@ -173,6 +178,7 @@ static char version[] __initdata =
 /* The cs8900 has 4 IRQ pins, software selectable. cs8900_irq_map maps
    them to system IRQ numbers. This mapping is card specific and is set to
    the configuration of the Cirrus Eval board for this chip. */
+#if defined(CONFIG_CS89x0_PLATFORM)
 #if defined(CONFIG_MACH_IXDP2351)
 static unsigned int netcard_portlist[] __used __initdata = {IXDP2351_VIRT_CS8900_BASE, 0};
 static unsigned int cs8900_irq_map[] = {IRQ_IXDP2351_CS8900, 0, 0, 0};
@@ -188,7 +194,17 @@ static unsigned int cs8900_irq_map[] = { QQ2440_CS8900_IRQ, 0, 0, 0 };
 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};
+static unsigned int cs8900_irq_map[] = {EXPIO_INT_ENET_INT, 0, 0, 0};
+#else
+/*
+ * Counter for the number of CS89x0 platform device instances, which is needed
+ * because this driver currently supports only one CS89x0 platform device
+ * instance.
+ */
+static atomic_t platform_dev_cnt = ATOMIC_INIT(0);
+static unsigned int cs8900_irq_map[] = {0, 0, 0, 0};
+static unsigned int netcard_portlist[] __used __initdata = {0, 0};
+#endif
 #else
 static unsigned int netcard_portlist[] __used __initdata =
    { 0x300, 0x320, 0x340, 0x360, 0x200, 0x220, 0x240, 0x260, 0x280, 0x2a0, 0x2c0, 0x2e0, 0};
@@ -737,7 +753,7 @@ cs89x0_probe1(struct net_device *dev, int ioaddr, int modular)
 	} else {
 		i = lp->isa_config & INT_NO_MASK;
 		if (lp->chip_type == CS8900) {
[...]
quoted hunk ↗ jump to hunk
 
-#ifdef MODULE
+#if defined(MODULE) && !defined(CONFIG_CS89x0_PLATFORM)
 
 static struct net_device *dev_cs89x0;
 
@@ -1900,7 +1916,78 @@ cleanup_module(void)
 	release_region(dev_cs89x0->base_addr, NETCARD_IO_EXTENT);
 	free_netdev(dev_cs89x0);
 }
-#endif /* MODULE */
+#endif /* MODULE && !CONFIG_CS89x0_PLATFORM */
+
+#ifdef CONFIG_CS89x0_PLATFORM
+static int cs89x0_platform_probe(struct platform_device *pdev)
+{
+	struct net_device *dev = alloc_etherdev(sizeof(struct net_local));
+	struct resource *mem_res;
+	struct resource *irq_res;
+	int err;
+
+	if (!dev)
+		return -ENODEV;
+
+	if (atomic_inc_return(&platform_dev_cnt) != 1)
+		return -EBUSY;
This deserves a comment like this:

/*
 * This driver uses global variables. Until this is fixed we can
 * only support a single instance.
 */
+
+	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
Don't use virtual addresses in resources. It's wrong. Pass in physical
addresses here and use ioremap() in the driver.
+	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (mem_res == NULL || irq_res == NULL) {
+		pr_warning("memory and/or interrupt resource missing.\n");
dev_warn please
+		err = -ENOENT;
+		goto out;
+	}
+
+	cs8900_irq_map[0] = irq_res->start;
+	err = cs89x0_probe1(dev, mem_res->start, 0);
+	if (err) {
+		pr_warning("no cs8900 or cs8920 detected.\n");
ditto
+		goto out;
+	}
+
+	platform_set_drvdata(pdev, dev);
+	return 0;
+
+out:
+	free_netdev(dev);
+	return err;
+}
+
+static int cs89x0_platform_remove(struct platform_device *pdev)
+{
+	struct net_device *dev = platform_get_drvdata(pdev);
+
+	unregister_netdev(dev);
+	free_netdev(dev);
+	return 0;
+}
+
+static struct platform_driver cs89x0_platform_driver = {
+	.driver = {
+		.name	= DRV_NAME,
+		.owner	= THIS_MODULE
+	},
+	.probe	= cs89x0_platform_probe,
+	.remove	= cs89x0_platform_remove
Please add a comma at the end of the last elements aswell. The rationale
if that we can add elements later without touching the existing lines.
quoted hunk ↗ jump to hunk
 
 /*
  * Local variables:
@@ -1911,3 +1998,4 @@ cleanup_module(void)
  * End:
  *
  */
+
Please don't add blank lines at the end of files.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help