1a0d58937SLukas WunnerContributions are solicited in particular to remedy the following issues: 2a0d58937SLukas Wunner 3a0d58937SLukas Wunnercpcihp: 4a0d58937SLukas Wunner 5a0d58937SLukas Wunner* There are no implementations of the ->hardware_test, ->get_power and 6a0d58937SLukas Wunner ->set_power callbacks in struct cpci_hp_controller_ops. Why were they 7a0d58937SLukas Wunner introduced? Can they be removed from the struct? 8a0d58937SLukas Wunner 9*e6e9a9a4SNam Cao* Returned code from pci_hp_add_bridge() is not checked. 10*e6e9a9a4SNam Cao 11a0d58937SLukas Wunnercpqphp: 12a0d58937SLukas Wunner 13a0d58937SLukas Wunner* The driver spawns a kthread cpqhp_event_thread() which is woken by the 14a0d58937SLukas Wunner hardirq handler cpqhp_ctrl_intr(). Convert this to threaded IRQ handling. 15a0d58937SLukas Wunner The kthread is also woken from the timer pushbutton_helper_thread(), 16a0d58937SLukas Wunner convert it to call irq_wake_thread(). Use pciehp as a template. 17a0d58937SLukas Wunner 18a0d58937SLukas Wunner* A large portion of cpqphp_ctrl.c and cpqphp_pci.c concerns resource 19a0d58937SLukas Wunner management. Doesn't this duplicate functionality in the core? 20a0d58937SLukas Wunner 21*e6e9a9a4SNam Cao* Returned code from pci_hp_add_bridge() is not checked. 22*e6e9a9a4SNam Cao 23a0d58937SLukas Wunneribmphp: 24a0d58937SLukas Wunner 25a0d58937SLukas Wunner* Implementations of hotplug_slot_ops callbacks such as get_adapter_present() 26a0d58937SLukas Wunner in ibmphp_core.c create a copy of the struct slot on the stack, then perform 27a0d58937SLukas Wunner the actual operation on that copy. Determine if this overhead is necessary, 28a0d58937SLukas Wunner delete it if not. The functions also perform a NULL pointer check on the 29a0d58937SLukas Wunner struct hotplug_slot, this seems superfluous. 30a0d58937SLukas Wunner 31a0d58937SLukas Wunner* Several functions access the pci_slot member in struct hotplug_slot even 32a0d58937SLukas Wunner though pci_hotplug.h declares it private. See get_max_bus_speed() for an 33a0d58937SLukas Wunner example. Either the pci_slot member should no longer be declared private 34a0d58937SLukas Wunner or ibmphp should store a pointer to its bus in struct slot. Probably the 35a0d58937SLukas Wunner former. 36a0d58937SLukas Wunner 37a0d58937SLukas Wunner* ibmphp_init_devno() takes a struct slot **, it could instead take a 38a0d58937SLukas Wunner struct slot *. 39a0d58937SLukas Wunner 40a0d58937SLukas Wunner* The return value of pci_hp_register() is not checked. 41a0d58937SLukas Wunner 42a0d58937SLukas Wunner* The various slot data structures are difficult to follow and need to be 43a0d58937SLukas Wunner simplified. A lot of functions are too large and too complex, they need 44a0d58937SLukas Wunner to be broken up into smaller, manageable pieces. Negative examples are 45a0d58937SLukas Wunner ebda_rsrc_controller() and configure_bridge(). 46a0d58937SLukas Wunner 47a0d58937SLukas Wunner* A large portion of ibmphp_res.c and ibmphp_pci.c concerns resource 48a0d58937SLukas Wunner management. Doesn't this duplicate functionality in the core? 49a0d58937SLukas Wunner 50*e6e9a9a4SNam Cao* Returned code from pci_hp_add_bridge() is not checked. 51*e6e9a9a4SNam Cao 52a0d58937SLukas Wunnershpchp: 53a0d58937SLukas Wunner 54a0d58937SLukas Wunner* The hardirq handler shpc_isr() queues events on a workqueue. It can be 55a0d58937SLukas Wunner simplified by converting it to threaded IRQ handling. Use pciehp as a 56a0d58937SLukas Wunner template. 57