Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 8 Aug 2023 15:58:00 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     "Rafael J. Wysocki" <rafael@kernel.org>
Cc:     linux-pci@vger.kernel.org, Len Brown <lenb@kernel.org>,
        Igor Mammedov <imammedo@redhat.com>,
        "Michael S . Tsirkin" <mst@redhat.com>,
        Woody Suwalski <terraluna977@gmail.com>,
        linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
        Bjorn Helgaas <bhelgaas@google.com>
Subject: Re: [PATCH] PCI: acpiphp: Log more slot and notification details
Message-ID: <20230808205800.GA332785@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJZ5v0jc5dn+6WtH6O30EeJfGDLewiLaAY9YJEAO6d_n+Uv7ig@mail.gmail.com>
Precedence: bulk

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@kernel.org> wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > 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?

Bjorn