| Message ID | 49D08A0A.2070102@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
* Taku Izumi <izumi.taku@jp.fujitsu.com>: > This patch creates the symlink to the hotplug driver at the hotplug slot directory > (/sys/bus/pci/slots/<slot_name>). As a result, we can know easily what driver manages > the hotplug slot. > > <changelog> > - change the symlink name from driver name to "driver" according to Kenji-san's > comment. > > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> > > --- > drivers/pci/hotplug/pci_hotplug_core.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > Index: linux-next.29rc6/drivers/pci/hotplug/pci_hotplug_core.c > =================================================================== > --- linux-next.29rc6.orig/drivers/pci/hotplug/pci_hotplug_core.c > +++ linux-next.29rc6/drivers/pci/hotplug/pci_hotplug_core.c > @@ -420,9 +420,20 @@ static int has_test_file(struct pci_slot > return -ENOENT; > } > > +static int has_owner_file(struct pci_slot *pci_slot) > +{ > + struct hotplug_slot *slot = pci_slot->hotplug; > + if ((!slot) || (!slot->ops)) Stylistically, you don't need those extra parens. > + return -ENODEV; > + if (slot->ops->owner) > + return 0; The return value from this function seems backwards from the name of the function. Usually, a has_foo() or is_foo() convenience function returns a non-zero value if the "foo" property is true. In other words, when I read code like: if (has_owner_file(slot) == 0) Then that makes me think "slot" does _not_ have an owner file. Oh, I just read the earlier part of this thread. Eike Beer complained about the same thing... Would you mind creating a cleanup patch to fix these horrible names? I know it's not your fault, and you're just trying to add some useful functionality, but let's take the opportunity to get it right. Thanks. /ac > + return -ENOENT; > +} > + > static int fs_add_slot(struct pci_slot *slot) > { > int retval = 0; > + struct hotplug_slot *hotplug = slot->hotplug; > > if (has_power_file(slot) == 0) { > retval = sysfs_create_file(&slot->kobj, &hotplug_slot_attr_power.attr); > @@ -472,8 +483,20 @@ static int fs_add_slot(struct pci_slot * > goto exit_test; > } > > + if (has_owner_file(slot) == 0) { > + retval = sysfs_create_link(&slot->kobj, > + &hotplug->ops->owner->mkobj.kobj, > + "driver"); > + if (retval) > + goto exit_owner; > + } > + > goto exit; > > +exit_owner: > + if (has_test_file(slot) == 0) > + sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_test.attr); > + > exit_test: > if (has_cur_bus_speed_file(slot) == 0) > sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_cur_bus_speed.attr); > @@ -524,6 +547,9 @@ static void fs_remove_slot(struct pci_sl > > if (has_test_file(slot) == 0) > sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_test.attr); > + > + if (has_owner_file(slot) == 0) > + sysfs_remove_link(&slot->kobj, "driver"); > } > > static struct hotplug_slot *get_slot_from_name (const char *name) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Taku Izumi wrote: > This patch creates the symlink to the hotplug driver at the hotplug slot directory > (/sys/bus/pci/slots/<slot_name>). As a result, we can know easily what driver manages > the hotplug slot. > > <changelog> > - change the symlink name from driver name to "driver" according to Kenji-san's > comment. > > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> > I tested your patch and I found the following problems. - Symbolic link is not created when the hotplug controller driver is configured as a built-in module. The reason is the "owner" field in struct hotplug_slot_ops (initialized by THIS_MODULE) is NULL in this case. - Build error when CONFIG_MODULES is not set. The reason is struct module is not defined. I'll send the updated patch soon. Thanks, Kenji Kaneshige -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-next.29rc6/drivers/pci/hotplug/pci_hotplug_core.c =================================================================== --- linux-next.29rc6.orig/drivers/pci/hotplug/pci_hotplug_core.c +++ linux-next.29rc6/drivers/pci/hotplug/pci_hotplug_core.c @@ -420,9 +420,20 @@ static int has_test_file(struct pci_slot return -ENOENT; } +static int has_owner_file(struct pci_slot *pci_slot) +{ + struct hotplug_slot *slot = pci_slot->hotplug; + if ((!slot) || (!slot->ops)) + return -ENODEV; + if (slot->ops->owner) + return 0; + return -ENOENT; +} + static int fs_add_slot(struct pci_slot *slot) { int retval = 0; + struct hotplug_slot *hotplug = slot->hotplug; if (has_power_file(slot) == 0) { retval = sysfs_create_file(&slot->kobj, &hotplug_slot_attr_power.attr); @@ -472,8 +483,20 @@ static int fs_add_slot(struct pci_slot * goto exit_test; } + if (has_owner_file(slot) == 0) { + retval = sysfs_create_link(&slot->kobj, + &hotplug->ops->owner->mkobj.kobj, + "driver"); + if (retval) + goto exit_owner; + } + goto exit; +exit_owner: + if (has_test_file(slot) == 0) + sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_test.attr); + exit_test: if (has_cur_bus_speed_file(slot) == 0) sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_cur_bus_speed.attr); @@ -524,6 +547,9 @@ static void fs_remove_slot(struct pci_sl if (has_test_file(slot) == 0) sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_test.attr); + + if (has_owner_file(slot) == 0) + sysfs_remove_link(&slot->kobj, "driver"); } static struct hotplug_slot *get_slot_from_name (const char *name)
This patch creates the symlink to the hotplug driver at the hotplug slot directory (/sys/bus/pci/slots/<slot_name>). As a result, we can know easily what driver manages the hotplug slot. <changelog> - change the symlink name from driver name to "driver" according to Kenji-san's comment. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- drivers/pci/hotplug/pci_hotplug_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html