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