On Wed, Sep 02, 2009 at 06:20:58PM +0900, Tejun Heo wrote: > Separate out pci_add_dynid() from store_new_id() and export it so that > in-kernel code can add PCI IDs dynamically. As the function will be > available regardless of HOTPLUG, put it and pull pci_free_dynids() > outside of CONFIG_HOTPLUG. > > This will be used by pci-stub to initialize initial IDs via module > param. > > Signed-off-by: Tejun Heo <tj@xxxxxxxxxx> > --- > drivers/pci/pci-driver.c | 119 +++++++++++++++++++++++++++++------------------ > include/linux/pci.h | 5 + > 2 files changed, 79 insertions(+), 45 deletions(-) > > Index: ata/drivers/pci/pci-driver.c > =================================================================== > --- ata.orig/drivers/pci/pci-driver.c > +++ ata/drivers/pci/pci-driver.c > @@ -19,37 +19,98 @@ > #include <linux/cpu.h> > #include "pci.h" > > -/* > - * Dynamic device IDs are disabled for !CONFIG_HOTPLUG > - */ The comment was removed here. Ok. > - > struct pci_dynid { > struct list_head node; > struct pci_device_id id; > }; > > -#ifdef CONFIG_HOTPLUG > +/** > + * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices > + * @drv: target pci driver > + * @vendor: PCI vendor ID > + * @device: PCI device ID > + * @subvendor: PCI subvendor ID > + * @subdevice: PCI subdevice ID > + * @class: PCI class > + * @class_mask: PCI class mask > + * @driver_data: private driver data > + * > + * Adds a new dynamic pci device ID to this driver and causes the > + * driver to probe for all devices again. @drv must have been > + * registered prior to calling this function. > + * > + * CONTEXT: > + * Does GFP_KERNEL allocation. > + * > + * RETURNS: > + * 0 on success, -errno on failure. > + */ > +int pci_add_dynid(struct pci_driver *drv, > + unsigned int vendor, unsigned int device, > + unsigned int subvendor, unsigned int subdevice, > + unsigned int class, unsigned int class_mask, > + unsigned long driver_data) > +{ > + struct pci_dynid *dynid; > + int retval; > + > + dynid = kzalloc(sizeof(*dynid), GFP_KERNEL); > + if (!dynid) > + return -ENOMEM; > + > + dynid->id.vendor = vendor; > + dynid->id.device = device; > + dynid->id.subvendor = subvendor; > + dynid->id.subdevice = subdevice; > + dynid->id.class = class; > + dynid->id.class_mask = class_mask; > + dynid->id.driver_data = driver_data; > > + spin_lock(&drv->dynids.lock); > + list_add_tail(&dynid->node, &drv->dynids.list); > + spin_unlock(&drv->dynids.lock); > + > + get_driver(&drv->driver); Return value ignored caught my attention... But I can't say if that's wrong. I'm not finding any documentation on get_driver() in Documentation/ . Looking at drivers/base/driver.c:get_driver() code, I'm confused why this function bothers returning struct device_driver *. My expectation is the input parameter "drv" is the same as "priv->driver" that gets returned. No? In any case, if the Docbook comments for get_driver() could explain whatever subtle difference there is, that would be helpful. > + retval = driver_attach(&drv->driver); > + put_driver(&drv->driver); > + > + return retval; > +} > + > +static void pci_free_dynids(struct pci_driver *drv) > +{ > + struct pci_dynid *dynid, *n; > + > + spin_lock(&drv->dynids.lock); > + list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) { > + list_del(&dynid->node); > + kfree(dynid); > + } > + spin_unlock(&drv->dynids.lock); > +} > + > +/* > + * Dynamic device ID manipulation via sysfs is disabled for !CONFIG_HOTPLUG > + */ But same comment added back here. Is this comment correct? If comments are the only thing that change, please add: Reviewed-by: Grant Grundler <grundler@xxxxxxxxxxxxxxxx> This patch seems pretty straight forward. thanks grant > +#ifdef CONFIG_HOTPLUG > /** > - * store_new_id - add a new PCI device ID to this driver and re-probe devices > + * store_new_id - sysfs frontend to pci_add_dynid() > * @driver: target device driver > * @buf: buffer for scanning device ID data > * @count: input size > * > - * Adds a new dynamic pci device ID to this driver, > - * and causes the driver to probe for all devices again. > + * Allow PCI IDs to be added to an existing driver via sysfs. > */ > static ssize_t > store_new_id(struct device_driver *driver, const char *buf, size_t count) > { > - struct pci_dynid *dynid; > struct pci_driver *pdrv = to_pci_driver(driver); > const struct pci_device_id *ids = pdrv->id_table; > __u32 vendor, device, subvendor=PCI_ANY_ID, > subdevice=PCI_ANY_ID, class=0, class_mask=0; > unsigned long driver_data=0; > int fields=0; > - int retval=0; > + int retval; > > fields = sscanf(buf, "%x %x %x %x %x %x %lx", > &vendor, &device, &subvendor, &subdevice, > @@ -72,27 +133,8 @@ store_new_id(struct device_driver *drive > return retval; > } > > - dynid = kzalloc(sizeof(*dynid), GFP_KERNEL); > - if (!dynid) > - return -ENOMEM; > - > - dynid->id.vendor = vendor; > - dynid->id.device = device; > - dynid->id.subvendor = subvendor; > - dynid->id.subdevice = subdevice; > - dynid->id.class = class; > - dynid->id.class_mask = class_mask; > - dynid->id.driver_data = driver_data; > - > - spin_lock(&pdrv->dynids.lock); > - list_add_tail(&dynid->node, &pdrv->dynids.list); > - spin_unlock(&pdrv->dynids.lock); > - > - if (get_driver(&pdrv->driver)) { > - retval = driver_attach(&pdrv->driver); > - put_driver(&pdrv->driver); > - } > - > + retval = pci_add_dynid(pdrv, vendor, device, subvendor, subdevice, > + class, class_mask, driver_data); > if (retval) > return retval; > return count; > @@ -145,19 +187,6 @@ store_remove_id(struct device_driver *dr > } > static DRIVER_ATTR(remove_id, S_IWUSR, NULL, store_remove_id); > > -static void > -pci_free_dynids(struct pci_driver *drv) > -{ > - struct pci_dynid *dynid, *n; > - > - spin_lock(&drv->dynids.lock); > - list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) { > - list_del(&dynid->node); > - kfree(dynid); > - } > - spin_unlock(&drv->dynids.lock); > -} > - > static int > pci_create_newid_file(struct pci_driver *drv) > { > @@ -186,7 +215,6 @@ static void pci_remove_removeid_file(str > driver_remove_file(&drv->driver, &driver_attr_remove_id); > } > #else /* !CONFIG_HOTPLUG */ > -static inline void pci_free_dynids(struct pci_driver *drv) {} > static inline int pci_create_newid_file(struct pci_driver *drv) > { > return 0; > @@ -1106,6 +1134,7 @@ static int __init pci_driver_init(void) > > postcore_initcall(pci_driver_init); > > +EXPORT_SYMBOL(pci_add_dynid); > EXPORT_SYMBOL(pci_match_id); > EXPORT_SYMBOL(__pci_register_driver); > EXPORT_SYMBOL(pci_unregister_driver); > Index: ata/include/linux/pci.h > =================================================================== > --- ata.orig/include/linux/pci.h > +++ ata/include/linux/pci.h > @@ -794,6 +794,11 @@ int __must_check __pci_register_driver(s > void pci_unregister_driver(struct pci_driver *dev); > void pci_remove_behind_bridge(struct pci_dev *dev); > struct pci_driver *pci_dev_driver(const struct pci_dev *dev); > +int pci_add_dynid(struct pci_driver *drv, > + unsigned int vendor, unsigned int device, > + unsigned int subvendor, unsigned int subdevice, > + unsigned int class, unsigned int class_mask, > + unsigned long driver_data); > const struct pci_device_id *pci_match_id(const struct pci_device_id *ids, > struct pci_dev *dev); > int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html