Re: [PATCH v2 9/9] PCI: hotplug: Prefix ctrl_*() dmesg logs with pciehp slot name

Bjorn Helgaas
2019-05-09T14:03:30Z

On Thu, May 02, 2019 at 10:59:46PM -0500, Frederick Lawler wrote:
> Remove current uses of "Slot(%s)" and then prefix ctrl_*() dmesg
> with pciehp slot name to include the slot name for all uses of ctrl_*()
> wrappers.
>
> Signed-off-by: Frederick Lawler <fred@fredlawl.com>
> ---
> drivers/pci/hotplug/pciehp.h | 12 ++++---
> drivers/pci/hotplug/pciehp_core.c | 9 +++--
> drivers/pci/hotplug/pciehp_ctrl.c | 58 ++++++++++++-------------------
> drivers/pci/hotplug/pciehp_hpc.c | 5 ++-
> 4 files changed, 38 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index 06ff9d31405e..e1cdc3565c62 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -32,13 +32,17 @@ extern int pciehp_poll_time;
> extern bool pciehp_debug;
>
> #define ctrl_dbg(ctrl, format, arg...) \
> - pci_dbg(ctrl->pcie->port, format, ## arg)
> + pci_dbg(ctrl->pcie->port, "Slot(%s): " format, \
> + slot_name(ctrl), ## arg)

This would be nice to do, but given the current code organization, I don't
think it's actually feasible to use slot_name() in these wrappers because
the slot name is initialized in init_slot(), but there are lots of places
we can emit messages before that, especially if debug is enabled:

pciehp_probe
pcie_init
dbg_ctrl
ctrl_info # no slot yet
ctrl_info("Slot #%d AttnBtn%c ...") # no slot yet
if (POWER_CTRL)
pciehp_get_power_status
ctrl_dbg("SLOTCTRL") # no slot yet
if (...)
pcie_disable_notification
pcie_write_cmd
pcie_do_write_cmd
ctrl_info("no response") # no slot yet
ctrl_dbg("SLOTCTRL") # no slot yet
pciehp_power_off_slot
ctrl_dbg("SLOTCTRL") # no slot yet
init_slot # slot valid after this returns