9676f40618
The ->get_mode1_ECC_cap callback in the shpchp_hpc_ops struct is never called, so remove it. [bhelgaas: squash] Link: https://lore.kernel.org/r/20221112142859.319733-2-ian@linux.cowan.aero Link: https://lore.kernel.org/r/20221112142859.319733-3-ian@linux.cowan.aero Link: https://lore.kernel.org/r/20221112142859.319733-4-ian@linux.cowan.aero Signed-off-by: Ian Cowan <ian@linux.cowan.aero> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
2.7 KiB
Contributions are solicited in particular to remedy the following issues:
cpcihp:
- There are no implementations of the ->hardware_test, ->get_power and ->set_power callbacks in struct cpci_hp_controller_ops. Why were they introduced? Can they be removed from the struct?
cpqphp:
-
The driver spawns a kthread cpqhp_event_thread() which is woken by the hardirq handler cpqhp_ctrl_intr(). Convert this to threaded IRQ handling. The kthread is also woken from the timer pushbutton_helper_thread(), convert it to call irq_wake_thread(). Use pciehp as a template.
-
A large portion of cpqphp_ctrl.c and cpqphp_pci.c concerns resource management. Doesn't this duplicate functionality in the core?
ibmphp:
-
Implementations of hotplug_slot_ops callbacks such as get_adapter_present() in ibmphp_core.c create a copy of the struct slot on the stack, then perform the actual operation on that copy. Determine if this overhead is necessary, delete it if not. The functions also perform a NULL pointer check on the struct hotplug_slot, this seems superfluous.
-
Several functions access the pci_slot member in struct hotplug_slot even though pci_hotplug.h declares it private. See get_max_bus_speed() for an example. Either the pci_slot member should no longer be declared private or ibmphp should store a pointer to its bus in struct slot. Probably the former.
-
ibmphp_init_devno() takes a struct slot **, it could instead take a struct slot *.
-
The return value of pci_hp_register() is not checked.
-
The various slot data structures are difficult to follow and need to be simplified. A lot of functions are too large and too complex, they need to be broken up into smaller, manageable pieces. Negative examples are ebda_rsrc_controller() and configure_bridge().
-
A large portion of ibmphp_res.c and ibmphp_pci.c concerns resource management. Doesn't this duplicate functionality in the core?
sgi_hotplug:
- Several functions access the pci_slot member in struct hotplug_slot even though pci_hotplug.h declares it private. See sn_hp_destroy() for an example. Either the pci_slot member should no longer be declared private or sgi_hotplug should store a pointer to it in struct slot. Probably the former.
shpchp:
-
There is only a single implementation of struct hpc_ops. Can the struct be removed and its functions invoked directly? This has already been done in pciehp with commit
82a9e79ef1("PCI: pciehp: remove hpc_ops"). Clarify if there was a specific reason not to apply the same change to shpchp. -
The hardirq handler shpc_isr() queues events on a workqueue. It can be simplified by converting it to threaded IRQ handling. Use pciehp as a template.