lkml.org 
[lkml]   [2008]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Create PNP device attributes via dev_attrs field of struct device
    On Tue, Sep 23, 2008 at 14:24, Drew Moseley <dmoseley@mvista.com> wrote:
    > I have seen an issue where the sysfs entries for a PNP device are
    > created nonatomically resulting in a race condition with freedesktop
    > HAL. The patch below is my first attempt at addressing this by creating
    > the device attributes as default bus attributes and allowing the
    > device_register() call to create them. Any nasty side effects I am not
    > addressing? Comments?

    > diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
    > index 9fd7bb9..b07787f 100644
    > --- a/drivers/pnp/base.h
    > +++ b/drivers/pnp/base.h
    > @@ -16,7 +16,7 @@ struct pnp_card *pnp_alloc_card(struct pnp_protocol *,
    > int id, char *pnpid);
    >
    > int pnp_add_device(struct pnp_dev *dev);
    > struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
    > -int pnp_interface_attach_device(struct pnp_dev *dev);
    > +void pnp_interface_attach_device(struct pnp_dev *dev);
    >
    > int pnp_add_card(struct pnp_card *card);
    > void pnp_remove_card(struct pnp_card *card);
    > diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
    > index a411582..bcd49ba 100644
    > --- a/drivers/pnp/core.c
    > +++ b/drivers/pnp/core.c
    > @@ -159,8 +159,6 @@ struct pnp_dev *pnp_alloc_dev(struct pnp_protocol
    > *protocol, int id, char *pnpid
    >
    > int __pnp_add_device(struct pnp_dev *dev)
    > {
    > - int ret;
    > -
    > pnp_fixup_device(dev);
    > dev->status = PNP_READY;
    > spin_lock(&pnp_lock);
    > @@ -168,12 +166,8 @@ int __pnp_add_device(struct pnp_dev *dev)
    > list_add_tail(&dev->protocol_list, &dev->protocol->devices);
    > spin_unlock(&pnp_lock);
    >
    > - ret = device_register(&dev->dev);
    > - if (ret)
    > - return ret;
    > -
    > pnp_interface_attach_device(dev);
    > - return 0;
    > + return device_register(&dev->dev);
    > }
    >
    > /*
    > diff --git a/drivers/pnp/interface.c b/drivers/pnp/interface.c
    > index a876ecf..442684d 100644
    > --- a/drivers/pnp/interface.c
    > +++ b/drivers/pnp/interface.c
    > @@ -243,8 +243,6 @@ static ssize_t pnp_show_options(struct device *dmdev,
    > return ret;
    > }
    >
    > -static DEVICE_ATTR(options, S_IRUGO, pnp_show_options, NULL);
    > -
    > static ssize_t pnp_show_current_resources(struct device *dmdev,
    > struct device_attribute *attr,
    > char *buf)
    > @@ -420,9 +418,6 @@ done:
    > return count;
    > }
    >
    > -static DEVICE_ATTR(resources, S_IRUGO | S_IWUSR,
    > - pnp_show_current_resources, pnp_set_current_resources);
    > -
    > static ssize_t pnp_show_current_ids(struct device *dmdev,
    > struct device_attribute *attr, char *buf)
    > {
    > @@ -437,27 +432,16 @@ static ssize_t pnp_show_current_ids(struct device
    > *dmdev,
    > return (str - buf);
    > }
    >
    > -static DEVICE_ATTR(id, S_IRUGO, pnp_show_current_ids, NULL);
    > +static struct device_attribute pnp_interface_attrs[] = {
    > + __ATTR(resources, S_IRUGO | S_IWUSR,
    > + pnp_show_current_resources,
    > + pnp_set_current_resources),
    > + __ATTR(options, S_IRUGO, pnp_show_options, NULL),
    > + __ATTR(id, S_IRUGO, pnp_show_current_ids, NULL),
    > + __ATTR_NULL,
    > +};
    >
    > -int pnp_interface_attach_device(struct pnp_dev *dev)
    > +void pnp_interface_attach_device(struct pnp_dev *dev)
    > {
    > - int rc = device_create_file(&dev->dev, &dev_attr_options);
    > -
    > - if (rc)
    > - goto err;
    > - rc = device_create_file(&dev->dev, &dev_attr_resources);
    > - if (rc)
    > - goto err_opt;
    > - rc = device_create_file(&dev->dev, &dev_attr_id);
    > - if (rc)
    > - goto err_res;
    > -
    > - return 0;
    > -
    > -err_res:
    > - device_remove_file(&dev->dev, &dev_attr_resources);
    > -err_opt:
    > - device_remove_file(&dev->dev, &dev_attr_options);
    > -err:
    > - return rc;
    > + dev->dev.bus->dev_attrs = pnp_interface_attrs;

    Any reason not to assign it statically to pnp_bus_type at in
    drivers/pnp/driver.c?

    > }

    Thanks,
    Kay


    \
     
     \ /
      Last update: 2008-09-24 07:01    [W:0.044 / U:0.016 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site