* Greg KH <greg@xxxxxxxxx>:
> On Mon, Jun 15, 2009 at 11:52:31AM -0600, Alex Chiang wrote:
> > * Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>:
> > > Create symbolic link to hotplug driver module in the PCI slot
> > > directory (/sys/bus/pci/slots/<SLOT#>). In the past, we need to load
> > > hotplug drivers one by one to identify the hotplug driver that handles
> > > the slot, and it was very inconvenient especially for trouble shooting.
> > > With this change, we can easily identify the hotplug driver.
> >
> > I think this implementation is much nicer than the previous one.
> >
> > One comment below regarding the symlink name.
> >
> > > Index: 20090612/drivers/pci/slot.c
> > > ===================================================================
> > > --- 20090612.orig/drivers/pci/slot.c
> > > +++ 20090612/drivers/pci/slot.c
> > > @@ -307,6 +307,45 @@ void pci_destroy_slot(struct pci_slot *s
> > > }
> > > EXPORT_SYMBOL_GPL(pci_destroy_slot);
> > >
> > > +#if defined(CONFIG_HOTPLUG_PCI) || defined(CONFIG_HOTPLUG_PCI_MODULE)
> > > +#include <linux/pci_hotplug.h>
> > > +/**
> > > + * pci_hp_create_link - create symbolic link to the hotplug driver module.
> > > + * @slot: struct pci_slot
> > > + *
> > > + * Helper function for pci_hotplug_core.c to create symbolic link to
> > > + * the hotplug driver module.
> > > + */
> > > +void pci_hp_create_module_link(struct pci_slot *pci_slot)
> > > +{
> > > + struct hotplug_slot *slot = pci_slot->hotplug;
> > > + struct kobject *kobj = NULL;
> > > + int no_warn;
> > > +
> > > + if (!slot || !slot->ops)
> > > + return;
> > > + kobj = kset_find_obj(module_kset, slot->ops->mod_name);
> > > + if (!kobj)
> > > + return;
> > > + no_warn = sysfs_create_link(&pci_slot->kobj, kobj, "module");
> >
> > I would prefer to call this link "hotplug_module" or "hp_module".
> > The reason is, if we ever decide to create another link for that
> > slot's low level device driver, we won't be limited by something
> > that already called "module".
>
> Note that "module" is already used in sysfs, so as long as you are
> pointing to the same thing that those other symlinks are, you should be
> fine. You are pointing to /sys/module/NAME, here, right?
Looks like the common use of "module" is:
/sys/bus/$bus/drivers/$driver/module -> /sys/module/NAME
What I was reacting to is that this patch proposes:
/sys/bus/pci/slots/$slot/module -> /sys/module/NAME
This scheme is fine as long as only PCI hotplug modules create
those $slot entries.
But it is limiting, if in the future, we teach the PCI core to
create $slot entries when a low level device driver, say tg3 or
whatever, is loaded.
In that scenario, it would be more logical if module pointed at
the LLDD's module, not whatever PCI hotplug module happens to
claim that slot
Since we're talking about an ABI agreement with userspace, I
think "hotplug_module" is better for future flexibility.
Thanks.
/ac
--
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
