162306a36Sopenharmony_ciContributions are solicited in particular to remedy the following issues:
262306a36Sopenharmony_ci
362306a36Sopenharmony_cicpcihp:
462306a36Sopenharmony_ci
562306a36Sopenharmony_ci* There are no implementations of the ->hardware_test, ->get_power and
662306a36Sopenharmony_ci  ->set_power callbacks in struct cpci_hp_controller_ops.  Why were they
762306a36Sopenharmony_ci  introduced?  Can they be removed from the struct?
862306a36Sopenharmony_ci
962306a36Sopenharmony_cicpqphp:
1062306a36Sopenharmony_ci
1162306a36Sopenharmony_ci* The driver spawns a kthread cpqhp_event_thread() which is woken by the
1262306a36Sopenharmony_ci  hardirq handler cpqhp_ctrl_intr().  Convert this to threaded IRQ handling.
1362306a36Sopenharmony_ci  The kthread is also woken from the timer pushbutton_helper_thread(),
1462306a36Sopenharmony_ci  convert it to call irq_wake_thread().  Use pciehp as a template.
1562306a36Sopenharmony_ci
1662306a36Sopenharmony_ci* A large portion of cpqphp_ctrl.c and cpqphp_pci.c concerns resource
1762306a36Sopenharmony_ci  management.  Doesn't this duplicate functionality in the core?
1862306a36Sopenharmony_ci
1962306a36Sopenharmony_ciibmphp:
2062306a36Sopenharmony_ci
2162306a36Sopenharmony_ci* Implementations of hotplug_slot_ops callbacks such as get_adapter_present()
2262306a36Sopenharmony_ci  in ibmphp_core.c create a copy of the struct slot on the stack, then perform
2362306a36Sopenharmony_ci  the actual operation on that copy.  Determine if this overhead is necessary,
2462306a36Sopenharmony_ci  delete it if not.  The functions also perform a NULL pointer check on the
2562306a36Sopenharmony_ci  struct hotplug_slot, this seems superfluous.
2662306a36Sopenharmony_ci
2762306a36Sopenharmony_ci* Several functions access the pci_slot member in struct hotplug_slot even
2862306a36Sopenharmony_ci  though pci_hotplug.h declares it private.  See get_max_bus_speed() for an
2962306a36Sopenharmony_ci  example.  Either the pci_slot member should no longer be declared private
3062306a36Sopenharmony_ci  or ibmphp should store a pointer to its bus in struct slot.  Probably the
3162306a36Sopenharmony_ci  former.
3262306a36Sopenharmony_ci
3362306a36Sopenharmony_ci* ibmphp_init_devno() takes a struct slot **, it could instead take a
3462306a36Sopenharmony_ci  struct slot *.
3562306a36Sopenharmony_ci
3662306a36Sopenharmony_ci* The return value of pci_hp_register() is not checked.
3762306a36Sopenharmony_ci
3862306a36Sopenharmony_ci* The various slot data structures are difficult to follow and need to be
3962306a36Sopenharmony_ci  simplified.  A lot of functions are too large and too complex, they need
4062306a36Sopenharmony_ci  to be broken up into smaller, manageable pieces.  Negative examples are
4162306a36Sopenharmony_ci  ebda_rsrc_controller() and configure_bridge().
4262306a36Sopenharmony_ci
4362306a36Sopenharmony_ci* A large portion of ibmphp_res.c and ibmphp_pci.c concerns resource
4462306a36Sopenharmony_ci  management.  Doesn't this duplicate functionality in the core?
4562306a36Sopenharmony_ci
4662306a36Sopenharmony_cisgi_hotplug:
4762306a36Sopenharmony_ci
4862306a36Sopenharmony_ci* Several functions access the pci_slot member in struct hotplug_slot even
4962306a36Sopenharmony_ci  though pci_hotplug.h declares it private.  See sn_hp_destroy() for an
5062306a36Sopenharmony_ci  example.  Either the pci_slot member should no longer be declared private
5162306a36Sopenharmony_ci  or sgi_hotplug should store a pointer to it in struct slot.  Probably the
5262306a36Sopenharmony_ci  former.
5362306a36Sopenharmony_ci
5462306a36Sopenharmony_cishpchp:
5562306a36Sopenharmony_ci
5662306a36Sopenharmony_ci* There is only a single implementation of struct hpc_ops.  Can the struct be
5762306a36Sopenharmony_ci  removed and its functions invoked directly?  This has already been done in
5862306a36Sopenharmony_ci  pciehp with commit 82a9e79ef132 ("PCI: pciehp: remove hpc_ops").  Clarify
5962306a36Sopenharmony_ci  if there was a specific reason not to apply the same change to shpchp.
6062306a36Sopenharmony_ci
6162306a36Sopenharmony_ci* The hardirq handler shpc_isr() queues events on a workqueue.  It can be
6262306a36Sopenharmony_ci  simplified by converting it to threaded IRQ handling.  Use pciehp as a
6362306a36Sopenharmony_ci  template.
64