On Sat, Apr 27, 2019 at 10:03:01PM +0200, Lukas Wunner wrote:
> On Sat, Apr 27, 2019 at 02:13:02PM -0500, fred@xxxxxxxxxxxx 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, "...");
