> @@ -94,6 +93,8 @@ static int init_slot(struct controller *ctrl)
> struct hotplug_slot_info *info = NULL;
> struct hotplug_slot_ops *ops = NULL;
> char name[SLOT_NAME_SIZE];
> + char *buffer;
> + int len;
> int retval = -ENOMEM;
>
> hotplug = kzalloc(sizeof(*hotplug), GFP_KERNEL);
> @@ -135,6 +136,19 @@ static int init_slot(struct controller *ctrl)
> if (retval)
> ctrl_err(ctrl,
> "pci_hp_register failed with error %d\n", retval);
I think it's natural to go to out: here if retval != 0.
I guess you intentionally didn't do that because you might want to do
workqueue cleanup in pciehp_release_ctrl() code path. But it is confusing
and hard to understand.
In the previous patch, you created the work queue in pcie_init_slot().
I think it's better. Maybe the reason you moved it from pcie_init_slot()
to init_slot() was that you needed the slot name by pci_hp_register().
How about using physical slot number, which is same as pci slot name by
pci_hp_register() on the normal platform, for the workqueue name instead?
> +
> + len = strlen(slot_name(slot)) + 16;
> + buffer = kzalloc(len, GFP_KERNEL);
> + if (!buffer) {
> + retval = -ENOMEM;
> + goto out;
> + }
> + snprintf(buffer, len, "pciehp_slot(%s)", slot_name(slot));
> + slot->wq = create_singlethread_workqueue(buffer);
> + if (!slot->wq)
> + retval = -ENOMEM;
> +
> + kfree(buffer);
> out:
> if (retval) {
> kfree(ops);
Regards,
Kenji Kaneshige
--
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
