From 308a773fb2af0f8eaa52d752736890cdfea84d01 Mon Sep 17 00:00:00 2001 From: Oron Peled Date: Wed, 22 Feb 2012 12:12:10 +0000 Subject: [PATCH] sysfs channels: cleanup device files handling * Shortcut CLASS_DEV_CREATE/CLASS_DEV_DESTROY. No need to pass repetitive data (and NULL) on every call. * Create/remove fixed device files (ctl, timer, channel, pseudo) via generic code (fixed_devfiles_create()/fixed_devfiles_remove()) instead of repetitive code and flags. * Try to make all removal/cleanup functions idempotent, so we can safely call them on any failure without the need for multiple goto destinations. * Rename 'device_state_flags' to 'should_cleanup' and its member flags to a better/consistent naming. * Rename dahdi_sysfs_exit() to dahdi_sysfs_cleanup() and call it from a new proper dahdi_sysfs_exit() * In dahdi_sysfs_init(), handle dahdi_sysfs_chan_init() failures * Add dahdi_dbg() message before creating/removing all DEVICES objects. * Also move two KERN_INFO messages to a more correct locations: - The version reporting should be first (in dahdi-base.c) - The "Telephony... on major" reporting should be at the end of dahdi_sysfs_init() Signed-off-by: Oron Peled Acked-by: Shaun Ruffell Acked-by: Tzafrir Cohen git-svn-id: http://svn.asterisk.org/svn/dahdi/linux/trunk@10464 a0bf4364-ded3-4de4-8d8a-66a801d63aff --- drivers/dahdi/dahdi-base.c | 1 + drivers/dahdi/dahdi-sysfs-chan.c | 174 ++++++++++++++++++------------- drivers/dahdi/dahdi-sysfs.c | 66 +++++++----- 3 files changed, 145 insertions(+), 96 deletions(-) diff --git a/drivers/dahdi/dahdi-base.c b/drivers/dahdi/dahdi-base.c index d529f16..020150e 100644 --- a/drivers/dahdi/dahdi-base.c +++ b/drivers/dahdi/dahdi-base.c @@ -10021,6 +10021,7 @@ static int __init dahdi_init(void) { int res = 0; + module_printk(KERN_INFO, "Version: %s\n", dahdi_version); #ifdef CONFIG_PROC_FS root_proc_entry = proc_mkdir("dahdi", NULL); if (!root_proc_entry) { diff --git a/drivers/dahdi/dahdi-sysfs-chan.c b/drivers/dahdi/dahdi-sysfs-chan.c index e54ddf0..283895d 100644 --- a/drivers/dahdi/dahdi-sysfs-chan.c +++ b/drivers/dahdi/dahdi-sysfs-chan.c @@ -11,6 +11,7 @@ #include "dahdi.h" #include "dahdi-sysfs.h" +/* shortcuts, for code readability */ #define MAKE_DAHDI_DEV(num, name) \ CLASS_DEV_CREATE(dahdi_class, MKDEV(DAHDI_MAJOR, num), NULL, name) #define DEL_DAHDI_DEV(num) \ @@ -35,9 +36,7 @@ int chan_sysfs_create(struct dahdi_chan *chan) if (test_bit(DAHDI_FLAGBIT_DEVFILE, &chan->flags)) return 0; snprintf(chan_name, sizeof(chan_name), "dahdi!%d", chan->channo); - dummy = (void *)CLASS_DEV_CREATE(dahdi_class, - MKDEV(DAHDI_MAJOR, chan->channo), - NULL, chan_name); + dummy = (void *)MAKE_DAHDI_DEV(chan->channo, chan_name); if (IS_ERR(dummy)) { res = PTR_ERR(dummy); chan_err(chan, "Failed creating sysfs device: %d\n", @@ -52,10 +51,13 @@ void chan_sysfs_remove(struct dahdi_chan *chan) { if (!test_bit(DAHDI_FLAGBIT_DEVFILE, &chan->flags)) return; - CLASS_DEV_DESTROY(dahdi_class, MKDEV(DAHDI_MAJOR, chan->channo)); + DEL_DAHDI_DEV(chan->channo); clear_bit(DAHDI_FLAGBIT_DEVFILE, &chan->flags); } +/* + * Used by dahdi_transcode.c + */ int dahdi_register_chardev(struct dahdi_chardev *dev) { static const char *DAHDI_STRING = "dahdi!"; @@ -68,33 +70,115 @@ int dahdi_register_chardev(struct dahdi_chardev *dev) strcpy(udevname, DAHDI_STRING); strcat(udevname, dev->name); - CLASS_DEV_CREATE(dahdi_class, - MKDEV(DAHDI_MAJOR, dev->minor), NULL, udevname); + MAKE_DAHDI_DEV(dev->minor, udevname); kfree(udevname); return 0; } EXPORT_SYMBOL(dahdi_register_chardev); +/* + * Used by dahdi_transcode.c + */ int dahdi_unregister_chardev(struct dahdi_chardev *dev) { - CLASS_DEV_DESTROY(dahdi_class, MKDEV(DAHDI_MAJOR, dev->minor)); - + DEL_DAHDI_DEV(dev->minor); return 0; } EXPORT_SYMBOL(dahdi_unregister_chardev); -/* Only used to flag that the device exists: */ +/*--------- Sysfs Device handling ----*/ + +/* + * Describe fixed device files and maintain their + * pointer so fixed_devfiles_remove() can always be called + * and work cleanly + */ static struct { - unsigned int ctl:1; - unsigned int timer:1; - unsigned int channel:1; - unsigned int pseudo:1; -} dummy_dev; + int minor; + char *name; + void *dev; /* FIXME: wrong type because of old kernels */ +} fixed_minors[] = { + { DAHDI_CTL, "dahdi!ctl", }, + { DAHDI_TIMER, "dahdi!timer", }, + { DAHDI_CHANNEL, "dahdi!channel",}, + { DAHDI_PSEUDO, "dahdi!pseudo", }, +}; + +/* + * Removes /dev/dahdi/{ctl,timer,channel,pseudo} + * + * It is safe to call it during initialization error handling, + * as it skips non existing objects. + */ +static void fixed_devfiles_remove(void) +{ + int i; + + if (!dahdi_class) + return; + for (i = 0; i < ARRAY_SIZE(fixed_minors); i++) { + void *d = fixed_minors[i].dev; + if (d && !IS_ERR(d)) + dahdi_dbg(DEVICES, "Removing fixed device file %s\n", + fixed_minors[i].name); + DEL_DAHDI_DEV(fixed_minors[i].minor); + } +} + +/* + * Creates /dev/dahdi/{ctl,timer,channel,pseudo} + */ +static int fixed_devfiles_create(void) +{ + int i; + int res = 0; + + if (!dahdi_class) { + dahdi_err("%s: dahdi_class is not initialized yet!\n", + __func__); + res = -ENODEV; + goto cleanup; + } + for (i = 0; i < ARRAY_SIZE(fixed_minors); i++) { + char *name = fixed_minors[i].name; + int minor = fixed_minors[i].minor; + void *dummy; + + dahdi_dbg(DEVICES, "Making fixed device file %s\n", name); + dummy = (void *)MAKE_DAHDI_DEV(minor, name); + if (IS_ERR(dummy)) { + int res = PTR_ERR(dummy); + + dahdi_err("%s: failed (%d: %s). Error: %d\n", + __func__, minor, name, res); + goto cleanup; + } + fixed_minors[i].dev = dummy; + } + return 0; +cleanup: + fixed_devfiles_remove(); + return res; +} + +/* + * Called during driver unload and while handling any error during + * driver load. + * Always clean any (and only) objects that were initialized (invariant) + */ +static void sysfs_channels_cleanup(void) +{ + fixed_devfiles_remove(); + if (dahdi_class) { + dahdi_dbg(DEVICES, "Destroying DAHDI class:\n"); + class_destroy(dahdi_class); + dahdi_class = NULL; + } +} int __init dahdi_sysfs_chan_init(const struct file_operations *fops) { int res = 0; - void *dev; dahdi_class = class_create(THIS_MODULE, "dahdi"); if (IS_ERR(dahdi_class)) { @@ -103,68 +187,16 @@ int __init dahdi_sysfs_chan_init(const struct file_operations *fops) __func__, res); goto cleanup; } - dahdi_dbg(DEVICES, "Creating /dev/dahdi/timer:\n"); - dev = MAKE_DAHDI_DEV(DAHDI_TIMER, "dahdi!timer"); - if (IS_ERR(dev)) { - res = PTR_ERR(dev); + res = fixed_devfiles_create(); + if (res) goto cleanup; - } - dummy_dev.timer = 1; - - dahdi_dbg(DEVICES, "Creating /dev/dahdi/channel:\n"); - dev = MAKE_DAHDI_DEV(DAHDI_CHANNEL, "dahdi!channel"); - if (IS_ERR(dev)) { - res = PTR_ERR(dev); - goto cleanup; - } - dummy_dev.channel = 1; - - dahdi_dbg(DEVICES, "Creating /dev/dahdi/pseudo:\n"); - dev = MAKE_DAHDI_DEV(DAHDI_PSEUDO, "dahdi!pseudo"); - if (IS_ERR(dev)) { - res = PTR_ERR(dev); - goto cleanup; - } - dummy_dev.pseudo = 1; - - dahdi_dbg(DEVICES, "Creating /dev/dahdi/ctl:\n"); - dev = MAKE_DAHDI_DEV(DAHDI_CTL, "dahdi!ctl"); - if (IS_ERR(dev)) { - res = PTR_ERR(dev); - goto cleanup; - } - dummy_dev.ctl = 1; return 0; cleanup: - dahdi_sysfs_chan_exit(); + sysfs_channels_cleanup(); return res; } void dahdi_sysfs_chan_exit(void) { - if (dummy_dev.pseudo) { - dahdi_dbg(DEVICES, "Removing /dev/dahdi/pseudo:\n"); - DEL_DAHDI_DEV(DAHDI_PSEUDO); - dummy_dev.pseudo = 0; - } - if (dummy_dev.channel) { - dahdi_dbg(DEVICES, "Removing /dev/dahdi/channel:\n"); - DEL_DAHDI_DEV(DAHDI_CHANNEL); - dummy_dev.channel = 0; - } - if (dummy_dev.timer) { - dahdi_dbg(DEVICES, "Removing /dev/dahdi/timer:\n"); - DEL_DAHDI_DEV(DAHDI_TIMER); - dummy_dev.timer = 0; - } - if (dummy_dev.ctl) { - dahdi_dbg(DEVICES, "Removing /dev/dahdi/ctl:\n"); - DEL_DAHDI_DEV(DAHDI_CTL); - dummy_dev.ctl = 0; - } - if (dahdi_class && !IS_ERR(dahdi_class)) { - dahdi_dbg(DEVICES, "Destroying DAHDI class\n"); - class_destroy(dahdi_class); - dahdi_class = NULL; - } + sysfs_channels_cleanup(); } diff --git a/drivers/dahdi/dahdi-sysfs.c b/drivers/dahdi/dahdi-sysfs.c index 2fa265c..f59663a 100644 --- a/drivers/dahdi/dahdi-sysfs.c +++ b/drivers/dahdi/dahdi-sysfs.c @@ -349,7 +349,7 @@ int span_sysfs_create(struct dahdi_span *span) for (x = 0; x < span->channels; x++) { res = chan_sysfs_create(span->chans[x]); - if (res < 0) + if (res) goto cleanup; } return 0; @@ -361,10 +361,11 @@ cleanup: /* Only used to flag that the device exists: */ static struct { - unsigned int sysfs_driver_registered:1; - unsigned int sysfs_spans_bus_type:1; - unsigned int dahdi_device_bus_registered:1; -} device_state_flags; + unsigned int clean_dahdi_driver:1; + unsigned int clean_span_bus_type:1; + unsigned int clean_device_bus:1; + unsigned int clean_chardev:1; +} should_cleanup; static inline struct dahdi_device *to_ddev(struct device *dev) { @@ -603,25 +604,30 @@ static struct bus_type dahdi_device_bus = { .dev_attrs = dahdi_device_attrs, }; -void dahdi_sysfs_exit(void) +static void dahdi_sysfs_cleanup(void) { dahdi_dbg(DEVICES, "SYSFS\n"); - dahdi_sysfs_chan_exit(); - if (device_state_flags.sysfs_driver_registered) { + if (should_cleanup.clean_dahdi_driver) { dahdi_dbg(DEVICES, "Unregister driver\n"); driver_unregister(&dahdi_driver); - device_state_flags.sysfs_driver_registered = 0; + should_cleanup.clean_dahdi_driver = 0; } - if (device_state_flags.sysfs_spans_bus_type) { + if (should_cleanup.clean_span_bus_type) { dahdi_dbg(DEVICES, "Unregister span bus type\n"); bus_unregister(&spans_bus_type); - device_state_flags.sysfs_spans_bus_type = 0; + should_cleanup.clean_span_bus_type = 0; + } + dahdi_sysfs_chan_exit(); + if (should_cleanup.clean_chardev) { + dahdi_dbg(DEVICES, "Unregister character device\n"); + unregister_chrdev(DAHDI_MAJOR, "dahdi"); + should_cleanup.clean_chardev = 0; } - unregister_chrdev(DAHDI_MAJOR, "dahdi"); - if (device_state_flags.dahdi_device_bus_registered) { + if (should_cleanup.clean_device_bus) { + dahdi_dbg(DEVICES, "Unregister DAHDI device bus\n"); bus_unregister(&dahdi_device_bus); - device_state_flags.dahdi_device_bus_registered = 0; + should_cleanup.clean_device_bus = 0; } } @@ -674,12 +680,14 @@ int __init dahdi_sysfs_init(const struct file_operations *dahdi_fops) { int res = 0; + dahdi_dbg(DEVICES, "Registering DAHDI device bus\n"); res = bus_register(&dahdi_device_bus); if (res) return res; + should_cleanup.clean_device_bus = 1; - device_state_flags.dahdi_device_bus_registered = 1; - + dahdi_dbg(DEVICES, + "Registering character device (major=%d)\n", DAHDI_MAJOR); res = register_chrdev(DAHDI_MAJOR, "dahdi", dahdi_fops); if (res) { module_printk(KERN_ERR, @@ -687,30 +695,38 @@ int __init dahdi_sysfs_init(const struct file_operations *dahdi_fops) "handler on %d\n", DAHDI_MAJOR); return res; } - module_printk(KERN_INFO, "Telephony Interface Registered on major %d\n", - DAHDI_MAJOR); - module_printk(KERN_INFO, "Version: %s\n", dahdi_version); + should_cleanup.clean_chardev = 1; + + res = dahdi_sysfs_chan_init(dahdi_fops); + if (res) + goto cleanup; - dahdi_sysfs_chan_init(dahdi_fops); res = bus_register(&spans_bus_type); - if (res != 0) { + if (res) { dahdi_err("%s: bus_register(%s) failed. Error number %d", __func__, spans_bus_type.name, res); goto cleanup; } - device_state_flags.sysfs_spans_bus_type = 1; + should_cleanup.clean_span_bus_type = 1; + res = driver_register(&dahdi_driver); - if (res < 0) { + if (res) { dahdi_err("%s: driver_register(%s) failed. Error number %d", __func__, dahdi_driver.name, res); goto cleanup; } - device_state_flags.sysfs_driver_registered = 1; + should_cleanup.clean_dahdi_driver = 1; + module_printk(KERN_INFO, "Telephony Interface Registered on major %d\n", + DAHDI_MAJOR); return 0; cleanup: - dahdi_sysfs_exit(); + dahdi_sysfs_cleanup(); return res; } +void __exit dahdi_sysfs_exit(void) +{ + dahdi_sysfs_cleanup(); +}