18c2ecf20Sopenharmony_ciContributions are solicited in particular to remedy the following issues:
28c2ecf20Sopenharmony_ci
38c2ecf20Sopenharmony_cicpcihp:
48c2ecf20Sopenharmony_ci
58c2ecf20Sopenharmony_ci* There are no implementations of the ->hardware_test, ->get_power and
68c2ecf20Sopenharmony_ci  ->set_power callbacks in struct cpci_hp_controller_ops.  Why were they
78c2ecf20Sopenharmony_ci  introduced?  Can they be removed from the struct?
88c2ecf20Sopenharmony_ci
98c2ecf20Sopenharmony_cicpqphp:
108c2ecf20Sopenharmony_ci
118c2ecf20Sopenharmony_ci* The driver spawns a kthread cpqhp_event_thread() which is woken by the
128c2ecf20Sopenharmony_ci  hardirq handler cpqhp_ctrl_intr().  Convert this to threaded IRQ handling.
138c2ecf20Sopenharmony_ci  The kthread is also woken from the timer pushbutton_helper_thread(),
148c2ecf20Sopenharmony_ci  convert it to call irq_wake_thread().  Use pciehp as a template.
158c2ecf20Sopenharmony_ci
168c2ecf20Sopenharmony_ci* A large portion of cpqphp_ctrl.c and cpqphp_pci.c concerns resource
178c2ecf20Sopenharmony_ci  management.  Doesn't this duplicate functionality in the core?
188c2ecf20Sopenharmony_ci
198c2ecf20Sopenharmony_ciibmphp:
208c2ecf20Sopenharmony_ci
218c2ecf20Sopenharmony_ci* Implementations of hotplug_slot_ops callbacks such as get_adapter_present()
228c2ecf20Sopenharmony_ci  in ibmphp_core.c create a copy of the struct slot on the stack, then perform
238c2ecf20Sopenharmony_ci  the actual operation on that copy.  Determine if this overhead is necessary,
248c2ecf20Sopenharmony_ci  delete it if not.  The functions also perform a NULL pointer check on the
258c2ecf20Sopenharmony_ci  struct hotplug_slot, this seems superfluous.
268c2ecf20Sopenharmony_ci
278c2ecf20Sopenharmony_ci* Several functions access the pci_slot member in struct hotplug_slot even
288c2ecf20Sopenharmony_ci  though pci_hotplug.h declares it private.  See get_max_bus_speed() for an
298c2ecf20Sopenharmony_ci  example.  Either the pci_slot member should no longer be declared private
308c2ecf20Sopenharmony_ci  or ibmphp should store a pointer to its bus in struct slot.  Probably the
318c2ecf20Sopenharmony_ci  former.
328c2ecf20Sopenharmony_ci
338c2ecf20Sopenharmony_ci* The functions get_max_adapter_speed() and get_bus_name() are commented out.
348c2ecf20Sopenharmony_ci  Can they be deleted?  There are also forward declarations at the top of
358c2ecf20Sopenharmony_ci  ibmphp_core.c as well as pointers in ibmphp_hotplug_slot_ops, likewise
368c2ecf20Sopenharmony_ci  commented out.
378c2ecf20Sopenharmony_ci
388c2ecf20Sopenharmony_ci* ibmphp_init_devno() takes a struct slot **, it could instead take a
398c2ecf20Sopenharmony_ci  struct slot *.
408c2ecf20Sopenharmony_ci
418c2ecf20Sopenharmony_ci* The return value of pci_hp_register() is not checked.
428c2ecf20Sopenharmony_ci
438c2ecf20Sopenharmony_ci* The various slot data structures are difficult to follow and need to be
448c2ecf20Sopenharmony_ci  simplified.  A lot of functions are too large and too complex, they need
458c2ecf20Sopenharmony_ci  to be broken up into smaller, manageable pieces.  Negative examples are
468c2ecf20Sopenharmony_ci  ebda_rsrc_controller() and configure_bridge().
478c2ecf20Sopenharmony_ci
488c2ecf20Sopenharmony_ci* A large portion of ibmphp_res.c and ibmphp_pci.c concerns resource
498c2ecf20Sopenharmony_ci  management.  Doesn't this duplicate functionality in the core?
508c2ecf20Sopenharmony_ci
518c2ecf20Sopenharmony_cisgi_hotplug:
528c2ecf20Sopenharmony_ci
538c2ecf20Sopenharmony_ci* Several functions access the pci_slot member in struct hotplug_slot even
548c2ecf20Sopenharmony_ci  though pci_hotplug.h declares it private.  See sn_hp_destroy() for an
558c2ecf20Sopenharmony_ci  example.  Either the pci_slot member should no longer be declared private
568c2ecf20Sopenharmony_ci  or sgi_hotplug should store a pointer to it in struct slot.  Probably the
578c2ecf20Sopenharmony_ci  former.
588c2ecf20Sopenharmony_ci
598c2ecf20Sopenharmony_cishpchp:
608c2ecf20Sopenharmony_ci
618c2ecf20Sopenharmony_ci* There is only a single implementation of struct hpc_ops.  Can the struct be
628c2ecf20Sopenharmony_ci  removed and its functions invoked directly?  This has already been done in
638c2ecf20Sopenharmony_ci  pciehp with commit 82a9e79ef132 ("PCI: pciehp: remove hpc_ops").  Clarify
648c2ecf20Sopenharmony_ci  if there was a specific reason not to apply the same change to shpchp.
658c2ecf20Sopenharmony_ci
668c2ecf20Sopenharmony_ci* The ->get_mode1_ECC_cap callback in shpchp_hpc_ops is never invoked.
678c2ecf20Sopenharmony_ci  Why was it introduced?  Can it be removed?
688c2ecf20Sopenharmony_ci
698c2ecf20Sopenharmony_ci* The hardirq handler shpc_isr() queues events on a workqueue.  It can be
708c2ecf20Sopenharmony_ci  simplified by converting it to threaded IRQ handling.  Use pciehp as a
718c2ecf20Sopenharmony_ci  template.
72