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

[Bjorn Helgaas]

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@xxxxxxxxxxxx>
> ---
>  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