All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem
@ 2015-04-25 13:07 Sudip Mukherjee
  2015-04-25 13:07 ` [PATCH v3 WIP 2/3] staging: panel: modify driver to use new parport device model Sudip Mukherjee
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2015-04-25 13:07 UTC (permalink / raw)
  To: Greg KH, Dan Carpenter, One Thousand Gnomes, jdelvare
  Cc: linux-kernel, Sudip Mukherjee

another WIP for your review. since this is not a formal patch for
applying so writing the comments here.

all points raised by Dan in v2 and v1 has been covered in this patch.
almost all the points raised by Greg in v1 has also been covered.

apart from those points, v2 started using probe function. Without
probe, whenever any driver is trying to register, it is getting bound
to all the available parallel ports. To solve that probe was required.
Now the driver is binding only to the device it has registered. And
that device will appear as a subdevice of the particular parallel port
it wants to use.

In v1 Greg mentioned that we do not need to maintain our own list. That
has been partially done. we are no longer maintaining the list of
drivers. But we still need to maintain the list of ports and devices as
that will be used by drivers which are not yet converted to device
model. When all drivers are converted to use the device-model parallel
port all these lists can be removed.

v2 had one more problem: it was creating some ghost parallel ports
during port probing. we have now introduced the use of parport_del_port
to remove registerd ports if probing has failed.

This version can be tested.
as of now only staging/panel and i2c-parport has been converted. I have
tested this code with staging/panel.

Hi Alan,
for your testing I think you will need paride and pcd, I was looking at
them now, but i suppose it will be little tricky to convert them as it
is doing its own probing of the parallel port based on the base address.
I will convert it and give you a patchset for your testing latest by
monday.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/parport/parport_pc.c |   4 +-
 drivers/parport/procfs.c     |  15 ++-
 drivers/parport/share.c      | 258 ++++++++++++++++++++++++++++++++++++++++---
 include/linux/parport.h      |  40 ++++++-
 4 files changed, 299 insertions(+), 18 deletions(-)

diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index 53d15b3..78530d1 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -2255,7 +2255,7 @@ out5:
 		release_region(base+0x3, 5);
 	release_region(base, 3);
 out4:
-	parport_put_port(p);
+	parport_del_port(p);
 out3:
 	kfree(priv);
 out2:
@@ -2294,7 +2294,7 @@ void parport_pc_unregister_port(struct parport *p)
 				    priv->dma_handle);
 #endif
 	kfree(p->private_data);
-	parport_put_port(p);
+	parport_del_port(p);
 	kfree(ops); /* hope no-one cached it */
 }
 EXPORT_SYMBOL(parport_pc_unregister_port);
diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index 3b47080..c776333 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -21,6 +21,7 @@
 #include <linux/parport.h>
 #include <linux/ctype.h>
 #include <linux/sysctl.h>
+#include <linux/device.h>
 
 #include <asm/uaccess.h>
 
@@ -558,8 +559,18 @@ int parport_device_proc_unregister(struct pardevice *device)
 
 static int __init parport_default_proc_register(void)
 {
+	int ret;
+
 	parport_default_sysctl_table.sysctl_header =
 		register_sysctl_table(parport_default_sysctl_table.dev_dir);
+	if (!parport_default_sysctl_table.sysctl_header)
+		return -ENOMEM;
+	ret = parport_bus_init();
+	if (ret) {
+		unregister_sysctl_table(parport_default_sysctl_table.
+					sysctl_header);
+		return ret;
+	}
 	return 0;
 }
 
@@ -570,6 +581,7 @@ static void __exit parport_default_proc_unregister(void)
 					sysctl_header);
 		parport_default_sysctl_table.sysctl_header = NULL;
 	}
+	parport_bus_exit();
 }
 
 #else /* no sysctl or no procfs*/
@@ -596,11 +608,12 @@ int parport_device_proc_unregister(struct pardevice *device)
 
 static int __init parport_default_proc_register (void)
 {
-	return 0;
+	return parport_bus_init();
 }
 
 static void __exit parport_default_proc_unregister (void)
 {
+	parport_bus_exit();
 }
 #endif
 
diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index 3fa6624..8f2dfec 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include <linux/kmod.h>
+#include <linux/device.h>
 
 #include <linux/spinlock.h>
 #include <linux/mutex.h>
@@ -100,6 +101,20 @@ static struct parport_operations dead_ops = {
 	.owner		= NULL,
 };
 
+static struct bus_type parport_bus_type = {
+	.name = "parport",
+};
+
+int parport_bus_init(void)
+{
+	return bus_register(&parport_bus_type);
+}
+
+void parport_bus_exit(void)
+{
+	bus_unregister(&parport_bus_type);
+}
+
 /* Call attach(port) for each registered driver. */
 static void attach_driver_chain(struct parport *port)
 {
@@ -157,6 +172,7 @@ int parport_register_driver (struct parport_driver *drv)
 
 	if (list_empty(&portlist))
 		get_lowlevel_driver ();
+	drv->devmodel = false;
 
 	mutex_lock(&registration_lock);
 	list_for_each_entry(port, &portlist, list)
@@ -167,6 +183,67 @@ int parport_register_driver (struct parport_driver *drv)
 	return 0;
 }
 
+/*
+ * iterates through all the devices connected to the bus and sends the device
+ * details to the match_port callback of the driver, so that the driver can
+ * know what are all the ports that are connected to the bus and choose the
+ * port to which it wants to register its device.
+ */
+static int port_check(struct device *dev, void *_drv)
+{
+	int ret;
+	struct parport_driver *drv = _drv;
+
+	ret = drv->match_port(to_parport_dev(dev));
+	if (ret)
+		return 0;
+
+	return 1;
+}
+
+/*
+ * __parport_register_drv - register a new parport driver
+ * @drv: the driver structure to register
+ * @owner: owner module of drv
+ * @mod_name: module name string
+ *
+ * Adds the driver structure to the list of registered drivers.
+ * Returns a negative value on error, otherwise 0.
+ * If no error occurred, the driver remains registered even if
+ * no device was claimed during registration.
+ */
+int __parport_register_drv(struct parport_driver *drv,
+			   struct module *owner, const char *mod_name)
+{
+	int ret;
+
+	if (!drv->probe) {
+		pr_err("please use probe in %s\n", drv->name);
+		return -ENODEV;
+	}
+
+	if (list_empty(&portlist))
+		get_lowlevel_driver();
+
+	/* initialize common driver fields */
+	drv->driver.name = drv->name;
+	drv->driver.bus = &parport_bus_type;
+	drv->driver.owner = owner;
+	drv->driver.mod_name = mod_name;
+	drv->driver.probe = drv->probe;
+	drv->devmodel = true;
+	ret = driver_register(&drv->driver);
+	if (ret)
+		return ret;
+
+	mutex_lock(&registration_lock);
+	bus_for_each_dev(&parport_bus_type, NULL, drv, port_check);
+	mutex_unlock(&registration_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(__parport_register_drv);
+
 /**
  *	parport_unregister_driver - deregister a parallel port device driver
  *	@drv: structure describing the driver that was given to
@@ -189,15 +266,21 @@ void parport_unregister_driver (struct parport_driver *drv)
 	struct parport *port;
 
 	mutex_lock(&registration_lock);
-	list_del_init(&drv->list);
-	list_for_each_entry(port, &portlist, list)
-		drv->detach(port);
+	if (drv->devmodel) {
+		driver_unregister(&drv->driver);
+	} else {
+		list_del_init(&drv->list);
+		list_for_each_entry(port, &portlist, list)
+			drv->detach(port);
+	}
 	mutex_unlock(&registration_lock);
 }
 
-static void free_port (struct parport *port)
+static void free_port(struct device *dev)
 {
 	int d;
+	struct parport *port = to_parport_dev(dev);
+
 	spin_lock(&full_list_lock);
 	list_del(&port->full_list);
 	spin_unlock(&full_list_lock);
@@ -223,9 +306,16 @@ static void free_port (struct parport *port)
 
 struct parport *parport_get_port (struct parport *port)
 {
-	atomic_inc (&port->ref_count);
-	return port;
+	struct device *dev = get_device(&port->bus_dev);
+
+	return to_parport_dev(dev);
+}
+
+void parport_del_port(struct parport *port)
+{
+	device_unregister(&port->bus_dev);
 }
+EXPORT_SYMBOL(parport_del_port);
 
 /**
  *	parport_put_port - decrement a port's reference count
@@ -237,11 +327,7 @@ struct parport *parport_get_port (struct parport *port)
 
 void parport_put_port (struct parport *port)
 {
-	if (atomic_dec_and_test (&port->ref_count))
-		/* Can destroy it now. */
-		free_port (port);
-
-	return;
+	put_device(&port->bus_dev);
 }
 
 /**
@@ -281,6 +367,7 @@ struct parport *parport_register_port(unsigned long base, int irq, int dma,
 	int num;
 	int device;
 	char *name;
+	int ret;
 
 	tmp = kzalloc(sizeof(struct parport), GFP_KERNEL);
 	if (!tmp) {
@@ -333,6 +420,9 @@ struct parport *parport_register_port(unsigned long base, int irq, int dma,
 	 */
 	sprintf(name, "parport%d", tmp->portnum = tmp->number);
 	tmp->name = name;
+	tmp->bus_dev.bus = &parport_bus_type;
+	tmp->bus_dev.release = free_port;
+	dev_set_name(&tmp->bus_dev, name);
 
 	for (device = 0; device < 5; device++)
 		/* assume the worst */
@@ -340,6 +430,12 @@ struct parport *parport_register_port(unsigned long base, int irq, int dma,
 
 	tmp->waithead = tmp->waittail = NULL;
 
+	ret = device_register(&tmp->bus_dev);
+	if (ret) {
+		put_device(&tmp->bus_dev);
+		return NULL;
+	}
+
 	return tmp;
 }
 
@@ -575,6 +671,7 @@ parport_register_device(struct parport *port, const char *name,
 	tmp->irq_func = irq_func;
 	tmp->waiting = 0;
 	tmp->timeout = 5 * HZ;
+	tmp->devmodel = false;
 
 	/* Chain this onto the list */
 	tmp->prev = NULL;
@@ -630,6 +727,136 @@ parport_register_device(struct parport *port, const char *name,
 	return NULL;
 }
 
+static void free_pardevice(struct device *dev)
+{
+	struct pardevice *par_dev = to_pardevice(dev);
+
+	kfree(par_dev->name);
+	kfree(par_dev);
+}
+
+struct pardevice *
+parport_register_dev_model(struct parport *port, const char *name,
+			   struct pardev_cb *par_dev_cb)
+{
+	struct pardevice *par_dev;
+	int ret;
+	char *devname;
+
+	if (port->physport->flags & PARPORT_FLAG_EXCL) {
+		/* An exclusive device is registered. */
+		pr_err("%s: no more devices allowed\n", port->name);
+		return NULL;
+	}
+
+	if (par_dev_cb->flags & PARPORT_DEV_LURK) {
+		if (!par_dev_cb->preempt || !par_dev_cb->wakeup) {
+			pr_info("%s: refused to register lurking device (%s) without callbacks\n",
+				port->name, name);
+			return NULL;
+		}
+	}
+
+	if (!try_module_get(port->ops->owner))
+		return NULL;
+
+	parport_get_port(port);
+
+	par_dev = kzalloc(sizeof(*par_dev), GFP_KERNEL);
+	if (!par_dev)
+		goto err_par_dev;
+
+	par_dev->state = kzalloc(sizeof(*par_dev->state), GFP_KERNEL);
+	if (!par_dev->state)
+		goto err_put_par_dev;
+
+	devname = kstrdup(name, GFP_KERNEL);
+	if (!devname)
+		goto err_free_par_dev;
+
+	par_dev->name = devname;
+	par_dev->port = port;
+	par_dev->daisy = -1;
+	par_dev->preempt = par_dev_cb->preempt;
+	par_dev->wakeup = par_dev_cb->wakeup;
+	par_dev->private = par_dev_cb->private;
+	par_dev->flags = par_dev_cb->flags;
+	par_dev->irq_func = par_dev_cb->irq_func;
+	par_dev->waiting = 0;
+	par_dev->timeout = 5 * HZ;
+
+	par_dev->dev.parent = &port->bus_dev;
+	par_dev->dev.bus = &parport_bus_type;
+	ret = dev_set_name(&par_dev->dev, "%s", devname);
+	if (ret)
+		goto err_free_devname;
+	par_dev->dev.release = free_pardevice;
+	par_dev->devmodel = true;
+	ret = device_register(&par_dev->dev);
+	if (ret)
+		goto err_put_dev;
+
+	/* Chain this onto the list */
+	par_dev->prev = NULL;
+	/*
+	 * This function must not run from an irq handler so we don' t need
+	 * to clear irq on the local CPU. -arca
+	 */
+	spin_lock(&port->physport->pardevice_lock);
+
+	if (par_dev_cb->flags & PARPORT_DEV_EXCL) {
+		if (port->physport->devices) {
+			spin_unlock(&port->physport->pardevice_lock);
+			pr_debug("%s: cannot grant exclusive access for device %s\n",
+				 port->name, name);
+			goto err_put_dev;
+		}
+		port->flags |= PARPORT_FLAG_EXCL;
+	}
+
+	par_dev->next = port->physport->devices;
+	wmb();	/*
+		 * Make sure that tmp->next is written before it's
+		 * added to the list; see comments marked 'no locking
+		 * required'
+		 */
+	if (port->physport->devices)
+		port->physport->devices->prev = par_dev;
+	port->physport->devices = par_dev;
+	spin_unlock(&port->physport->pardevice_lock);
+
+	init_waitqueue_head(&par_dev->wait_q);
+	par_dev->timeslice = parport_default_timeslice;
+	par_dev->waitnext = NULL;
+	par_dev->waitprev = NULL;
+
+	/*
+	 * This has to be run as last thing since init_state may need other
+	 * pardevice fields. -arca
+	 */
+	port->ops->init_state(par_dev, par_dev->state);
+	port->proc_device = par_dev;
+	parport_device_proc_register(par_dev);
+
+	return par_dev;
+
+err_put_dev:
+	put_device(&par_dev->dev);
+err_free_devname:
+	kfree(devname);
+err_free_par_dev:
+	kfree(par_dev->state);
+err_put_par_dev:
+	if (!par_dev->devmodel)
+		kfree(par_dev);
+err_par_dev:
+	parport_put_port(port);
+	module_put(port->ops->owner);
+
+	return NULL;
+}
+EXPORT_SYMBOL(parport_register_dev_model);
+
 /**
  *	parport_unregister_device - deregister a device on a parallel port
  *	@dev: pointer to structure representing device
@@ -691,7 +918,10 @@ void parport_unregister_device(struct pardevice *dev)
 	spin_unlock_irq(&port->waitlist_lock);
 
 	kfree(dev->state);
-	kfree(dev);
+	if (dev->devmodel)
+		device_unregister(&dev->dev);
+	else
+		kfree(dev);
 
 	module_put(port->ops->owner);
 	parport_put_port (port);
@@ -926,8 +1156,8 @@ int parport_claim_or_block(struct pardevice *dev)
 			       dev->port->physport->cad ?
 			       dev->port->physport->cad->name:"nobody");
 #endif
-	}
-	dev->waiting = 0;
+	} else if (r == 0)
+		dev->waiting = 0;
 	return r;
 }
 
diff --git a/include/linux/parport.h b/include/linux/parport.h
index c22f125..8973da8 100644
--- a/include/linux/parport.h
+++ b/include/linux/parport.h
@@ -13,6 +13,7 @@
 #include <linux/wait.h>
 #include <linux/irqreturn.h>
 #include <linux/semaphore.h>
+#include <linux/device.h>
 #include <asm/ptrace.h>
 #include <uapi/linux/parport.h>
 
@@ -145,6 +146,8 @@ struct pardevice {
 	unsigned int flags;
 	struct pardevice *next;
 	struct pardevice *prev;
+	struct device dev;
+	bool devmodel;
 	struct parport_state *state;     /* saved status over preemption */
 	wait_queue_head_t wait_q;
 	unsigned long int time;
@@ -156,6 +159,8 @@ struct pardevice {
 	void * sysctl_table;
 };
 
+#define to_pardevice(n) container_of(n, struct pardevice, dev)
+
 /* IEEE1284 information */
 
 /* IEEE1284 phases. These are exposed to userland through ppdev IOCTL
@@ -195,7 +200,7 @@ struct parport {
 				 * This may unfortulately be null if the
 				 * port has a legacy driver.
 				 */
-
+	struct device bus_dev;	/* to link with the bus */
 	struct parport *physport;
 				/* If this is a non-default mux
 				   parport, i.e. we're a clone of a real
@@ -245,15 +250,24 @@ struct parport {
 	struct parport *slaves[3];
 };
 
+#define to_parport_dev(n) container_of(n, struct parport, bus_dev)
+
 #define DEFAULT_SPIN_TIME 500 /* us */
 
 struct parport_driver {
 	const char *name;
 	void (*attach) (struct parport *);
 	void (*detach) (struct parport *);
+	int (*match_port)(struct parport *);
+	int (*probe)(struct device *);
+	struct device_driver driver;
+	bool devmodel;
 	struct list_head list;
 };
 
+int parport_bus_init(void);
+void parport_bus_exit(void);
+
 /* parport_register_port registers a new parallel port at the given
    address (if one does not already exist) and returns a pointer to it.
    This entails claiming the I/O region, IRQ and DMA.  NULL is returned
@@ -274,8 +288,19 @@ extern void parport_remove_port(struct parport *port);
 /* Register a new high-level driver. */
 extern int parport_register_driver (struct parport_driver *);
 
+int __must_check __parport_register_drv(struct parport_driver *,
+					struct module *,
+					const char *mod_name);
+/*
+ * parport_register_drv must be a macro so that KBUILD_MODNAME can
+ * be expanded
+ */
+#define parport_register_drv(driver)             \
+	__parport_register_drv(driver, THIS_MODULE, KBUILD_MODNAME)
+
 /* Unregister a high-level driver. */
 extern void parport_unregister_driver (struct parport_driver *);
+void parport_unregister_driver(struct parport_driver *);
 
 /* If parport_register_driver doesn't fit your needs, perhaps
  * parport_find_xxx does. */
@@ -288,6 +313,15 @@ extern irqreturn_t parport_irq_handler(int irq, void *dev_id);
 /* Reference counting for ports. */
 extern struct parport *parport_get_port (struct parport *);
 extern void parport_put_port (struct parport *);
+void parport_del_port(struct parport *);
+
+struct pardev_cb {
+	int (*preempt)(void *);
+	void (*wakeup)(void *);
+	void *private;
+	void (*irq_func)(void *);
+	unsigned int flags;
+};
 
 /* parport_register_device declares that a device is connected to a
    port, and tells the kernel all it needs to know.
@@ -301,6 +335,10 @@ struct pardevice *parport_register_device(struct parport *port,
 			  void (*irq_func)(void *), 
 			  int flags, void *handle);
 
+struct pardevice *
+parport_register_dev_model(struct parport *port, const char *name,
+			   struct pardev_cb *par_dev_cb);
+
 /* parport_unregister unlinks a device from the chain. */
 extern void parport_unregister_device(struct pardevice *dev);
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 WIP 2/3] staging: panel: modify driver to use new parport device model
  2015-04-25 13:07 [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem Sudip Mukherjee
@ 2015-04-25 13:07 ` Sudip Mukherjee
  2015-04-25 13:07 ` [PATCH v3 WIP 3/3] i2c-parport: " Sudip Mukherjee
  2015-04-25 22:35 ` [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem One Thousand Gnomes
  2 siblings, 0 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2015-04-25 13:07 UTC (permalink / raw)
  To: Greg KH, Dan Carpenter, One Thousand Gnomes, jdelvare
  Cc: linux-kernel, Sudip Mukherjee

modify panel driver to use the new parallel port device model.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/panel/panel.c | 63 +++++++++++++++++--------------------------
 1 file changed, 24 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index ea54fb4..0aca810 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -2188,25 +2188,35 @@ static struct notifier_block panel_notifier = {
 	0
 };
 
-static void panel_attach(struct parport *port)
+static int panel_probe(struct device *dev)
+{
+	if (strcmp(dev_name(dev), "panel"))
+		return -ENODEV;
+
+	return 0;
+}
+
+struct pardev_cb panel_cb = {
+	.flags = 0,	/*PARPORT_DEV_EXCL */
+	.private = &pprt,
+};
+
+static int panel_attach(struct parport *port)
 {
 	if (port->number != parport)
-		return;
+		return -ENODEV;
 
 	if (pprt) {
 		pr_err("%s: port->number=%d parport=%d, already registered!\n",
 		       __func__, port->number, parport);
-		return;
+		return -EAGAIN;
 	}
 
-	pprt = parport_register_device(port, "panel", NULL, NULL,  /* pf, kf */
-				       NULL,
-				       /*PARPORT_DEV_EXCL */
-				       0, (void *)&pprt);
+	pprt = parport_register_dev_model(port, "panel", &panel_cb);
 	if (pprt == NULL) {
 		pr_err("%s: port->number=%d parport=%d, parport_register_device() failed\n",
 		       __func__, port->number, parport);
-		return;
+		return -ENODEV;
 	}
 
 	if (parport_claim(pprt)) {
@@ -2230,7 +2240,7 @@ static void panel_attach(struct parport *port)
 			goto err_lcd_unreg;
 	}
 	register_reboot_notifier(&panel_notifier);
-	return;
+	return 0;
 
 err_lcd_unreg:
 	if (lcd.enabled)
@@ -2238,40 +2248,14 @@ err_lcd_unreg:
 err_unreg_device:
 	parport_unregister_device(pprt);
 	pprt = NULL;
-}
 
-static void panel_detach(struct parport *port)
-{
-	if (port->number != parport)
-		return;
-
-	if (!pprt) {
-		pr_err("%s: port->number=%d parport=%d, nothing to unregister.\n",
-		       __func__, port->number, parport);
-		return;
-	}
-
-	unregister_reboot_notifier(&panel_notifier);
-
-	if (keypad.enabled && keypad_initialized) {
-		misc_deregister(&keypad_dev);
-		keypad_initialized = 0;
-	}
-
-	if (lcd.enabled && lcd.initialized) {
-		misc_deregister(&lcd_dev);
-		lcd.initialized = false;
-	}
-
-	parport_release(pprt);
-	parport_unregister_device(pprt);
-	pprt = NULL;
+	return -ENODEV;
 }
 
 static struct parport_driver panel_driver = {
 	.name = "panel",
-	.attach = panel_attach,
-	.detach = panel_detach,
+	.match_port = panel_attach,
+	.probe = panel_probe,
 };
 
 /* init function */
@@ -2380,7 +2364,7 @@ static int __init panel_init_module(void)
 		return -ENODEV;
 	}
 
-	err = parport_register_driver(&panel_driver);
+	err = parport_register_drv(&panel_driver);
 	if (err) {
 		pr_err("could not register with parport. Aborting.\n");
 		return err;
@@ -2398,6 +2382,7 @@ static int __init panel_init_module(void)
 
 static void __exit panel_cleanup_module(void)
 {
+	unregister_reboot_notifier(&panel_notifier);
 
 	if (scan_timer.function != NULL)
 		del_timer_sync(&scan_timer);
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 WIP 3/3] i2c-parport: modify driver to use new parport device model
  2015-04-25 13:07 [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem Sudip Mukherjee
  2015-04-25 13:07 ` [PATCH v3 WIP 2/3] staging: panel: modify driver to use new parport device model Sudip Mukherjee
@ 2015-04-25 13:07 ` Sudip Mukherjee
  2015-04-25 22:32   ` One Thousand Gnomes
  2015-04-25 22:35 ` [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem One Thousand Gnomes
  2 siblings, 1 reply; 6+ messages in thread
From: Sudip Mukherjee @ 2015-04-25 13:07 UTC (permalink / raw)
  To: Greg KH, Dan Carpenter, One Thousand Gnomes, jdelvare
  Cc: linux-kernel, Sudip Mukherjee

modify i2c-parport driver to use the new parallel port device model.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

H Jean,
based on our discussion I have given one more module parameter with
default as parport0 to connect to. And I have tested with i2c-parport
connected to my parport1 and staging/panel connected to parport0.
will request you to kindly test this series on your hardware...

 drivers/i2c/busses/i2c-parport.c | 76 ++++++++++++++++++++++++----------------
 drivers/i2c/busses/i2c-parport.h |  4 +++
 2 files changed, 49 insertions(+), 31 deletions(-)

diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
index a1fac5a..e173c72 100644
--- a/drivers/i2c/busses/i2c-parport.c
+++ b/drivers/i2c/busses/i2c-parport.c
@@ -160,20 +160,29 @@ static void i2c_parport_irq(void *data)
 			"SMBus alert received but no ARA client!\n");
 }
 
-static void i2c_parport_attach(struct parport *port)
+static struct pardev_cb i2c_parport_cb = {
+	.flags = PARPORT_FLAG_EXCL,
+	.irq_func = i2c_parport_irq,
+};
+
+static int i2c_parport_attach(struct parport *port)
 {
 	struct i2c_par *adapter;
 
+	if (port->number != parport)
+		return -ENODEV;
+
 	adapter = kzalloc(sizeof(struct i2c_par), GFP_KERNEL);
 	if (adapter == NULL) {
 		printk(KERN_ERR "i2c-parport: Failed to kzalloc\n");
-		return;
+		return -ENOMEM;
 	}
+	i2c_parport_cb.private = adapter;
 
 	pr_debug("i2c-parport: attaching to %s\n", port->name);
 	parport_disable_irq(port);
-	adapter->pdev = parport_register_device(port, "i2c-parport",
-		NULL, NULL, i2c_parport_irq, PARPORT_FLAG_EXCL, adapter);
+	adapter->pdev = parport_register_dev_model(port, "i2c-parport",
+						   &i2c_parport_cb);
 	if (!adapter->pdev) {
 		printk(KERN_ERR "i2c-parport: Unable to register with parport\n");
 		goto err_free;
@@ -230,46 +239,28 @@ static void i2c_parport_attach(struct parport *port)
 	mutex_lock(&adapter_list_lock);
 	list_add_tail(&adapter->node, &adapter_list);
 	mutex_unlock(&adapter_list_lock);
-	return;
+	return 0;
 
  err_unregister:
 	parport_release(adapter->pdev);
 	parport_unregister_device(adapter->pdev);
  err_free:
 	kfree(adapter);
+	return -ENODEV;
 }
 
-static void i2c_parport_detach(struct parport *port)
+static int i2c_parport_probe(struct device *dev)
 {
-	struct i2c_par *adapter, *_n;
+	if (strcmp(dev_name(dev), "i2c-parport"))
+		return -ENODEV;
 
-	/* Walk the list */
-	mutex_lock(&adapter_list_lock);
-	list_for_each_entry_safe(adapter, _n, &adapter_list, node) {
-		if (adapter->pdev->port == port) {
-			if (adapter->ara) {
-				parport_disable_irq(port);
-				i2c_unregister_device(adapter->ara);
-			}
-			i2c_del_adapter(&adapter->adapter);
-
-			/* Un-init if needed (power off...) */
-			if (adapter_parm[type].init.val)
-				line_set(port, 0, &adapter_parm[type].init);
-
-			parport_release(adapter->pdev);
-			parport_unregister_device(adapter->pdev);
-			list_del(&adapter->node);
-			kfree(adapter);
-		}
-	}
-	mutex_unlock(&adapter_list_lock);
+	return 0;
 }
 
 static struct parport_driver i2c_parport_driver = {
 	.name	= "i2c-parport",
-	.attach	= i2c_parport_attach,
-	.detach	= i2c_parport_detach,
+	.match_port	= i2c_parport_attach,
+	.probe	= i2c_parport_probe,
 };
 
 /* ----- Module loading, unloading and information ------------------------ */
@@ -286,11 +277,34 @@ static int __init i2c_parport_init(void)
 		return -ENODEV;
 	}
 
-	return parport_register_driver(&i2c_parport_driver);
+	return parport_register_drv(&i2c_parport_driver);
 }
 
 static void __exit i2c_parport_exit(void)
 {
+	struct i2c_par *adapter, *_n;
+
+	/* Walk the list */
+	mutex_lock(&adapter_list_lock);
+	list_for_each_entry_safe(adapter, _n, &adapter_list, node) {
+		if (adapter->ara) {
+			parport_disable_irq(adapter->pdev->port);
+			i2c_unregister_device(adapter->ara);
+		}
+		i2c_del_adapter(&adapter->adapter);
+
+		/* Un-init if needed (power off...) */
+		if (adapter_parm[type].init.val)
+			line_set(adapter->pdev->port, 0,
+				 &adapter_parm[type].init);
+
+		parport_release(adapter->pdev);
+		parport_unregister_device(adapter->pdev);
+		list_del(&adapter->node);
+		kfree(adapter);
+	}
+	mutex_unlock(&adapter_list_lock);
+
 	parport_unregister_driver(&i2c_parport_driver);
 }
 
diff --git a/drivers/i2c/busses/i2c-parport.h b/drivers/i2c/busses/i2c-parport.h
index 4e12945..59afa83 100644
--- a/drivers/i2c/busses/i2c-parport.h
+++ b/drivers/i2c/busses/i2c-parport.h
@@ -104,3 +104,7 @@ MODULE_PARM_DESC(type,
 	" 6 = Barco LPT->DVI (K5800236) adapter\n"
 	" 7 = One For All JP1 parallel port adapter\n"
 );
+
+static int parport;
+module_param(parport, int, 0);
+MODULE_PARM_DESC(parport, "The parport to be used, default parport0\n");
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 WIP 3/3] i2c-parport: modify driver to use new parport device model
  2015-04-25 13:07 ` [PATCH v3 WIP 3/3] i2c-parport: " Sudip Mukherjee
@ 2015-04-25 22:32   ` One Thousand Gnomes
  2015-04-26  5:04     ` Sudip Mukherjee
  0 siblings, 1 reply; 6+ messages in thread
From: One Thousand Gnomes @ 2015-04-25 22:32 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: Greg KH, Dan Carpenter, jdelvare, linux-kernel

> +static int parport;
> +module_param(parport, int, 0);
> +MODULE_PARM_DESC(parport, "The parport to be used, default parport0\n");

Minor suggested enhancement: this ought to be an array so you can attach
multiple instances to a system.

Alan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem
  2015-04-25 13:07 [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem Sudip Mukherjee
  2015-04-25 13:07 ` [PATCH v3 WIP 2/3] staging: panel: modify driver to use new parport device model Sudip Mukherjee
  2015-04-25 13:07 ` [PATCH v3 WIP 3/3] i2c-parport: " Sudip Mukherjee
@ 2015-04-25 22:35 ` One Thousand Gnomes
  2 siblings, 0 replies; 6+ messages in thread
From: One Thousand Gnomes @ 2015-04-25 22:35 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: Greg KH, Dan Carpenter, jdelvare, linux-kernel

> for your testing I think you will need paride and pcd, I was looking at
> them now, but i suppose it will be little tricky to convert them as it
> is doing its own probing of the parallel port based on the base address.
> I will convert it and give you a patchset for your testing latest by
> monday.

No hurry I doubt the CDROM will be here until the middle of the week, but
will give it a go once it arrives. To be honest I suspect ZIP (imm and
epp) are probably the only drives that still get used much.

Alan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 WIP 3/3] i2c-parport: modify driver to use new parport device model
  2015-04-25 22:32   ` One Thousand Gnomes
@ 2015-04-26  5:04     ` Sudip Mukherjee
  0 siblings, 0 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2015-04-26  5:04 UTC (permalink / raw)
  To: One Thousand Gnomes; +Cc: Greg KH, Dan Carpenter, jdelvare, linux-kernel

On Sat, Apr 25, 2015 at 11:32:47PM +0100, One Thousand Gnomes wrote:
> > +static int parport;
> > +module_param(parport, int, 0);
> > +MODULE_PARM_DESC(parport, "The parport to be used, default parport0\n");
> 
> Minor suggested enhancement: this ought to be an array so you can attach
> multiple instances to a system.
ok. infact, i was looking at paride yesterday night, and it is also
having option to register maximum of 4 instances. the current patchset
allows registration of device to only one port. I need to change that
part.

regards
sudip


> 
> Alan

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-04-26  5:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-25 13:07 [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem Sudip Mukherjee
2015-04-25 13:07 ` [PATCH v3 WIP 2/3] staging: panel: modify driver to use new parport device model Sudip Mukherjee
2015-04-25 13:07 ` [PATCH v3 WIP 3/3] i2c-parport: " Sudip Mukherjee
2015-04-25 22:32   ` One Thousand Gnomes
2015-04-26  5:04     ` Sudip Mukherjee
2015-04-25 22:35 ` [PATCH v3 WIP 1/3] parport: add device-model to parport subsystem One Thousand Gnomes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.