lkml.org 
[lkml]   [2006]   [Oct]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [Alsa-devel] [PATCH] Driver core: Don't ignore error returns from probing
    On Fri, 6 Oct 2006 11:41:05 +0200 (CEST),
    Jaroslav Kysela <perex@suse.cz> wrote:

    > > Hm, I don't think we should call device_release_driver if
    > > bus_attach_device failed (and I think calling bus_remove_device if
    > > bus_attach_device failed is unintuitive). I did a patch that added a
    > > function which undid just the things bus_add_device did (here:
    > > http://marc.theaimsgroup.com/?l=linux-kernel&m=115816560424389&w=2),
    > > which unfortunately got lost somewhere... (I'll rebase and resend.)
    >
    > Yes, but it might be better to check dev->is_registered flag in
    > bus_remove_device() before device_release_driver() call to save some code,
    > rather than reuse most of code in bus_delete_device().

    If we undid things (symlinks et al.) in the order we added them, we can
    factor out bus_delete_device() from bus_remove_device() and avoid both
    code duplication and calling bus_remove_device() if bus_attach_device()
    failed. Something like the patch below (untested).


    --- linux-2.6-CH.orig/drivers/base/base.h
    +++ linux-2.6-CH/drivers/base/base.h
    @@ -17,6 +17,7 @@ extern int attribute_container_init(void

    extern int bus_add_device(struct device * dev);
    extern int bus_attach_device(struct device * dev);
    +extern void bus_delete_device(struct device * dev);
    extern void bus_remove_device(struct device * dev);
    extern struct bus_type *get_bus(struct bus_type * bus);
    extern void put_bus(struct bus_type * bus);
    --- linux-2.6-CH.orig/drivers/base/bus.c
    +++ linux-2.6-CH/drivers/base/bus.c
    @@ -360,7 +360,7 @@ static void device_remove_attrs(struct b
    * bus_add_device - add device to bus
    * @dev: device being added
    *
    - * - Add the device to its bus's list of devices.
    + * - Add attributes.
    * - Create link to device's bus.
    */
    int bus_add_device(struct device * dev)
    @@ -424,29 +424,44 @@ int bus_attach_device(struct device * de
    }

    /**
    - * bus_remove_device - remove device from bus
    - * @dev: device to be removed
    + * bus_delete_device - undo bus_add_device
    + * @dev: device being deleted
    *
    - * - Remove symlink from bus's directory.
    - * - Delete device from bus's list.
    - * - Detach from its driver.
    - * - Drop reference taken in bus_add_device().
    + * - Remove symlink from bus's directory.
    + * - Remove attributes.
    + * - Drop reference taken in bus_add_device().
    */
    -void bus_remove_device(struct device * dev)
    +void bus_delete_device(struct device * dev)
    {
    if (dev->bus) {
    sysfs_remove_link(&dev->kobj, "subsystem");
    sysfs_remove_link(&dev->kobj, "bus");
    sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id);
    device_remove_attrs(dev->bus, dev);
    - dev->is_registered = 0;
    - klist_del(&dev->knode_bus);
    - pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
    - device_release_driver(dev);
    put_bus(dev->bus);
    }
    }

    +/**
    + * bus_remove_device - remove device from bus
    + * @dev: device to be removed
    + *
    + * - Remove symlink from bus's directory.
    + * - Delete device from bus's list.
    + * - Detach from its driver.
    + * - Drop reference taken in bus_add_device().
    + */
    +void bus_remove_device(struct device * dev)
    +{
    + if (!dev->bus)
    + return;
    + dev->is_registered = 0;
    + klist_del(&dev->knode_bus);
    + pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
    + device_release_driver(dev);
    + bus_delete_device(dev);
    +}
    +
    static int driver_add_attrs(struct bus_type * bus, struct device_driver * drv)
    {
    int error = 0;
    --- linux-2.6-CH.orig/drivers/base/core.c
    +++ linux-2.6-CH/drivers/base/core.c
    @@ -479,7 +479,9 @@ int device_add(struct device *dev)
    if ((error = bus_add_device(dev)))
    goto BusError;
    kobject_uevent(&dev->kobj, KOBJ_ADD);
    - bus_attach_device(dev);
    + error = bus_attach_device(dev);
    + if (error)
    + goto attachError;
    if (parent)
    klist_add_tail(&dev->knode_parent, &parent->klist_children);

    @@ -498,6 +500,8 @@ int device_add(struct device *dev)
    kfree(class_name);
    put_device(dev);
    return error;
    + attachError:
    + bus_delete_device(dev);
    BusError:
    device_pm_remove(dev);
    PMError:
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2006-10-06 13:17    [W:4.096 / U:0.300 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site