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

[Rafael J. Wysocki]

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
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> ---
>  drivers/pci/hotplug/acpiphp_core.c |  4 ----
>  drivers/pci/hotplug/acpiphp_glue.c | 23 +++++++++++++++++++++--
>  2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
> index c02257f4b61c..19d47607d009 100644
> --- a/drivers/pci/hotplug/acpiphp_core.c
> +++ b/drivers/pci/hotplug/acpiphp_core.c
> @@ -282,8 +282,6 @@ int acpiphp_register_hotplug_slot(struct acpiphp_slot *acpiphp_slot,
>                 goto error_slot;
>         }
>
> -       pr_info("Slot [%s] registered\n", slot_name(slot));
> -
>         return 0;
>  error_slot:
>         kfree(slot);
> @@ -296,8 +294,6 @@ void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *acpiphp_slot)
>  {
>         struct slot *slot = acpiphp_slot->slot;
>
> -       pr_info("Slot [%s] unregistered\n", slot_name(slot));
> -
>         pci_hp_deregister(&slot->hotplug_slot);
>         kfree(slot);
>  }
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 328d1e416014..eeca2753a5c7 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -25,7 +25,7 @@
>   *    bus. It loses the refcount when the driver unloads.
>   */
>
> -#define pr_fmt(fmt) "acpiphp_glue: " fmt
> +#define pr_fmt(fmt) "acpiphp: " fmt
>
>  #include <linux/module.h>
>
> @@ -333,6 +333,12 @@ static acpi_status acpiphp_add_context(acpi_handle handle, u32 lvl, void *data,
>                                        &val, 60*1000))
>                 slot->flags |= SLOT_ENABLED;
>
> +       if (slot->slot)
> +               pr_info("pci %04x:%02x:%02x Slot(%s) registered (%s)\n",
> +                       pci_domain_nr(slot->bus), slot->bus->number,
> +                       slot->device, slot_name(slot->slot),
> +                       slot->flags & SLOT_ENABLED ? "enabled" : "empty");
> +
>         return AE_OK;
>  }
>
> @@ -351,8 +357,13 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
>                         acpi_unlock_hp_context();
>                 }
>                 slot->flags |= SLOT_IS_GOING_AWAY;
> -               if (slot->slot)
> +               if (slot->slot) {
> +                       pr_info("pci %04x:%02x:%02x Slot(%s) unregistered\n",
> +                               pci_domain_nr(slot->bus), slot->bus->number,
> +                               slot->device, slot_name(slot->slot));
> +
>                         acpiphp_unregister_hotplug_slot(slot);
> +               }
>         }
>
>         mutex_lock(&bridge_mutex);
> @@ -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.

>         switch (type) {
>         case ACPI_NOTIFY_BUS_CHECK:
>                 /* bus re-enumerate */
> --