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