All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
To: gregkh@linuxfoundation.org, dan.carpenter@oracle.com
Cc: linux-kernel@vger.kernel.org,
	Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Subject: [PATCH WIP] parport: add device model
Date: Fri, 10 Apr 2015 20:00:38 +0530	[thread overview]
Message-ID: <1428676238-17141-1-git-send-email-sudipm.mukherjee@gmail.com> (raw)

This is work-in-progree, not for applying to any tree. Posting now for
your comments so that I know if I am in the proper track.

in parport_register_driver() driver is registered but i am not linking
anywhere the device with the driver, but yet when I am testing this
patch I am seeing in sys tree that parport0 is linked with
the lp driver. Is it done in the device core? I am missing this step
somewhere.

In parport_claim() the attach is unchecked as of now, I think we will
need my initial patch series of monitoring the attach return value along
with it.

while testing I am getting NULL dereference with daisy.c, and after
disabling PARPORT_1284 , I am getting some new errors. so if you are
testing this patch please keep in mind that still lots of work is
pending.
My main intention to post it now is to know if my approach is correct.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/parport/procfs.c | 12 ++++++--
 drivers/parport/share.c  | 72 ++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/parport.h  | 21 +++++++++++++-
 3 files changed, 99 insertions(+), 6 deletions(-)

diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index 3b47080..1c49540 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -558,9 +558,15 @@ 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);
-	return 0;
+	ret = parport_bus_init();
+	if (ret)
+		unregister_sysctl_table(parport_default_sysctl_table.
+					sysctl_header);
+	return ret;
 }
 
 static void __exit parport_default_proc_unregister(void)
@@ -570,6 +576,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 +603,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..042863a 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,33 @@ static struct parport_operations dead_ops = {
 	.owner		= NULL,
 };
 
+/*
+ * This currently matches any parport driver to any parport device
+ * drivers themselves make the decision whether to drive this device
+ * in their probe method.
+ */
+
+static int parport_match(struct device *dev, struct device_driver *drv)
+{
+	return 1;
+}
+
+struct bus_type parport_bus_type = {
+	.name           = "parport",
+	.match		= parport_match,
+};
+EXPORT_SYMBOL(parport_bus_type);
+
+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)
 {
@@ -151,9 +179,11 @@ static void get_lowlevel_driver (void)
  *	Returns 0 on success.  Currently it always succeeds.
  **/
 
-int parport_register_driver (struct parport_driver *drv)
+int __parport_register_driver(struct parport_driver *drv,
+			      struct module *owner, const char *mod_name)
 {
 	struct parport *port;
+	int ret;
 
 	if (list_empty(&portlist))
 		get_lowlevel_driver ();
@@ -164,7 +194,22 @@ int parport_register_driver (struct parport_driver *drv)
 	list_add(&drv->list, &drivers);
 	mutex_unlock(&registration_lock);
 
-	return 0;
+	drv->driver.name = drv->name;
+	drv->driver.bus = &parport_bus_type;
+	drv->driver.owner = owner;
+	drv->driver.mod_name = mod_name;
+
+	/* register with core */
+	ret = driver_register(&drv->driver);
+	if (ret < 0) {
+		mutex_lock(&registration_lock);
+		list_del_init(&drv->list);
+		list_for_each_entry(port, &portlist, list)
+			drv->detach(port);
+		mutex_unlock(&registration_lock);
+	}
+
+	return ret;
 }
 
 /**
@@ -188,6 +233,7 @@ void parport_unregister_driver (struct parport_driver *drv)
 {
 	struct parport *port;
 
+	driver_unregister(&drv->driver);
 	mutex_lock(&registration_lock);
 	list_del_init(&drv->list);
 	list_for_each_entry(port, &portlist, list)
@@ -281,6 +327,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 +380,8 @@ struct parport *parport_register_port(unsigned long base, int irq, int dma,
 	 */
 	sprintf(name, "parport%d", tmp->portnum = tmp->number);
 	tmp->name = name;
+	tmp->ddev.bus = &parport_bus_type;
+	tmp->ddev.init_name = name;
 
 	for (device = 0; device < 5; device++)
 		/* assume the worst */
@@ -340,6 +389,12 @@ struct parport *parport_register_port(unsigned long base, int irq, int dma,
 
 	tmp->waithead = tmp->waittail = NULL;
 
+	ret = device_register(&tmp->ddev);
+	if (ret < 0) {
+		kfree(tmp);
+		return NULL;
+	}
+
 	return tmp;
 }
 
@@ -442,6 +497,8 @@ void parport_remove_port(struct parport *port)
 
 	mutex_unlock(&registration_lock);
 
+	device_unregister(&port->ddev);
+
 	parport_proc_unregister(port);
 
 	for (i = 1; i < 3; i++) {
@@ -774,12 +831,19 @@ int parport_claim(struct pardevice *dev)
 	struct pardevice *oldcad;
 	struct parport *port = dev->port->physport;
 	unsigned long flags;
+	int ret;
 
 	if (port->cad == dev) {
 		printk(KERN_INFO "%s: %s already owner\n",
 		       dev->port->name,dev->name);
 		return 0;
 	}
+	dev->dev.bus = &parport_bus_type;
+	dev->dev.parent = &port->ddev;
+	dev->dev.init_name = dev->name;
+	ret = device_register(&dev->dev);
+	if (ret < 0)
+		return ret;
 
 	/* Preempt any current device */
 	write_lock_irqsave (&port->cad_lock, flags);
@@ -866,6 +930,7 @@ blocked:
 		spin_unlock (&port->waitlist_lock);
 	}
 	write_unlock_irqrestore (&port->cad_lock, flags);
+	device_unregister(&dev->dev);
 	return -EAGAIN;
 }
 
@@ -971,6 +1036,7 @@ void parport_release(struct pardevice *dev)
 
 	port->cad = NULL;
 	write_unlock_irqrestore(&port->cad_lock, flags);
+	device_unregister(&dev->dev);
 
 	/* Save control registers */
 	port->ops->save_state(port, dev->state);
@@ -1019,7 +1085,7 @@ EXPORT_SYMBOL(parport_release);
 EXPORT_SYMBOL(parport_register_port);
 EXPORT_SYMBOL(parport_announce_port);
 EXPORT_SYMBOL(parport_remove_port);
-EXPORT_SYMBOL(parport_register_driver);
+EXPORT_SYMBOL(__parport_register_driver);
 EXPORT_SYMBOL(parport_unregister_driver);
 EXPORT_SYMBOL(parport_register_device);
 EXPORT_SYMBOL(parport_unregister_device);
diff --git a/include/linux/parport.h b/include/linux/parport.h
index c22f125..d7e4451 100644
--- a/include/linux/parport.h
+++ b/include/linux/parport.h
@@ -15,6 +15,7 @@
 #include <linux/semaphore.h>
 #include <asm/ptrace.h>
 #include <uapi/linux/parport.h>
+#include <linux/device.h>
 
 /* Define this later. */
 struct parport;
@@ -146,6 +147,7 @@ struct pardevice {
 	struct pardevice *next;
 	struct pardevice *prev;
 	struct parport_state *state;     /* saved status over preemption */
+	struct device dev;
 	wait_queue_head_t wait_q;
 	unsigned long int time;
 	unsigned long int timeslice;
@@ -156,6 +158,11 @@ struct pardevice {
 	void * sysctl_table;
 };
 
+static inline struct pardevice *to_pardevice(struct device *n)
+{
+	return container_of(n, struct pardevice, dev);
+}
+
 /* IEEE1284 information */
 
 /* IEEE1284 phases. These are exposed to userland through ppdev IOCTL
@@ -195,6 +202,7 @@ struct parport {
 				 * This may unfortulately be null if the
 				 * port has a legacy driver.
 				 */
+	struct device ddev;	/* to link with the bus */
 
 	struct parport *physport;
 				/* If this is a non-default mux
@@ -251,6 +259,7 @@ struct parport_driver {
 	const char *name;
 	void (*attach) (struct parport *);
 	void (*detach) (struct parport *);
+	struct device_driver driver;
 	struct list_head list;
 };
 
@@ -267,12 +276,22 @@ struct parport *parport_register_port(unsigned long base, int irq, int dma,
    determining the IEEE 1284.3 topology of the port and collecting
    DeviceIDs). */
 void parport_announce_port (struct parport *port);
+void parport_bus_exit(void);
+extern struct bus_type parport_bus_type;
 
+int parport_bus_init(void);
 /* Unregister a port. */
 extern void parport_remove_port(struct parport *port);
 
 /* Register a new high-level driver. */
-extern int parport_register_driver (struct parport_driver *);
+int __parport_register_driver(struct parport_driver *,
+			      struct module *, const char *);
+/*
+ * parport_register_driver must be a macro so that KBUILD_MODNAME can
+ * be expanded
+ */
+#define parport_register_driver(driver)             \
+	__parport_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
 
 /* Unregister a high-level driver. */
 extern void parport_unregister_driver (struct parport_driver *);
-- 
1.8.1.2


             reply	other threads:[~2015-04-10 14:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-10 14:30 Sudip Mukherjee [this message]
2015-04-10 14:49 ` [PATCH WIP] parport: add device model Dan Carpenter
2015-04-11  5:26   ` Sudip Mukherjee
2015-04-11  7:27     ` Greg KH
2015-04-11  8:11       ` Sudip Mukherjee
2015-04-13  8:27         ` Sudip Mukherjee
2015-04-13  8:43         ` Greg KH
2015-04-13 10:02           ` Sudip Mukherjee
2015-04-13 10:42             ` Greg KH
2015-04-13 10:38     ` Dan Carpenter
2015-04-10 18:24 ` Ondrej Zary
2015-04-11  5:05   ` Sudip Mukherjee
2015-04-11  9:24     ` Ondrej Zary

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1428676238-17141-1-git-send-email-sudipm.mukherjee@gmail.com \
    --to=sudipm.mukherjee@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.