linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API
@ 2011-05-19 21:33 Jim Cromie
  2011-05-19 21:33 ` [PATCH 02/23] reimplement alloc_chrdev_region with register_chrdev_ids Jim Cromie
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh

over on kernelnewbies, gregkh said:

     The chardev stuff is a mess, I keep meaning for years to clean it
     up.  Any proposals on a sane interface for this stuff is greatly
     appreciated.

this is a 1st step.

register_chrdev_ids() replaces and deprecates register_chrdev_region()
and alloc_chrdev_region() with a single function that works for both
dynamic and static major numbers.

Like alloc_chrdev_region(), 1st arg is a dev_t*, but its an in/out
parameter, and expects both major and minor to be preset, and thus the
separate minor arg is dropped.  If major == 0, a dynamic major is
reserved, saved into 1st arg, and thus available to caller afterwards.

[PATCH 01/23] add register_chrdev_ids() to char_dev.c, API
[PATCH 02/23] reimplement alloc_chrdev_region with
[PATCH 03/23] use register_chrdev_ids to replace
[PATCH 04/23] use register_chrdev_ids in drivers/tty/
[PATCH 05/23] use register_chrdev_ids in drivers/infiniband/
[PATCH 06/23] use register_chrdev_ids in drivers/media/
[PATCH 07/23] use register_chrdev_ids in drivers/s390/
[PATCH 08/23] use register_chrdev_ids in drivers/scsi/
[PATCH 09/23] use register_chrdev_ids in drivers/staging/

Ive held back the rest, no point in spamming.


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

* [PATCH 02/23] reimplement alloc_chrdev_region with register_chrdev_ids
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
@ 2011-05-19 21:33 ` Jim Cromie
  2011-05-19 21:33 ` [PATCH 03/23] use register_chrdev_ids to replace (register|alloc)_chrdev_region Jim Cromie
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Jim Cromie


Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 fs/char_dev.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index 9149b7c..c1bbb9c 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -234,12 +234,8 @@ int __deprecated
 alloc_chrdev_region(dev_t *dev, unsigned baseminor, unsigned count,
 		    const char *name)
 {
-	struct char_device_struct *cd;
-	cd = __register_chrdev_region(0, baseminor, count, name);
-	if (IS_ERR(cd))
-		return PTR_ERR(cd);
-	*dev = MKDEV(cd->major, cd->baseminor);
-	return 0;
+	*dev = MKDEV(0, baseminor);
+	return register_chrdev_ids(dev, count, name);
 }
 
 /**
-- 
1.7.4.4


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

* [PATCH 03/23] use register_chrdev_ids to replace (register|alloc)_chrdev_region
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
  2011-05-19 21:33 ` [PATCH 02/23] reimplement alloc_chrdev_region with register_chrdev_ids Jim Cromie
@ 2011-05-19 21:33 ` Jim Cromie
  2011-05-19 21:33 ` [PATCH 04/23] use register_chrdev_ids in drivers/tty/ Jim Cromie
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Jim Cromie

replace __deprecated (register|alloc)_chrdev_region api calls with
register_chrdev_ids.

This patch brought to you by coccinelle-spatch with the following
cocci-file.  It transforms ~40 callsites, which are broken out on
MAINTAINER boundaries, but misses those with MKDEV(X,Y) in the
param-list.

@ combo @	// if-major-alloc-else-register
dev_t devid;
identifier rc;
expression major, minor;
expression CT, DEVNAME;
@@

-	if (major) {
-		devid = MKDEV(major, 0);
-		rc = register_chrdev_region(devid, CT, DEVNAME);
-	} else {
-		rc = alloc_chrdev_region(&devid, minor, CT, DEVNAME);
-		major = MAJOR(devid);
-	}

+	devid = MKDEV(major, minor);
+	rc = register_chrdev_ids(&devid, CT, DEVNAME);

// general rules, implicitly depend on !combo

@ register_chrdev_region @
dev_t devid;			// typed simple var, not MKDEV(x,y)
expression ct, name;
@@

- register_chrdev_region(devid, ct, name)
+ register_chrdev_ids(&devid, ct, name)

@ alloc_chrdev_region @
dev_t devid;			// typed simple var, not MKDEV(x,y)
expression minor, ct, name;
@@

- alloc_chrdev_region(&devid, minor, ct, name)
+ register_chrdev_ids(&devid, ct, name)

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/char/pc8736x_gpio.c |   20 ++++++--------------
 drivers/char/scx200_gpio.c  |   12 ++++--------
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/char/pc8736x_gpio.c b/drivers/char/pc8736x_gpio.c
index b304ec0..cf01c99 100644
--- a/drivers/char/pc8736x_gpio.c
+++ b/drivers/char/pc8736x_gpio.c
@@ -254,7 +254,7 @@ static struct cdev pc8736x_gpio_cdev;
 static int __init pc8736x_gpio_init(void)
 {
 	int rc;
-	dev_t devid;
+	dev_t devid = MKDEV(major, 0);
 
 	pdev = platform_device_alloc(DEVNAME, 0);
 	if (!pdev)
@@ -300,24 +300,16 @@ static int __init pc8736x_gpio_init(void)
 			pc8736x_gpio_base);
 		goto undo_platform_dev_add;
 	}
-	dev_info(&pdev->dev, "GPIO ioport %x reserved\n", pc8736x_gpio_base);
-
-	if (major) {
-		devid = MKDEV(major, 0);
-		rc = register_chrdev_region(devid, PC8736X_GPIO_CT, DEVNAME);
-	} else {
-		rc = alloc_chrdev_region(&devid, 0, PC8736X_GPIO_CT, DEVNAME);
-		major = MAJOR(devid);
-	}
 
+	rc = register_chrdev_ids(&devid, PC8736X_GPIO_CT, DEVNAME);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "register-chrdev failed: %d\n", rc);
 		goto undo_request_region;
 	}
-	if (!major) {
-		major = rc;
-		dev_dbg(&pdev->dev, "got dynamic major %d\n", major);
-	}
+	major = MAJOR(devid);
+
+	dev_info(&pdev->dev, "GPIO ioport=%x, device-major=%d\n",
+		 pc8736x_gpio_base, major);
 
 	pc8736x_init_shadow();
 
diff --git a/drivers/char/scx200_gpio.c b/drivers/char/scx200_gpio.c
index 0bc135b..9b53a01 100644
--- a/drivers/char/scx200_gpio.c
+++ b/drivers/char/scx200_gpio.c
@@ -75,7 +75,7 @@ static struct cdev scx200_gpio_cdev;  /* use 1 cdev for all pins */
 static int __init scx200_gpio_init(void)
 {
 	int rc;
-	dev_t devid;
+	dev_t devid = MKDEV(major, 0);
 
 	if (!scx200_gpio_present()) {
 		printk(KERN_ERR DRVNAME ": no SCx200 gpio present\n");
@@ -94,17 +94,13 @@ static int __init scx200_gpio_init(void)
 	/* nsc_gpio uses dev_dbg(), so needs this */
 	scx200_gpio_ops.dev = &pdev->dev;
 
-	if (major) {
-		devid = MKDEV(major, 0);
-		rc = register_chrdev_region(devid, MAX_PINS, "scx200_gpio");
-	} else {
-		rc = alloc_chrdev_region(&devid, 0, MAX_PINS, "scx200_gpio");
-		major = MAJOR(devid);
-	}
+	rc = register_chrdev_ids(&devid, MAX_PINS, "scx200_gpio");
 	if (rc < 0) {
 		dev_err(&pdev->dev, "SCx200 chrdev_region err: %d\n", rc);
 		goto undo_platform_device_add;
 	}
+	major = MAJOR(devid);
+	dev_info(&pdev->dev, "GPIO device-major=%d\n", major);
 
 	cdev_init(&scx200_gpio_cdev, &scx200_gpio_fileops);
 	cdev_add(&scx200_gpio_cdev, devid, MAX_PINS);
-- 
1.7.4.4


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

* [PATCH 04/23] use register_chrdev_ids in drivers/tty/
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
  2011-05-19 21:33 ` [PATCH 02/23] reimplement alloc_chrdev_region with register_chrdev_ids Jim Cromie
  2011-05-19 21:33 ` [PATCH 03/23] use register_chrdev_ids to replace (register|alloc)_chrdev_region Jim Cromie
@ 2011-05-19 21:33 ` Jim Cromie
  2011-05-19 21:33 ` [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/ Jim Cromie
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Jim Cromie

cc: Greg Kroah-Hartman <gregkh@suse.de>

Convert register_chrdev_region(MKDEV(x,y)...) uses.
This patch brought to you by coccinelle-spatch.

Some manual post-work was needed; rules insert multiple dev_t
declarations if 2 different MKDEV()s are found.

@ rcr_md @
identifier f;
expression major, minor;
expression ct, name;
@@

 f(...) {
++	dev_t devt;			// ++ gives multiple inserts
++	devt = MKDEV(major,minor);	// fresh identifier may help

<+...
-	register_chrdev_region
+	register_chrdev_ids
	(
-	MKDEV(major,minor),
+	&devt,
	ct, name)
...+>
 }

@ all_md depends on rcr_md @	// where above changes made, also do
identifier f;
expression major, minor;
@@

	f(...) {
	dev_t devt;
	devt = MKDEV(major,minor);

<+...
-	MKDEV(major,minor)
+	devt
...+>
	}

 # Please enter the commit message for your
changes. Lines starting # with '#' will be ignored, and an empty
message aborts the commit.  # Not currently on any branch.  # Changes
to be committed: # (use "git reset HEAD^1 <file>..." to unstage) # #
modified: drivers/tty/pty.c # modified: drivers/tty/tty_io.c #
modified: drivers/tty/vt/vt.c # # Untracked files: # (use "git add
<file>..." to include in what will be committed) # # bar # boo # buc #
bum # buz # chrdev-inline.cocci # chrdev.cocci-expr #
chrdev.cocci-inline # foo # joe # joebob # junk # list # mkdev.cocci #
newconf # newconfig # spam.cocci

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/tty/pty.c    |    8 +++++---
 drivers/tty/tty_io.c |   23 +++++++++++++----------
 drivers/tty/vt/vt.c  |    8 +++++---
 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 2107747..07034f2 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -703,6 +703,8 @@ static struct file_operations ptmx_fops;
 
 static void __init unix98_pty_init(void)
 {
+	dev_t devt = MKDEV(TTYAUX_MAJOR, 2);
+
 	ptm_driver = alloc_tty_driver(NR_UNIX98_PTY_MAX);
 	if (!ptm_driver)
 		panic("Couldn't allocate Unix98 ptm driver");
@@ -757,10 +759,10 @@ static void __init unix98_pty_init(void)
 	ptmx_fops.open = ptmx_open;
 
 	cdev_init(&ptmx_cdev, &ptmx_fops);
-	if (cdev_add(&ptmx_cdev, MKDEV(TTYAUX_MAJOR, 2), 1) ||
-	    register_chrdev_region(MKDEV(TTYAUX_MAJOR, 2), 1, "/dev/ptmx") < 0)
+	if (cdev_add(&ptmx_cdev, devt, 1) ||
+	    register_chrdev_ids(&devt, 1, "/dev/ptmx") < 0)
 		panic("Couldn't register /dev/ptmx driver\n");
-	device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 2), NULL, "ptmx");
+	device_create(tty_class, NULL, devt, NULL, "ptmx");
 }
 
 #else
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index d7d50b4..efbb5e0 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3051,15 +3051,14 @@ int tty_register_driver(struct tty_driver *driver)
 	}
 
 	if (!driver->major) {
-		error = alloc_chrdev_region(&dev, driver->minor_start,
-						driver->num, driver->name);
+		error = register_chrdev_ids(&dev, driver->num, driver->name);
 		if (!error) {
 			driver->major = MAJOR(dev);
 			driver->minor_start = MINOR(dev);
 		}
 	} else {
 		dev = MKDEV(driver->major, driver->minor_start);
-		error = register_chrdev_region(dev, driver->num, driver->name);
+		error = register_chrdev_ids(&dev, driver->num, driver->name);
 	}
 	if (error < 0) {
 		kfree(p);
@@ -3294,18 +3293,22 @@ void console_sysfs_notify(void)
  */
 int __init tty_init(void)
 {
+	dev_t devt;
+
+	devt = MKDEV(TTYAUX_MAJOR, 0);
 	cdev_init(&tty_cdev, &tty_fops);
-	if (cdev_add(&tty_cdev, MKDEV(TTYAUX_MAJOR, 0), 1) ||
-	    register_chrdev_region(MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
+	if (cdev_add(&tty_cdev, devt, 1) ||
+	    register_chrdev_ids(&devt, 1, "/dev/tty") < 0)
 		panic("Couldn't register /dev/tty driver\n");
-	device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 0), NULL, "tty");
+	device_create(tty_class, NULL, devt, NULL, "tty");
 
+	devt = MKDEV(TTYAUX_MAJOR, 1);
 	cdev_init(&console_cdev, &console_fops);
-	if (cdev_add(&console_cdev, MKDEV(TTYAUX_MAJOR, 1), 1) ||
-	    register_chrdev_region(MKDEV(TTYAUX_MAJOR, 1), 1, "/dev/console") < 0)
+	if (cdev_add(&console_cdev, devt, 1) ||
+	    register_chrdev_ids(&devt, 1, "/dev/console") < 0)
 		panic("Couldn't register /dev/console driver\n");
-	consdev = device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 1), NULL,
-			      "console");
+	consdev = device_create(tty_class, NULL, devt, NULL, "console");
+
 	if (IS_ERR(consdev))
 		consdev = NULL;
 	else
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 4bea1ef..819b31c 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2973,11 +2973,13 @@ static DEVICE_ATTR(active, S_IRUGO, show_tty_active, NULL);
 
 int __init vty_init(const struct file_operations *console_fops)
 {
+	dev_t devt = MKDEV(TTY_MAJOR, 0);
+
 	cdev_init(&vc0_cdev, console_fops);
-	if (cdev_add(&vc0_cdev, MKDEV(TTY_MAJOR, 0), 1) ||
-	    register_chrdev_region(MKDEV(TTY_MAJOR, 0), 1, "/dev/vc/0") < 0)
+	if (cdev_add(&vc0_cdev, devt, 1) ||
+	    register_chrdev_ids(&devt, 1, "/dev/vc/0") < 0)
 		panic("Couldn't register /dev/tty0 driver\n");
-	tty0dev = device_create(tty_class, NULL, MKDEV(TTY_MAJOR, 0), NULL, "tty0");
+	tty0dev = device_create(tty_class, NULL, devt, NULL, "tty0");
 	if (IS_ERR(tty0dev))
 		tty0dev = NULL;
 	else
-- 
1.7.4.4


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

* [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
                   ` (2 preceding siblings ...)
  2011-05-19 21:33 ` [PATCH 04/23] use register_chrdev_ids in drivers/tty/ Jim Cromie
@ 2011-05-19 21:33 ` Jim Cromie
  2011-05-19 21:33 ` [PATCH 06/23] use register_chrdev_ids in drivers/media/ Jim Cromie
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, Jim Cromie, Hal Rosenstock, Roland Dreier, Sean Hefty,
	linux-rdma

Since new api passes dev_t*, hoist inline MKDEV out to local var
assignment, and replace other inline MKDEVs with new var.

This patch brought to you by coccinelle/spatch,
with some manual rework afterwards.

cc: Hal Rosenstock <hal.rosenstock@gmail.com>
cc: Roland Dreier <roland@kernel.org>
cc: Sean Hefty <sean.hefty@intel.com>
cc: linux-rdma@vger.kernel.org

@ rcr_md @
identifier f;
expression major, minor;
expression ct, name;
@@

	f(...) {
// ++ gives multiple inserts, needed for tty_io.c, fix up manually
// fresh identifier apparently also helps here
++	dev_t devt;
++	devt = MKDEV(major,minor);

<+...
-	register_chrdev_region
+	register_chrdev_ids
	(
-	MKDEV(major,minor),
+	&devt,
	ct, name)
...+>

}

@ all_md depends on rcr_md @	// where above changes made, also do
identifier f;
expression major, minor;
@@

	f(...) {
	dev_t devt;
	devt = MKDEV(major,minor);

<+...
-	MKDEV(major,minor)
+	devt
...+>
	}

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/infiniband/core/ucm.c                |    2 +-
 drivers/infiniband/core/user_mad.c           |    7 ++++---
 drivers/infiniband/core/uverbs_main.c        |    3 ++-
 drivers/infiniband/hw/ipath/ipath_file_ops.c |    2 +-
 drivers/infiniband/hw/qib/qib_file_ops.c     |    2 +-
 5 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c
index 08f948d..472a7b2 100644
--- a/drivers/infiniband/core/ucm.c
+++ b/drivers/infiniband/core/ucm.c
@@ -1244,7 +1244,7 @@ static int find_overflow_devnum(void)
 	int ret;
 
 	if (!overflow_maj) {
-		ret = alloc_chrdev_region(&overflow_maj, 0, IB_UCM_MAX_DEVICES,
+		ret = register_chrdev_ids(&overflow_maj, IB_UCM_MAX_DEVICES,
 					  "infiniband_cm");
 		if (ret) {
 			printk(KERN_ERR "ucm: couldn't register dynamic device number\n");
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index cd1996d..7352683 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -980,7 +980,8 @@ static int find_overflow_devnum(void)
 	int ret;
 
 	if (!overflow_maj) {
-		ret = alloc_chrdev_region(&overflow_maj, 0, IB_UMAD_MAX_PORTS * 2,
+		ret = register_chrdev_ids(&overflow_maj,
+					  IB_UMAD_MAX_PORTS * 2,
 					  "infiniband_mad");
 		if (ret) {
 			printk(KERN_ERR "user_mad: couldn't register dynamic device number\n");
@@ -1180,8 +1181,8 @@ static int __init ib_umad_init(void)
 {
 	int ret;
 
-	ret = register_chrdev_region(base_dev, IB_UMAD_MAX_PORTS * 2,
-				     "infiniband_mad");
+	ret = register_chrdev_ids(&base_dev, IB_UMAD_MAX_PORTS * 2,
+				  "infiniband_mad");
 	if (ret) {
 		printk(KERN_ERR "user_mad: couldn't register device number\n");
 		goto out;
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index ec83e9f..2f92115 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -711,7 +711,8 @@ static int find_overflow_devnum(void)
 	int ret;
 
 	if (!overflow_maj) {
-		ret = alloc_chrdev_region(&overflow_maj, 0, IB_UVERBS_MAX_DEVICES,
+		ret = register_chrdev_ids(&overflow_maj,
+					  IB_UVERBS_MAX_DEVICES,
 					  "infiniband_verbs");
 		if (ret) {
 			printk(KERN_ERR "user_verbs: couldn't register dynamic device number\n");
diff --git a/drivers/infiniband/hw/ipath/ipath_file_ops.c b/drivers/infiniband/hw/ipath/ipath_file_ops.c
index ee79a2d..04abb61 100644
--- a/drivers/infiniband/hw/ipath/ipath_file_ops.c
+++ b/drivers/infiniband/hw/ipath/ipath_file_ops.c
@@ -2519,7 +2519,7 @@ static int user_init(void)
 {
 	int ret;
 
-	ret = register_chrdev_region(dev, IPATH_NMINORS, IPATH_DRV_NAME);
+	ret = register_chrdev_ids(&dev, IPATH_NMINORS, IPATH_DRV_NAME);
 	if (ret < 0) {
 		printk(KERN_ERR IPATH_DRV_NAME ": Could not register "
 		       "chrdev region (err %d)\n", -ret);
diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index 406fca5..a5ecea2 100644
--- a/drivers/infiniband/hw/qib/qib_file_ops.c
+++ b/drivers/infiniband/hw/qib/qib_file_ops.c
@@ -2238,7 +2238,7 @@ int __init qib_dev_init(void)
 {
 	int ret;
 
-	ret = alloc_chrdev_region(&qib_dev, 0, QIB_NMINORS, QIB_DRV_NAME);
+	ret = register_chrdev_ids(&qib_dev, QIB_NMINORS, QIB_DRV_NAME);
 	if (ret < 0) {
 		printk(KERN_ERR QIB_DRV_NAME ": Could not allocate "
 		       "chrdev region (err %d)\n", -ret);
-- 
1.7.4.4


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

* [PATCH 06/23] use register_chrdev_ids in drivers/media/
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
                   ` (3 preceding siblings ...)
  2011-05-19 21:33 ` [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/ Jim Cromie
@ 2011-05-19 21:33 ` Jim Cromie
  2011-05-21 12:48   ` Mauro Carvalho Chehab
  2011-05-19 21:33 ` [PATCH 07/23] use register_chrdev_ids in drivers/s390/ Jim Cromie
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Jim Cromie, Mauro Carvalho Chehab, linux-media

Since new api passes dev_t*, hoist inline MKDEV out to local var
assignment, and replace other inline MKDEVs with new var.

This and 2 subsequent patches brought to you by coccinelle/spatch

cc: Mauro Carvalho Chehab <mchehab@infradead.org>
cc: linux-media@vger.kernel.org

@ rcr_md @
identifier f;
expression major, minor;
expression ct, name;
@@

	f(...) {
// ++ gives multiple inserts, needed for tty_io.c, fix up manually
// fresh identifier apparently also helps here
++	dev_t devt;
++	devt = MKDEV(major,minor);

<+...
-	register_chrdev_region
+	register_chrdev_ids
	(
-	MKDEV(major,minor),
+	&devt,
	ct, name)
...+>

}

@ all_md depends on rcr_md @	// where above changes made, also do
identifier f;
expression major, minor;
@@

	f(...) {
	dev_t devt;
	devt = MKDEV(major,minor);

<+...
-	MKDEV(major,minor)
+	devt
...+>
	}

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/media/dvb/dvb-core/dvbdev.c |    6 ++++--
 drivers/media/media-devnode.c       |    3 +--
 drivers/media/rc/lirc_dev.c         |    4 ++--
 drivers/media/video/v4l2-dev.c      |    2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c
index f732877..225b9d5 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -464,8 +464,10 @@ static int __init init_dvbdev(void)
 	int retval;
 	dev_t dev = MKDEV(DVB_MAJOR, 0);
 
-	if ((retval = register_chrdev_region(dev, MAX_DVB_MINORS, "DVB")) != 0) {
-		printk(KERN_ERR "dvb-core: unable to get major %d\n", DVB_MAJOR);
+	retval = register_chrdev_ids(&dev, MAX_DVB_MINORS, "DVB");
+	if (retval != 0) {
+		printk(KERN_ERR "dvb-core: unable to get major %d\n",
+		       DVB_MAJOR);
 		return retval;
 	}
 
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index af5263c..e45f322 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -289,8 +289,7 @@ static int __init media_devnode_init(void)
 	int ret;
 
 	printk(KERN_INFO "Linux media interface: v0.10\n");
-	ret = alloc_chrdev_region(&media_dev_t, 0, MEDIA_NUM_DEVICES,
-				  MEDIA_NAME);
+	ret = register_chrdev_ids(&media_dev_t, MEDIA_NUM_DEVICES, MEDIA_NAME);
 	if (ret < 0) {
 		printk(KERN_WARNING "media: unable to allocate major\n");
 		return ret;
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index fd237ab..28f2968 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -780,11 +780,11 @@ static int __init lirc_dev_init(void)
 		goto error;
 	}
 
-	retval = alloc_chrdev_region(&lirc_base_dev, 0, MAX_IRCTL_DEVICES,
+	retval = register_chrdev_ids(&lirc_base_dev, MAX_IRCTL_DEVICES,
 				     IRCTL_DEV_NAME);
 	if (retval) {
 		class_destroy(lirc_class);
-		printk(KERN_ERR "lirc_dev: alloc_chrdev_region failed\n");
+		printk(KERN_ERR "lirc_dev: register_chrdev_ids() failed\n");
 		goto error;
 	}
 
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 6dc7196..9ae24e2 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -761,7 +761,7 @@ static int __init videodev_init(void)
 	int ret;
 
 	printk(KERN_INFO "Linux video capture interface: v2.00\n");
-	ret = register_chrdev_region(dev, VIDEO_NUM_DEVICES, VIDEO_NAME);
+	ret = register_chrdev_ids(&dev, VIDEO_NUM_DEVICES, VIDEO_NAME);
 	if (ret < 0) {
 		printk(KERN_WARNING "videodev: unable to get major %d\n",
 				VIDEO_MAJOR);
-- 
1.7.4.4


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

* [PATCH 07/23] use register_chrdev_ids in drivers/s390/
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
                   ` (4 preceding siblings ...)
  2011-05-19 21:33 ` [PATCH 06/23] use register_chrdev_ids in drivers/media/ Jim Cromie
@ 2011-05-19 21:33 ` Jim Cromie
  2011-05-19 21:33 ` [PATCH 08/23] use register_chrdev_ids in drivers/scsi/ Jim Cromie
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, Jim Cromie, Heiko Carstens, Martin Schwidefsky,
	linux-s390, linux390

cc: Heiko Carstens <heiko.carstens@de.ibm.com>
cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
cc: linux-s390@vger.kernel.org
cc: linux390@de.ibm.com
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/s390/char/tape_char.c |    2 +-
 drivers/s390/char/vmlogrdr.c  |    2 +-
 drivers/s390/char/vmur.c      |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/s390/char/tape_char.c b/drivers/s390/char/tape_char.c
index 87cd0ab..e321435 100644
--- a/drivers/s390/char/tape_char.c
+++ b/drivers/s390/char/tape_char.c
@@ -494,7 +494,7 @@ tapechar_init (void)
 {
 	dev_t	dev;
 
-	if (alloc_chrdev_region(&dev, 0, 256, "tape") != 0)
+	if (register_chrdev_ids(&dev, 256, "tape") != 0)
 		return -1;
 
 	tapechar_major = MAJOR(dev);
diff --git a/drivers/s390/char/vmlogrdr.c b/drivers/s390/char/vmlogrdr.c
index c837d74..15eeab7 100644
--- a/drivers/s390/char/vmlogrdr.c
+++ b/drivers/s390/char/vmlogrdr.c
@@ -866,7 +866,7 @@ static int __init vmlogrdr_init(void)
 
         recording_class_AB = vmlogrdr_get_recording_class_AB();
 
-	rc = alloc_chrdev_region(&dev, 0, MAXMINOR, "vmlogrdr");
+	rc = register_chrdev_ids(&dev, MAXMINOR, "vmlogrdr");
 	if (rc)
 		return rc;
 	vmlogrdr_major = MAJOR(dev);
diff --git a/drivers/s390/char/vmur.c b/drivers/s390/char/vmur.c
index f6b00c3..e752c84 100644
--- a/drivers/s390/char/vmur.c
+++ b/drivers/s390/char/vmur.c
@@ -1037,9 +1037,9 @@ static int __init ur_init(void)
 	if (rc)
 		goto fail_class_destroy;
 
-	rc = alloc_chrdev_region(&dev, 0, NUM_MINORS, "vmur");
+	rc = register_chrdev_ids(&dev, NUM_MINORS, "vmur");
 	if (rc) {
-		pr_err("Kernel function alloc_chrdev_region failed with "
+		pr_err("register_chrdev_ids() failed with "
 		       "error code %d\n", rc);
 		goto fail_unregister_driver;
 	}
-- 
1.7.4.4


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

* [PATCH 08/23] use register_chrdev_ids in drivers/scsi/
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
                   ` (5 preceding siblings ...)
  2011-05-19 21:33 ` [PATCH 07/23] use register_chrdev_ids in drivers/s390/ Jim Cromie
@ 2011-05-19 21:33 ` Jim Cromie
  2011-05-20 15:42   ` Boaz Harrosh
  2011-05-19 21:33 ` [PATCH 09/23] use register_chrdev_ids in drivers/staging/ Jim Cromie
  2011-05-19 22:44 ` [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Greg KH
  8 siblings, 1 reply; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, Jim Cromie, Doug Gilbert, linux-scsi, Benny Halevy,
	Boaz Harrosh, osd-dev, Anil Ravindranath

cc: Doug Gilbert <dgilbert@interlog.com>
cc: linux-scsi@vger.kernel.org
cc: Benny Halevy <bhalevy@panasas.com>
cc: Boaz Harrosh <bharrosh@panasas.com>
cc: osd-dev@open-osd.org
cc: Anil Ravindranath <anil_ravindranath@pmc-sierra.com>
cc: linux-scsi@vger.kernel.org
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/scsi/osd/osd_uld.c |    6 +++---
 drivers/scsi/pmcraid.c     |    3 +--
 drivers/scsi/sg.c          |    6 +++---
 drivers/scsi/st.c          |    7 +++----
 4 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index b31a8e3..18dcd38 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -528,6 +528,7 @@ static struct scsi_driver osd_driver = {
 static int __init osd_uld_init(void)
 {
 	int err;
+	dev_t devt = MKDEV(SCSI_OSD_MAJOR, 0);
 
 	err = class_register(&osd_uld_class);
 	if (err) {
@@ -535,8 +536,7 @@ static int __init osd_uld_init(void)
 		return err;
 	}
 
-	err = register_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0),
-				     SCSI_OSD_MAX_MINOR, osd_name);
+	err = register_chrdev_ids(&devt, SCSI_OSD_MAX_MINOR, osd_name);
 	if (err) {
 		OSD_ERR("Unable to register major %d for osd ULD => %d\n",
 			SCSI_OSD_MAJOR, err);
@@ -553,7 +553,7 @@ static int __init osd_uld_init(void)
 	return 0;
 
 err_out_chrdev:
-	unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR);
+	unregister_chrdev_region(devt, SCSI_OSD_MAX_MINOR);
 err_out:
 	class_unregister(&osd_uld_class);
 	return err;
diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 7f636b1..61a2207 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -6100,8 +6100,7 @@ static int __init pmcraid_init(void)
 			 PMCRAID_DRIVER_NAME,
 			 PMCRAID_DRIVER_VERSION, PMCRAID_DRIVER_DATE);
 
-	error = alloc_chrdev_region(&dev, 0,
-				    PMCRAID_MAX_ADAPTERS,
+	error = register_chrdev_ids(&dev, PMCRAID_MAX_ADAPTERS,
 				    PMCRAID_DEVFILE);
 
 	if (error) {
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 909ed9e..e741988 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1575,6 +1575,7 @@ static int __init
 init_sg(void)
 {
 	int rc;
+	dev_t devt = MKDEV(SCSI_GENERIC_MAJOR, 0);
 
 	if (scatter_elem_sz < PAGE_SIZE) {
 		scatter_elem_sz = PAGE_SIZE;
@@ -1585,8 +1586,7 @@ init_sg(void)
 	else
 		def_reserved_size = sg_big_buff;
 
-	rc = register_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0), 
-				    SG_MAX_DEVS, "sg");
+	rc = register_chrdev_ids(&devt, SG_MAX_DEVS, "sg");
 	if (rc)
 		return rc;
         sg_sysfs_class = class_create(THIS_MODULE, "scsi_generic");
@@ -1604,7 +1604,7 @@ init_sg(void)
 	}
 	class_destroy(sg_sysfs_class);
 err_out:
-	unregister_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0), SG_MAX_DEVS);
+	unregister_chrdev_region(devt, SG_MAX_DEVS);
 	return rc;
 }
 
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 1871b8a..5cce227 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -4260,6 +4260,7 @@ static void scsi_tape_release(struct kref *kref)
 static int __init init_st(void)
 {
 	int err;
+	dev_t devt = MKDEV(SCSI_TAPE_MAJOR, 0);
 
 	validate_options();
 
@@ -4272,8 +4273,7 @@ static int __init init_st(void)
 		return PTR_ERR(st_sysfs_class);
 	}
 
-	err = register_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
-				     ST_MAX_TAPE_ENTRIES, "st");
+	err = register_chrdev_ids(&devt, ST_MAX_TAPE_ENTRIES, "st");
 	if (err) {
 		printk(KERN_ERR "Unable to get major %d for SCSI tapes\n",
 		       SCSI_TAPE_MAJOR);
@@ -4293,8 +4293,7 @@ static int __init init_st(void)
 err_scsidrv:
 	scsi_unregister_driver(&st_template.gendrv);
 err_chrdev:
-	unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
-				 ST_MAX_TAPE_ENTRIES);
+	unregister_chrdev_region(devt, ST_MAX_TAPE_ENTRIES);
 err_class:
 	class_destroy(st_sysfs_class);
 	return err;
-- 
1.7.4.4


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

* [PATCH 09/23] use register_chrdev_ids in drivers/staging/
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
                   ` (6 preceding siblings ...)
  2011-05-19 21:33 ` [PATCH 08/23] use register_chrdev_ids in drivers/scsi/ Jim Cromie
@ 2011-05-19 21:33 ` Jim Cromie
  2011-05-19 22:44 ` [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Greg KH
  8 siblings, 0 replies; 17+ messages in thread
From: Jim Cromie @ 2011-05-19 21:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Jim Cromie, devel

cc: Greg Kroah-Hartman <gregkh@suse.de>
cc: devel@driverdev.osuosl.org
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/staging/comedi/comedi_fops.c             |   15 ++++++---------
 drivers/staging/cs5535_gpio/cs5535_gpio.c        |   10 +---------
 drivers/staging/iio/industrialio-core.c          |    2 +-
 drivers/staging/tidspbridge/rmgr/drv_interface.c |    2 +-
 4 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index e7e72b8..55f831f 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1960,6 +1960,7 @@ static int __init comedi_init(void)
 {
 	int i;
 	int retval;
+	dev_t devt = MKDEV(COMEDI_MAJOR, 0);
 
 	printk(KERN_INFO "comedi: version " COMEDI_RELEASE
 	       " - http://www.comedi.org\n");
@@ -1983,24 +1984,21 @@ static int __init comedi_init(void)
 	memset(comedi_file_info_table, 0,
 	       sizeof(struct comedi_device_file_info *) * COMEDI_NUM_MINORS);
 
-	retval = register_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
-					COMEDI_NUM_MINORS, "comedi");
+	retval = register_chrdev_ids(&devt, COMEDI_NUM_MINORS, "comedi");
 	if (retval)
 		return -EIO;
 	cdev_init(&comedi_cdev, &comedi_fops);
 	comedi_cdev.owner = THIS_MODULE;
 	kobject_set_name(&comedi_cdev.kobj, "comedi");
-	if (cdev_add(&comedi_cdev, MKDEV(COMEDI_MAJOR, 0), COMEDI_NUM_MINORS)) {
-		unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
-					 COMEDI_NUM_MINORS);
+	if (cdev_add(&comedi_cdev, devt, COMEDI_NUM_MINORS)) {
+		unregister_chrdev_region(devt, COMEDI_NUM_MINORS);
 		return -EIO;
 	}
 	comedi_class = class_create(THIS_MODULE, "comedi");
 	if (IS_ERR(comedi_class)) {
 		printk(KERN_ERR "comedi: failed to create class");
 		cdev_del(&comedi_cdev);
-		unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
-					 COMEDI_NUM_MINORS);
+		unregister_chrdev_region(devt, COMEDI_NUM_MINORS);
 		return PTR_ERR(comedi_class);
 	}
 
@@ -2014,8 +2012,7 @@ static int __init comedi_init(void)
 		if (minor < 0) {
 			comedi_cleanup_legacy_minors();
 			cdev_del(&comedi_cdev);
-			unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
-						 COMEDI_NUM_MINORS);
+			unregister_chrdev_region(devt, COMEDI_NUM_MINORS);
 			return minor;
 		}
 	}
diff --git a/drivers/staging/cs5535_gpio/cs5535_gpio.c b/drivers/staging/cs5535_gpio/cs5535_gpio.c
index b25f9d1..539a08a 100644
--- a/drivers/staging/cs5535_gpio/cs5535_gpio.c
+++ b/drivers/staging/cs5535_gpio/cs5535_gpio.c
@@ -223,15 +223,7 @@ static int __init cs5535_gpio_init(void)
 		return -ENODEV;
 	}
 
-	if (major) {
-		dev_id = MKDEV(major, 0);
-		retval = register_chrdev_region(dev_id, CS5535_GPIO_COUNT,
-						NAME);
-	} else {
-		retval = alloc_chrdev_region(&dev_id, 0, CS5535_GPIO_COUNT,
-					     NAME);
-		major = MAJOR(dev_id);
-	}
+	retval = register_chrdev_ids(&dev_id, CS5535_GPIO_COUNT, NAME);
 
 	if (retval) {
 		release_region(gpio_base, CS5535_GPIO_SIZE);
diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
index 1795ee1..1ef7198 100644
--- a/drivers/staging/iio/industrialio-core.c
+++ b/drivers/staging/iio/industrialio-core.c
@@ -443,7 +443,7 @@ static int __init iio_dev_init(void)
 {
 	int err;
 
-	err = alloc_chrdev_region(&iio_devt, 0, IIO_DEV_MAX, "iio");
+	err = register_chrdev_ids(&iio_devt, IIO_DEV_MAX, "iio");
 	if (err < 0)
 		printk(KERN_ERR "%s: failed to allocate char dev region\n",
 		       __FILE__);
diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c b/drivers/staging/tidspbridge/rmgr/drv_interface.c
index c43c7e3..ec7773f 100644
--- a/drivers/staging/tidspbridge/rmgr/drv_interface.c
+++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c
@@ -349,7 +349,7 @@ static int __devinit omap34_xx_bridge_probe(struct platform_device *pdev)
 		goto err1;
 
 	/* use 2.6 device model */
-	err = alloc_chrdev_region(&dev, 0, 1, driver_name);
+	err = register_chrdev_ids(&dev, 1, driver_name);
 	if (err) {
 		pr_err("%s: Can't get major %d\n", __func__, driver_major);
 		goto err1;
-- 
1.7.4.4


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

* Re: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API
  2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
                   ` (7 preceding siblings ...)
  2011-05-19 21:33 ` [PATCH 09/23] use register_chrdev_ids in drivers/staging/ Jim Cromie
@ 2011-05-19 22:44 ` Greg KH
  2011-05-21  5:15   ` Jim Cromie
  8 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2011-05-19 22:44 UTC (permalink / raw)
  To: Jim Cromie; +Cc: linux-kernel

On Thu, May 19, 2011 at 03:33:03PM -0600, Jim Cromie wrote:
> over on kernelnewbies, gregkh said:
> 
>      The chardev stuff is a mess, I keep meaning for years to clean it
>      up.  Any proposals on a sane interface for this stuff is greatly
>      appreciated.
> 
> this is a 1st step.
> 
> register_chrdev_ids() replaces and deprecates register_chrdev_region()
> and alloc_chrdev_region() with a single function that works for both
> dynamic and static major numbers.
> 
> Like alloc_chrdev_region(), 1st arg is a dev_t*, but its an in/out
> parameter, and expects both major and minor to be preset, and thus the
> separate minor arg is dropped.  If major == 0, a dynamic major is
> reserved, saved into 1st arg, and thus available to caller afterwards.
> 
> [PATCH 01/23] add register_chrdev_ids() to char_dev.c, API
> [PATCH 02/23] reimplement alloc_chrdev_region with
> [PATCH 03/23] use register_chrdev_ids to replace
> [PATCH 04/23] use register_chrdev_ids in drivers/tty/
> [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/
> [PATCH 06/23] use register_chrdev_ids in drivers/media/
> [PATCH 07/23] use register_chrdev_ids in drivers/s390/
> [PATCH 08/23] use register_chrdev_ids in drivers/scsi/
> [PATCH 09/23] use register_chrdev_ids in drivers/staging/
> 
> Ive held back the rest, no point in spamming.

It's a nice first step, but that's the easy part, what is your 2nd
through 4th one going to be?  :)

I'd also like to sanatize the function namespace a bit as well, how
about chrdev_register_ids() instead?

Ideally, we could drop down to a single register/unregister pair of
functions, that are easy to use and understand.  Do you think you can
get there with this intermediate step or do you want to step back and
rethink this?

thanks,

greg k-h

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

* Re: [PATCH 08/23] use register_chrdev_ids in drivers/scsi/
  2011-05-19 21:33 ` [PATCH 08/23] use register_chrdev_ids in drivers/scsi/ Jim Cromie
@ 2011-05-20 15:42   ` Boaz Harrosh
  2011-05-21  4:21     ` Jim Cromie
  0 siblings, 1 reply; 17+ messages in thread
From: Boaz Harrosh @ 2011-05-20 15:42 UTC (permalink / raw)
  To: Jim Cromie
  Cc: linux-kernel, gregkh, Doug Gilbert, linux-scsi, Benny Halevy,
	osd-dev, Anil Ravindranath

On 05/20/2011 12:33 AM, Jim Cromie wrote:
> cc: Doug Gilbert <dgilbert@interlog.com>
> cc: linux-scsi@vger.kernel.org
> cc: Benny Halevy <bhalevy@panasas.com>
> cc: Boaz Harrosh <bharrosh@panasas.com>
> cc: osd-dev@open-osd.org
> cc: Anil Ravindranath <anil_ravindranath@pmc-sierra.com>
> cc: linux-scsi@vger.kernel.org
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
>  drivers/scsi/osd/osd_uld.c |    6 +++---
>  drivers/scsi/pmcraid.c     |    3 +--
>  drivers/scsi/sg.c          |    6 +++---
>  drivers/scsi/st.c          |    7 +++----
>  4 files changed, 10 insertions(+), 12 deletions(-)

Do you have a git tree with all these that I can pull
and test?

Thanks
Boaz

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

* Re: [PATCH 08/23] use register_chrdev_ids in drivers/scsi/
  2011-05-20 15:42   ` Boaz Harrosh
@ 2011-05-21  4:21     ` Jim Cromie
  2011-05-21  7:59       ` Boaz Harrosh
  0 siblings, 1 reply; 17+ messages in thread
From: Jim Cromie @ 2011-05-21  4:21 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: linux-kernel, gregkh, Doug Gilbert, linux-scsi, Benny Halevy,
	osd-dev, Anil Ravindranath

On Fri, May 20, 2011 at 9:42 AM, Boaz Harrosh <bharrosh@panasas.com> wrote:
> On 05/20/2011 12:33 AM, Jim Cromie wrote:
>> cc: Doug Gilbert <dgilbert@interlog.com>
>> cc: linux-scsi@vger.kernel.org
>> cc: Benny Halevy <bhalevy@panasas.com>
>> cc: Boaz Harrosh <bharrosh@panasas.com>
>> cc: osd-dev@open-osd.org
>> cc: Anil Ravindranath <anil_ravindranath@pmc-sierra.com>
>> cc: linux-scsi@vger.kernel.org
>> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
>> ---
>>  drivers/scsi/osd/osd_uld.c |    6 +++---
>>  drivers/scsi/pmcraid.c     |    3 +--
>>  drivers/scsi/sg.c          |    6 +++---
>>  drivers/scsi/st.c          |    7 +++----
>>  4 files changed, 10 insertions(+), 12 deletions(-)
>
> Do you have a git tree with all these that I can pull
> and test?
>

I do, its at git://github.com/jimc/linux-2.6.git  chrdev-pub1 branch

For you, I think there are 3 patches of interest
1 - adds new call, deprecates old.
if you build with this applied and CONFIG_ENABLE_WARN_DEPRECATED=y
you should get a deprecated warning
2 - reimplements alloc_chardev_region() with register_chardev_ids()
your driver may be using register_chardev_region(), if so this is uninteresting.
3 - patch that adapts your scsi parts.

> Thanks
> Boaz
>

no, thank you
Jim Cromie

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

* Re: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API
  2011-05-19 22:44 ` [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Greg KH
@ 2011-05-21  5:15   ` Jim Cromie
  2011-05-21 21:14     ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: Jim Cromie @ 2011-05-21  5:15 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

On Thu, May 19, 2011 at 4:44 PM, Greg KH <gregkh@suse.de> wrote:
> On Thu, May 19, 2011 at 03:33:03PM -0600, Jim Cromie wrote:
>> over on kernelnewbies, gregkh said:
>>
>>      The chardev stuff is a mess, I keep meaning for years to clean it
>>      up.  Any proposals on a sane interface for this stuff is greatly
>>      appreciated.
>>
>> this is a 1st step.
>>
>> register_chrdev_ids() replaces and deprecates register_chrdev_region()
>> and alloc_chrdev_region() with a single function that works for both
>> dynamic and static major numbers.
>>
>> Like alloc_chrdev_region(), 1st arg is a dev_t*, but its an in/out
>> parameter, and expects both major and minor to be preset, and thus the
>> separate minor arg is dropped.  If major == 0, a dynamic major is
>> reserved, saved into 1st arg, and thus available to caller afterwards.
>>
>> [PATCH 01/23] add register_chrdev_ids() to char_dev.c, API
>> [PATCH 02/23] reimplement alloc_chrdev_region with
>> [PATCH 03/23] use register_chrdev_ids to replace
>> [PATCH 04/23] use register_chrdev_ids in drivers/tty/
>> [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/
>> [PATCH 06/23] use register_chrdev_ids in drivers/media/
>> [PATCH 07/23] use register_chrdev_ids in drivers/s390/
>> [PATCH 08/23] use register_chrdev_ids in drivers/scsi/
>> [PATCH 09/23] use register_chrdev_ids in drivers/staging/
>>
>> Ive held back the rest, no point in spamming.
>
> It's a nice first step, but that's the easy part, what is your 2nd
> through 4th one going to be?  :)
>
> I'd also like to sanatize the function namespace a bit as well, how
> about chrdev_register_ids() instead?

that seems sensible, modern.
also have register_chrdev(), which I presume should also be fixed.

> Ideally, we could drop down to a single register/unregister pair of
> functions, that are easy to use and understand.

__register_chrdev() does more stuff, mainly around cdevs, fops.
If fops was passed as NULL, we just do the __register_chardev_region()
and return early, skipping the cdev_alloc() and everything afterwards,
thus yielding register_chrdev_ids() behavior.

> Do you think you can
> get there with this intermediate step or do you want to step back and
> rethink this?

hmm.  If above is right, theres no need for the new api fn I added,
and probably should also drop the __ on both (un)?register_chardev.
So thats step 2 :)  Any ideas for 3 ?

btw, I think theres a major/minor error in the linuxdoc for the count
param in some of these register-* functions.  I'll take a closer look,
and send a patch RSN if needed, even if fn is going away later.


> thanks,
>
> greg k-h
>

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

* Re: [PATCH 08/23] use register_chrdev_ids in drivers/scsi/
  2011-05-21  4:21     ` Jim Cromie
@ 2011-05-21  7:59       ` Boaz Harrosh
  0 siblings, 0 replies; 17+ messages in thread
From: Boaz Harrosh @ 2011-05-21  7:59 UTC (permalink / raw)
  To: Jim Cromie
  Cc: linux-kernel, gregkh, Doug Gilbert, linux-scsi, Benny Halevy,
	osd-dev, Anil Ravindranath

On 05/21/2011 07:21 AM, Jim Cromie wrote:
> On Fri, May 20, 2011 at 9:42 AM, Boaz Harrosh <bharrosh@panasas.com> wrote:
>> On 05/20/2011 12:33 AM, Jim Cromie wrote:
>>> cc: Doug Gilbert <dgilbert@interlog.com>
>>> cc: linux-scsi@vger.kernel.org
>>> cc: Benny Halevy <bhalevy@panasas.com>
>>> cc: Boaz Harrosh <bharrosh@panasas.com>
>>> cc: osd-dev@open-osd.org
>>> cc: Anil Ravindranath <anil_ravindranath@pmc-sierra.com>
>>> cc: linux-scsi@vger.kernel.org
>>> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
>>> ---
>>>  drivers/scsi/osd/osd_uld.c |    6 +++---
>>>  drivers/scsi/pmcraid.c     |    3 +--
>>>  drivers/scsi/sg.c          |    6 +++---
>>>  drivers/scsi/st.c          |    7 +++----
>>>  4 files changed, 10 insertions(+), 12 deletions(-)
>>
>> Do you have a git tree with all these that I can pull
>> and test?
>>
> 
> I do, its at git://github.com/jimc/linux-2.6.git  chrdev-pub1 branch
> 
> For you, I think there are 3 patches of interest
> 1 - adds new call, deprecates old.
> if you build with this applied and CONFIG_ENABLE_WARN_DEPRECATED=y
> you should get a deprecated warning
> 2 - reimplements alloc_chardev_region() with register_chardev_ids()
> your driver may be using register_chardev_region(), if so this is uninteresting.
> 3 - patch that adapts your scsi parts.
> 
>> Thanks
>> Boaz
>>
> 
> no, thank you
> Jim Cromie

Grate many thanks. I will test it next week and confirm.

Boaz

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

* Re: [PATCH 06/23] use register_chrdev_ids in drivers/media/
  2011-05-19 21:33 ` [PATCH 06/23] use register_chrdev_ids in drivers/media/ Jim Cromie
@ 2011-05-21 12:48   ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2011-05-21 12:48 UTC (permalink / raw)
  To: Jim Cromie; +Cc: linux-kernel, gregkh, linux-media

Em 19-05-2011 18:33, Jim Cromie escreveu:
> Since new api passes dev_t*, hoist inline MKDEV out to local var
> assignment, and replace other inline MKDEVs with new var.

While I don't see the need for this change, I'm ok with that.

Please notice that it is not clear if you expect me to apply the patch or not,
as you simply c/c me and Greg on it.

So I'm assuming that somebody else will be applying it. In this case:

Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>

> 
> This and 2 subsequent patches brought to you by coccinelle/spatch
> 
> cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> cc: linux-media@vger.kernel.org
> 
> @ rcr_md @
> identifier f;
> expression major, minor;
> expression ct, name;
> @@
> 
> 	f(...) {
> // ++ gives multiple inserts, needed for tty_io.c, fix up manually
> // fresh identifier apparently also helps here
> ++	dev_t devt;
> ++	devt = MKDEV(major,minor);
> 
> <+...
> -	register_chrdev_region
> +	register_chrdev_ids
> 	(
> -	MKDEV(major,minor),
> +	&devt,
> 	ct, name)
> ...+>
> 
> }
> 
> @ all_md depends on rcr_md @	// where above changes made, also do
> identifier f;
> expression major, minor;
> @@
> 
> 	f(...) {
> 	dev_t devt;
> 	devt = MKDEV(major,minor);
> 
> <+...
> -	MKDEV(major,minor)
> +	devt
> ...+>
> 	}
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
>  drivers/media/dvb/dvb-core/dvbdev.c |    6 ++++--
>  drivers/media/media-devnode.c       |    3 +--
>  drivers/media/rc/lirc_dev.c         |    4 ++--
>  drivers/media/video/v4l2-dev.c      |    2 +-
>  4 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c
> index f732877..225b9d5 100644
> --- a/drivers/media/dvb/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb/dvb-core/dvbdev.c
> @@ -464,8 +464,10 @@ static int __init init_dvbdev(void)
>  	int retval;
>  	dev_t dev = MKDEV(DVB_MAJOR, 0);
>  
> -	if ((retval = register_chrdev_region(dev, MAX_DVB_MINORS, "DVB")) != 0) {
> -		printk(KERN_ERR "dvb-core: unable to get major %d\n", DVB_MAJOR);
> +	retval = register_chrdev_ids(&dev, MAX_DVB_MINORS, "DVB");
> +	if (retval != 0) {
> +		printk(KERN_ERR "dvb-core: unable to get major %d\n",
> +		       DVB_MAJOR);
>  		return retval;
>  	}
>  
> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> index af5263c..e45f322 100644
> --- a/drivers/media/media-devnode.c
> +++ b/drivers/media/media-devnode.c
> @@ -289,8 +289,7 @@ static int __init media_devnode_init(void)
>  	int ret;
>  
>  	printk(KERN_INFO "Linux media interface: v0.10\n");
> -	ret = alloc_chrdev_region(&media_dev_t, 0, MEDIA_NUM_DEVICES,
> -				  MEDIA_NAME);
> +	ret = register_chrdev_ids(&media_dev_t, MEDIA_NUM_DEVICES, MEDIA_NAME);
>  	if (ret < 0) {
>  		printk(KERN_WARNING "media: unable to allocate major\n");
>  		return ret;
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> index fd237ab..28f2968 100644
> --- a/drivers/media/rc/lirc_dev.c
> +++ b/drivers/media/rc/lirc_dev.c
> @@ -780,11 +780,11 @@ static int __init lirc_dev_init(void)
>  		goto error;
>  	}
>  
> -	retval = alloc_chrdev_region(&lirc_base_dev, 0, MAX_IRCTL_DEVICES,
> +	retval = register_chrdev_ids(&lirc_base_dev, MAX_IRCTL_DEVICES,
>  				     IRCTL_DEV_NAME);
>  	if (retval) {
>  		class_destroy(lirc_class);
> -		printk(KERN_ERR "lirc_dev: alloc_chrdev_region failed\n");
> +		printk(KERN_ERR "lirc_dev: register_chrdev_ids() failed\n");
>  		goto error;
>  	}
>  
> diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
> index 6dc7196..9ae24e2 100644
> --- a/drivers/media/video/v4l2-dev.c
> +++ b/drivers/media/video/v4l2-dev.c
> @@ -761,7 +761,7 @@ static int __init videodev_init(void)
>  	int ret;
>  
>  	printk(KERN_INFO "Linux video capture interface: v2.00\n");
> -	ret = register_chrdev_region(dev, VIDEO_NUM_DEVICES, VIDEO_NAME);
> +	ret = register_chrdev_ids(&dev, VIDEO_NUM_DEVICES, VIDEO_NAME);
>  	if (ret < 0) {
>  		printk(KERN_WARNING "videodev: unable to get major %d\n",
>  				VIDEO_MAJOR);


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

* Re: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API
  2011-05-21  5:15   ` Jim Cromie
@ 2011-05-21 21:14     ` Greg KH
  2011-05-22 14:55       ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2011-05-21 21:14 UTC (permalink / raw)
  To: Jim Cromie; +Cc: linux-kernel

On Fri, May 20, 2011 at 11:15:04PM -0600, Jim Cromie wrote:
> On Thu, May 19, 2011 at 4:44 PM, Greg KH <gregkh@suse.de> wrote:
> > On Thu, May 19, 2011 at 03:33:03PM -0600, Jim Cromie wrote:
> >> over on kernelnewbies, gregkh said:
> >>
> >>      The chardev stuff is a mess, I keep meaning for years to clean it
> >>      up.  Any proposals on a sane interface for this stuff is greatly
> >>      appreciated.
> >>
> >> this is a 1st step.
> >>
> >> register_chrdev_ids() replaces and deprecates register_chrdev_region()
> >> and alloc_chrdev_region() with a single function that works for both
> >> dynamic and static major numbers.
> >>
> >> Like alloc_chrdev_region(), 1st arg is a dev_t*, but its an in/out
> >> parameter, and expects both major and minor to be preset, and thus the
> >> separate minor arg is dropped.  If major == 0, a dynamic major is
> >> reserved, saved into 1st arg, and thus available to caller afterwards.
> >>
> >> [PATCH 01/23] add register_chrdev_ids() to char_dev.c, API
> >> [PATCH 02/23] reimplement alloc_chrdev_region with
> >> [PATCH 03/23] use register_chrdev_ids to replace
> >> [PATCH 04/23] use register_chrdev_ids in drivers/tty/
> >> [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/
> >> [PATCH 06/23] use register_chrdev_ids in drivers/media/
> >> [PATCH 07/23] use register_chrdev_ids in drivers/s390/
> >> [PATCH 08/23] use register_chrdev_ids in drivers/scsi/
> >> [PATCH 09/23] use register_chrdev_ids in drivers/staging/
> >>
> >> Ive held back the rest, no point in spamming.
> >
> > It's a nice first step, but that's the easy part, what is your 2nd
> > through 4th one going to be?  :)
> >
> > I'd also like to sanatize the function namespace a bit as well, how
> > about chrdev_register_ids() instead?
> 
> that seems sensible, modern.
> also have register_chrdev(), which I presume should also be fixed.
> 
> > Ideally, we could drop down to a single register/unregister pair of
> > functions, that are easy to use and understand.
> 
> __register_chrdev() does more stuff, mainly around cdevs, fops.
> If fops was passed as NULL, we just do the __register_chardev_region()
> and return early, skipping the cdev_alloc() and everything afterwards,
> thus yielding register_chrdev_ids() behavior.
> 
> > Do you think you can
> > get there with this intermediate step or do you want to step back and
> > rethink this?
> 
> hmm.  If above is right, theres no need for the new api fn I added,
> and probably should also drop the __ on both (un)?register_chardev.
> So thats step 2 :)  Any ideas for 3 ?

Well, what do you think the end result should look like?  That will
determine the steps needed here.

thanks,

greg k-h

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

* Re: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API
  2011-05-21 21:14     ` Greg KH
@ 2011-05-22 14:55       ` Greg KH
  0 siblings, 0 replies; 17+ messages in thread
From: Greg KH @ 2011-05-22 14:55 UTC (permalink / raw)
  To: Greg KH; +Cc: Jim Cromie, linux-kernel

On Sat, May 21, 2011 at 02:14:23PM -0700, Greg KH wrote:
> On Fri, May 20, 2011 at 11:15:04PM -0600, Jim Cromie wrote:
> > On Thu, May 19, 2011 at 4:44 PM, Greg KH <gregkh@suse.de> wrote:
> > > On Thu, May 19, 2011 at 03:33:03PM -0600, Jim Cromie wrote:
> > >> over on kernelnewbies, gregkh said:
> > >>
> > >>      The chardev stuff is a mess, I keep meaning for years to clean it
> > >>      up.  Any proposals on a sane interface for this stuff is greatly
> > >>      appreciated.
> > >>
> > >> this is a 1st step.
> > >>
> > >> register_chrdev_ids() replaces and deprecates register_chrdev_region()
> > >> and alloc_chrdev_region() with a single function that works for both
> > >> dynamic and static major numbers.
> > >>
> > >> Like alloc_chrdev_region(), 1st arg is a dev_t*, but its an in/out
> > >> parameter, and expects both major and minor to be preset, and thus the
> > >> separate minor arg is dropped.  If major == 0, a dynamic major is
> > >> reserved, saved into 1st arg, and thus available to caller afterwards.
> > >>
> > >> [PATCH 01/23] add register_chrdev_ids() to char_dev.c, API
> > >> [PATCH 02/23] reimplement alloc_chrdev_region with
> > >> [PATCH 03/23] use register_chrdev_ids to replace
> > >> [PATCH 04/23] use register_chrdev_ids in drivers/tty/
> > >> [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/
> > >> [PATCH 06/23] use register_chrdev_ids in drivers/media/
> > >> [PATCH 07/23] use register_chrdev_ids in drivers/s390/
> > >> [PATCH 08/23] use register_chrdev_ids in drivers/scsi/
> > >> [PATCH 09/23] use register_chrdev_ids in drivers/staging/
> > >>
> > >> Ive held back the rest, no point in spamming.
> > >
> > > It's a nice first step, but that's the easy part, what is your 2nd
> > > through 4th one going to be?  :)
> > >
> > > I'd also like to sanatize the function namespace a bit as well, how
> > > about chrdev_register_ids() instead?
> > 
> > that seems sensible, modern.
> > also have register_chrdev(), which I presume should also be fixed.
> > 
> > > Ideally, we could drop down to a single register/unregister pair of
> > > functions, that are easy to use and understand.
> > 
> > __register_chrdev() does more stuff, mainly around cdevs, fops.
> > If fops was passed as NULL, we just do the __register_chardev_region()
> > and return early, skipping the cdev_alloc() and everything afterwards,
> > thus yielding register_chrdev_ids() behavior.
> > 
> > > Do you think you can
> > > get there with this intermediate step or do you want to step back and
> > > rethink this?
> > 
> > hmm.  If above is right, theres no need for the new api fn I added,
> > and probably should also drop the __ on both (un)?register_chardev.
> > So thats step 2 :)  Any ideas for 3 ?
> 
> Well, what do you think the end result should look like?  That will
> determine the steps needed here.

Oh, I also forgot, you need to look at the cdev_* api as well, that all
needs to be merged together with what you decide on here.

good luck,

greg k-h

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

end of thread, other threads:[~2011-05-22 15:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-19 21:33 [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Jim Cromie
2011-05-19 21:33 ` [PATCH 02/23] reimplement alloc_chrdev_region with register_chrdev_ids Jim Cromie
2011-05-19 21:33 ` [PATCH 03/23] use register_chrdev_ids to replace (register|alloc)_chrdev_region Jim Cromie
2011-05-19 21:33 ` [PATCH 04/23] use register_chrdev_ids in drivers/tty/ Jim Cromie
2011-05-19 21:33 ` [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/ Jim Cromie
2011-05-19 21:33 ` [PATCH 06/23] use register_chrdev_ids in drivers/media/ Jim Cromie
2011-05-21 12:48   ` Mauro Carvalho Chehab
2011-05-19 21:33 ` [PATCH 07/23] use register_chrdev_ids in drivers/s390/ Jim Cromie
2011-05-19 21:33 ` [PATCH 08/23] use register_chrdev_ids in drivers/scsi/ Jim Cromie
2011-05-20 15:42   ` Boaz Harrosh
2011-05-21  4:21     ` Jim Cromie
2011-05-21  7:59       ` Boaz Harrosh
2011-05-19 21:33 ` [PATCH 09/23] use register_chrdev_ids in drivers/staging/ Jim Cromie
2011-05-19 22:44 ` [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API Greg KH
2011-05-21  5:15   ` Jim Cromie
2011-05-21 21:14     ` Greg KH
2011-05-22 14:55       ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).