2011-05-19 21:33:35

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API

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.


2011-05-19 21:36:02

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 02/23] reimplement alloc_chrdev_region with register_chrdev_ids


Signed-off-by: Jim Cromie <[email protected]>
---
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

2011-05-19 21:34:08

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 03/23] use register_chrdev_ids to replace (register|alloc)_chrdev_region

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 <[email protected]>
---
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

2011-05-19 21:41:53

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 04/23] use register_chrdev_ids in drivers/tty/

cc: Greg Kroah-Hartman <[email protected]>

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 <[email protected]>
---
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

2011-05-19 21:35:59

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 05/23] use register_chrdev_ids in drivers/infiniband/

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 <[email protected]>
cc: Roland Dreier <[email protected]>
cc: Sean Hefty <[email protected]>
cc: [email protected]

@ 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 <[email protected]>
---
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

2011-05-19 21:35:57

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 06/23] use register_chrdev_ids in drivers/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 <[email protected]>
cc: [email protected]

@ 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 <[email protected]>
---
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

2011-05-19 21:35:26

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 07/23] use register_chrdev_ids in drivers/s390/

cc: Heiko Carstens <[email protected]>
cc: Martin Schwidefsky <[email protected]>
cc: [email protected]
cc: [email protected]
Signed-off-by: Jim Cromie <[email protected]>
---
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

2011-05-19 21:34:15

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 08/23] use register_chrdev_ids in drivers/scsi/

cc: Doug Gilbert <[email protected]>
cc: [email protected]
cc: Benny Halevy <[email protected]>
cc: Boaz Harrosh <[email protected]>
cc: [email protected]
cc: Anil Ravindranath <[email protected]>
cc: [email protected]
Signed-off-by: Jim Cromie <[email protected]>
---
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

2011-05-19 21:34:13

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 09/23] use register_chrdev_ids in drivers/staging/

cc: Greg Kroah-Hartman <[email protected]>
cc: [email protected]
Signed-off-by: Jim Cromie <[email protected]>
---
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

2011-05-19 23:04:50

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API

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

2011-05-20 15:42:30

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH 08/23] use register_chrdev_ids in drivers/scsi/

On 05/20/2011 12:33 AM, Jim Cromie wrote:
> cc: Doug Gilbert <[email protected]>
> cc: [email protected]
> cc: Benny Halevy <[email protected]>
> cc: Boaz Harrosh <[email protected]>
> cc: [email protected]
> cc: Anil Ravindranath <[email protected]>
> cc: [email protected]
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> 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

2011-05-21 04:21:36

by Jim Cromie

[permalink] [raw]
Subject: Re: [PATCH 08/23] use register_chrdev_ids in drivers/scsi/

On Fri, May 20, 2011 at 9:42 AM, Boaz Harrosh <[email protected]> wrote:
> On 05/20/2011 12:33 AM, Jim Cromie wrote:
>> cc: Doug Gilbert <[email protected]>
>> cc: [email protected]
>> cc: Benny Halevy <[email protected]>
>> cc: Boaz Harrosh <[email protected]>
>> cc: [email protected]
>> cc: Anil Ravindranath <[email protected]>
>> cc: [email protected]
>> Signed-off-by: Jim Cromie <[email protected]>
>> ---
>> ?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

2011-05-21 05:15:37

by Jim Cromie

[permalink] [raw]
Subject: Re: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API

On Thu, May 19, 2011 at 4:44 PM, Greg KH <[email protected]> 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
>

2011-05-21 07:59:18

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH 08/23] use register_chrdev_ids in drivers/scsi/

On 05/21/2011 07:21 AM, Jim Cromie wrote:
> On Fri, May 20, 2011 at 9:42 AM, Boaz Harrosh <[email protected]> wrote:
>> On 05/20/2011 12:33 AM, Jim Cromie wrote:
>>> cc: Doug Gilbert <[email protected]>
>>> cc: [email protected]
>>> cc: Benny Halevy <[email protected]>
>>> cc: Boaz Harrosh <[email protected]>
>>> cc: [email protected]
>>> cc: Anil Ravindranath <[email protected]>
>>> cc: [email protected]
>>> Signed-off-by: Jim Cromie <[email protected]>
>>> ---
>>> 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

2011-05-21 12:48:20

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH 06/23] use register_chrdev_ids in drivers/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 <[email protected]>

>
> This and 2 subsequent patches brought to you by coccinelle/spatch
>
> cc: Mauro Carvalho Chehab <[email protected]>
> cc: [email protected]
>
> @ 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 <[email protected]>
> ---
> 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);

2011-05-21 21:14:37

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API

On Fri, May 20, 2011 at 11:15:04PM -0600, Jim Cromie wrote:
> On Thu, May 19, 2011 at 4:44 PM, Greg KH <[email protected]> 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

2011-05-22 15:03:35

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 00/23] add register_chrdev_ids() to char_dev.c, API

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 <[email protected]> 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