On Thu, May 02, 2019 at 10:59:44PM -0500, Frederick Lawler wrote:
> dbg() and ctrl_dbg() requires pciehp_debug module parameter to be set
> for debug log purposes. There are niche situations in pciehp_hpc.c where
> pciehp_debug is used: dbg_ctrl(), and pci_bus_check_dev().
>
> Enabling CONFIG_DYNAMIC_DEBUG/DEBUG is well known for logging debug
> information. Therefore, prefer pr/pci_dbg() for debug information, and
> reserve pciehp_debug for niche situations.
I guess by "niche situations", you mean the tests in
pci_bus_check_dev() and dbg_ctrl()?
I think pci_bus_check_dev() could skip the test and just use pr_dbg().
For dbg_ctrl(), we could just remove the test (and maybe reformat
those strings so they match other messages). That's a one-time thing
so I don't think two extra config reads that we might not print are a
problem.
I'd probably reorder these so you have five patches (or maybe more if
I missed something):
- remove pciehp_debug uses from pci_bus_check_dev() and dbg_ctrl()
- convert pciehp_debug to dyndbg
- convert "dev_*(&ctrl->pcie->device)" to "pci_*(ctrl->pcie->port)"
- add "Slot(%s)" to wrappers (could this be done with dev_fmt?)
- remove unused wrappers
> Signed-off-by: Frederick Lawler <fred@xxxxxxxxxxxx>
> ---
> drivers/pci/hotplug/pciehp.h | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index 78325c8d961e..e852aa478802 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -32,10 +32,7 @@ extern int pciehp_poll_time;
> extern bool pciehp_debug;
>
> #define dbg(format, arg...) \
> -do { \
> - if (pciehp_debug) \
> - pr_info(format, ## arg); \
> -} while (0)
> + pr_debug(format, ## arg);
> #define err(format, arg...) \
> pr_err(format, ## arg)
> #define info(format, arg...) \
> @@ -44,11 +41,7 @@ do { \
> pr_warn(format, ## arg)
>
> #define ctrl_dbg(ctrl, format, arg...) \
> - do { \
> - if (pciehp_debug) \
> - pci_info(ctrl->pcie->port, \
> - format, ## arg); \
> - } while (0)
> + pci_dbg(ctrl->pcie->port, format, ## arg)
> #define ctrl_err(ctrl, format, arg...) \
> pci_err(ctrl->pcie->port, format, ## arg)
> #define ctrl_info(ctrl, format, arg...) \
> --
> 2.17.1
>
