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