Re: [PATCH] PCI: acpiphp: Log more slot and notification details

[Rafael J. Wysocki]

On Tue, Aug 8, 2023 at 10:58 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
> On Tue, Aug 08, 2023 at 09:39:22PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Aug 8, 2023 at 9:27 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> > > From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > >
> > > When registering an acpiphp slot, log the slot name in the same style as
> > > pciehp and include the PCI bus/device and whether a device is present or
> > > the slot is empty.
> > >
> > > When handling an ACPI notification, log the PCI bus/device and notification
> > > type.
> > >
> > > Sample dmesg log diff:
> > >
> > >     ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
> > >   - acpiphp: Slot [3] registered
> > >   - acpiphp: Slot [4] registered
> > >     PCI host bridge to bus 0000:00
> > >     pci 0000:00:03.0: [8086:100e] type 00 class 0x020000
> > >     <ACPI Device Check notification>
> > >     pci 0000:00:04.0: [8086:100e] type 00 class 0x020000
> > >
> > >     ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
> > >   + acpiphp: pci 0000:00:03 Slot(3) registered (enabled)
> > >   + acpiphp: pci 0000:00:04 Slot(4) registered (empty)
> > >     PCI host bridge to bus 0000:00
> > >     pci 0000:00:03.0: [8086:100e] type 00 class 0x020000
> > >     <ACPI Device Check notification>
> > >   + acpiphp: pci 0000:00:04 Slot(4) Device Check
> > >     pci 0000:00:04.0: [8086:100e] type 00 class 0x020000
> > > ...
>
> > > @@ -793,6 +804,14 @@ static void hotplug_event(u32 type, struct acpiphp_context *context)
> > >
> > >         pci_lock_rescan_remove();
> > >
> > > +       pr_info("pci %04x:%02x:%02x Slot(%s) %s\n",
> > > +               pci_domain_nr(slot->bus), slot->bus->number,
> > > +               slot->device, slot_name(slot->slot),
> > > +               type == ACPI_NOTIFY_BUS_CHECK ? "Bus Check" :
> > > +               type == ACPI_NOTIFY_DEVICE_CHECK ? "Device Check" :
> > > +               type == ACPI_NOTIFY_EJECT_REQUEST ? "Eject Request" :
> > > +               "Notification");
> >
> > pr_debug() perhaps?
> >
> > On systems that don't have any hotplug problems these messages will
> > just be filling the kernel log unnecessarily.
>
> If these notifications are really common, pr_debug() sounds like the
> right thing.  I assumed that they would not be common, e.g., they
> would happen for user-time things like dock/undock, plug/unplug,
> suspend/resume, etc.
>
> In pciehp, we use _info for attention button presses, presence detect
> changes, link up/down, and I assumed the ACPI notify events would
> roughly correspond to those.  No?

Depending on how often the system gets suspended and resumed, they may
end up in the log quite often and if there are no problems related to
them, they are just noise.

IMHO in that case the users are taught to ignore stuff that lands in
the log which is not fantastic.