lkml.org 
[lkml]   [2019]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/4] PCI: pciehp: Replace ctrl_*() with pci_*()
On Sat, Apr 27, 2019 at 10:03:01PM +0200, Lukas Wunner wrote:
> On Sat, Apr 27, 2019 at 02:13:02PM -0500, fred@fredlawl.com wrote:
> > Hotplug useses custom ctrl_*() dev_*() printk wrappers for logging
> > messages. To make hotplug conform to pci logging, replace uses of these
> > wrappers with pci_*() printk wrappers. In addition, replace any
> > printk() calls with pr_*() wrappers.
>
> A lot of pciehp's messages are preceded by "Slot(%s): ", where %s is
> replaced by the Physical Slot Number in the Slot Capabilities register
> (which is cached in struct controller) plus an optional suffix if the
> same PSN is used by multiple slots. For some reason (probably a
> historic artefact), this prefix is included only in *some* of the
> messages.
>
> I think it would be useful to make the messages consistent by *always*
> including the "Slot(%s): " prefix. However the prefix is unknown until
> pci_hp_initialize() has been called. I'd solve this by keeping the
> ctrl_*() wrappers and amending them to print the "Slot(%s): " prefix,
> then making sure that ctrl_*() is not called before pci_hp_initialize().
> (pci_*() has to be used instead).

I was hoping to get rid of the ctrl_*() wrappers completely, but the
"Slot(%s)" prefix is actually a pretty good reason to keep them.

Moving the prefix from the call site to the ctrl_*() wrappers should be a
separate patch that doesn't change the output at all (except maybe
adding the prefix to messages that don't currently include it).

So you probably need three steps (each in a separate patch):

1) Convert ctrl_*() to use pci_dev instead of pcie_device, something
like this:

+ #define pr_fmt(fmt) "pciehp: " fmt
+ #define dev_fmt pr_fmt

#define ctrl_info(ctrl, format, arg...) \
- dev_info(&ctrl->pcie->device, format, ## arg)
+ pci_info(&ctrl->pcie->port, format, ## arg)

2) Convert any output before pci_hp_initialize() from ctrl_*() to
pci_*().

3) Centralize the "Slot(%s): " prefix, something like this:

#define ctrl_info(ctrl, format, arg...) \
- pci_info(&ctrl->pcie->port, format, ## arg)
+ pci_info(&ctrl->pcie->port, "Slot(%s): " format, slot_name(ctrl), ## arg)

- ctrl_info(ctrl, "Slot(%s): ...", slot_name(ctrl));
+ ctrl_info(ctrl, "...");

\
 
 \ /
  Last update: 2019-04-29 02:37    [W:0.061 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site