pcmcia: export pcmcia_bus_type

Export pcmcia_bus_type so module-based drivers can register bus notifiers, and add it to what seems to be the main include file.

Quoted 2 lines · Olof Johansson

Export pcmcia_bus_type so module-based drivers can register bus notifiers, and add it to what seems to be the main include file.

This all sounds rather fishy to me. Care to send the code along that makes use of it.

And even if we're going to add the export it should be _GPL because it's a deeply internal implementation detail. Then again I'm not exactly convinced we should export it yet.

drivers/pcmcia/electra_cf.c new file +6 -0

+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+
+#include <pcmcia/ss.h>

drivers/pcmcia/electra_cf.c new file +10 -0

+
+	cf = container_of(s, struct electra_cf_socket, socket);
+
+	/* NOTE CF is always 3VCARD */
+	if (electra_cf_present(cf)) {
+		struct electra_cf_socket *cf;
+
+		*sp = SS_READY | SS_DETECT | SS_POWERON | SS_3VCARD;
+		cf = container_of(s, struct electra_cf_socket, socket);
+		s->pci_irq = cf->irq;

...

drivers/pcmcia/electra_cf.c new file +80 -0

+static int __devinit electra_cf_probe(struct of_device *ofdev,
+				      const struct of_device_id *match)
+{
+	struct device *device = &ofdev->dev;
+	struct device_node *np = ofdev->node;
+	struct electra_cf_socket   *cf;
+	struct resource mem, io;
+	int status;
+	const unsigned int *prop;
+	int err;
+
+	err = of_address_to_resource(np, 0, &mem);
+	if (err)
+		return -EINVAL;
+
+	err = of_address_to_resource(np, 1, &io);
+	if (err)
+		return -EINVAL;
+
+	cf = kzalloc(sizeof *cf, GFP_KERNEL);
+	if (!cf)
+		return -ENOMEM;
+
+	init_timer(&cf->timer);
+	cf->timer.function = electra_cf_timer;
+	cf->timer.data = (unsigned long) cf;
+
+	cf->ofdev = ofdev;
+	cf->mem_phys = mem.start;
+	cf->mem_base = ioremap(mem.start, mem.end - mem.start);
+	cf->io_base = ioremap(io.start, io.end - io.start);
+	cf->gpio_base = ioremap(0xfc103000, 0x1000);
+	dev_set_drvdata(device, cf);
+
+	if (!cf->mem_base || !cf->io_base || !cf->gpio_base) {
+		dev_err(device, "can't ioremap ranges\n");
+		status = -ENOMEM;
+		goto fail1;
+	}
+
+	cf->iomem.start = (unsigned long)cf->io_base;
+	cf->iomem.end = (unsigned long)cf->io_base + (io.end - io.start);
+	cf->iomem.flags = IORESOURCE_MEM;
+
+	cf->irq = irq_of_parse_and_map(np, 0);
+
+	status = request_irq(cf->irq, electra_cf_irq, IRQF_SHARED,
+			     driver_name, cf);
+	if (status < 0) {
+		dev_err(device, "request_irq failed\n");
+		goto fail1;
+	}
+
+	cf->socket.pci_irq = cf->irq;
+
+	prop = get_property(np, "card-detect-gpio", NULL);
+	cf->gpio_detect = *prop;
+	prop = get_property(np, "card-vsense-gpio", NULL);
+	cf->gpio_vsense = *prop;
+	prop = get_property(np, "card-3v-gpio", NULL);
+	cf->gpio_3v = *prop;
+	prop = get_property(np, "card-5v-gpio", NULL);
+	cf->gpio_5v = *prop;
+
+	/* pcmcia layer only remaps "real" memory not iospace */
+	cf->socket.io_offset = (unsigned long)cf->io_base;
+
+	/* reserve chip-select regions */
+	if (!request_mem_region(mem.start, mem.end + 1 - mem.start,
+				driver_name)) {
+		status = -ENXIO;
+		dev_err(device, "Can't claim memory region\n");
+		goto fail1;
+	}
+
+	if (!request_mem_region(io.start, io.end + 1 - io.start,
+				driver_name)) {
+		status = -ENXIO;
+		dev_err(device, "Can't claim I/O region\n");
+		goto fail2;

drivers/pcmcia/electra_cf.c new file +32 -0

+
+	status = pcmcia_register_socket(&cf->socket);
+	if (status < 0) {
+		dev_err(device, "pcmcia_register_socket failed\n");
+		goto fail3;
+	}
+
+	dev_info(device, "at mem 0x%lx io 0x%lx irq %d\n",
+		 mem.start, io.start, cf->irq);
+
+	cf->active = 1;
+	electra_cf_timer((unsigned long)cf);
+	return 0;
+
+fail3:
+	release_mem_region(io.start, io.end + 1 - io.start);
+fail2:
+	release_mem_region(mem.start, mem.end + 1 - mem.start);
+fail1:
+	if (cf->io_base)
+		iounmap(cf->io_base);
+	if (cf->mem_base)
+		iounmap(cf->mem_base);
+	if (cf->gpio_base)
+		iounmap(cf->gpio_base);
+	device_init_wakeup(&ofdev->dev, 0);
+	kfree(cf);
+	return status;
+
+}
+
+static int __devexit electra_cf_remove(struct of_device *ofdev)

drivers/pcmcia/electra_cf.c new file +11 -0

+
+	iounmap(cf->io_base);
+	iounmap(cf->mem_base);
+	iounmap(cf->gpio_base);
+	release_mem_region(cf->mem_phys, cf->mem_size);
+	release_region(cf->io_phys, cf->io_size);
+
+	kfree(cf);
+
+	return 0;
+}

Some random review comments on your driver, while I was passing. Spotted a few things.

drivers/pcmcia/electra_cf.c line 27

Quoted 5 lines

drivers/pcmcia/electra_cf.c new file +5 -0

+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/platform_device.h>
+#include <linux/platform_device.h>

Silly duplicate includes.

Quoted 10 lines

drivers/pcmcia/electra_cf.c new file +9 -0

+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+
+#include <pcmcia/ss.h>
+#include <asm/of_platform.h>
+
+static const char driver_name[] = "electra-cf";
...

drivers/pcmcia/electra_cf.c line 103

Quoted 6 lines

drivers/pcmcia/electra_cf.c new file +6 -0

+static int electra_cf_get_status(struct pcmcia_socket *s, u_int *sp)
+{
+	struct electra_cf_socket *cf;
+
+	if (!sp)
+		return -EINVAL;

Never called with a NULL argument, so useless check.

Quoted 15 lines

drivers/pcmcia/electra_cf.c new file +14 -0

+
+	cf = container_of(s, struct electra_cf_socket, socket);
+
+	/* NOTE CF is always 3VCARD */
+	if (electra_cf_present(cf)) {
+		struct electra_cf_socket *cf;
+
+		*sp = SS_READY | SS_DETECT | SS_POWERON | SS_3VCARD;
+		cf = container_of(s, struct electra_cf_socket, socket);
+		s->pci_irq = cf->irq;
+	} else
+		*sp = 0;
+	return 0;
+}
...

drivers/pcmcia/electra_cf.c line 157

Quoted 5 lines

drivers/pcmcia/electra_cf.c new file +5 -0

+static int electra_cf_ss_suspend(struct pcmcia_socket *s)
+{
+	pr_debug("%s: %s\n", driver_name, __FUNCTION__);
+	return electra_cf_set_socket(s, &dead_socket);
+}

That's already done for you. Remove this function entirely and set the ".suspend" method to NULL.

drivers/pcmcia/electra_cf.c line 169

Quoted 12 lines

drivers/pcmcia/electra_cf.c new file +11 -0

+static int electra_cf_set_io_map(struct pcmcia_socket *s,
+				 struct pccard_io_map *io)
+{
+	struct electra_cf_socket *cf;
+
+	cf = container_of(s, struct electra_cf_socket, socket);
+	io->flags &= MAP_ACTIVE|MAP_ATTRIB|MAP_16BIT;
+	io->start = (unsigned long)cf->io_base;
+	io->stop = (unsigned long)cf->io_base + 0x800 - 1;
+	return 0;
+}

PCMCIA code will ignore your writes to 'io'. Therefore, does this do anything useful? If not, an empty function will do.

drivers/pcmcia/electra_cf.c line 284

Quoted 8 lines

drivers/pcmcia/electra_cf.c new file +8 -0

+	cf->socket.owner = THIS_MODULE;
+	cf->socket.dev.parent = &ofdev->dev;
+	cf->socket.ops = &electra_cf_ops;
+	cf->socket.resource_ops = &pccard_static_ops;
+	cf->socket.features = SS_CAP_PCCARD | SS_CAP_STATIC_MAP |
+				SS_CAP_MEM_ALIGN;
+	cf->socket.map_size = 0x800;
+	cf->socket.io[0].res = &cf->iomem;

Not a good idea - PCMCIA manages this resources itself and will free it when the card is removed. Use ss_cap_static_map and socket.io_offset.

Quoted 6 lines

drivers/pcmcia/electra_cf.c new file +6 -0

+static int __devexit electra_cf_remove(struct of_device *ofdev)
+{
+	struct device *device = &ofdev->dev;
+	struct electra_cf_socket *cf;
+
+	cf = dev_get_drvdata(device);

drivers/pcmcia/electra_cf.c line 326

Quoted 5 lines

drivers/pcmcia/electra_cf.c new file +5 -0

+
+	cf->active = 0;
+	pcmcia_unregister_socket(&cf->socket);
+	del_timer_sync(&cf->timer);
+	free_irq(cf->irq, cf);

What if an IRQ occurs after the timer has been deleted? Doesn't the interrupt handler re-add the timer back?