On Wed, 15 Jan 2025 09:37:53 +0800
Shuai Xue <[email protected]> wrote:
> +#define CREATE_TRACE_POINTS
> +#include "trace.h"
> +
> /* The following routines constitute the bulk of the
> hotplug controller logic
> */
> @@ -244,12 +247,20 @@ void pciehp_handle_presence_or_link_change(struct
> controller *ctrl, u32 events)
> case ON_STATE:
> ctrl->state = POWEROFF_STATE;
> mutex_unlock(&ctrl->state_lock);
> - if (events & PCI_EXP_SLTSTA_DLLSC)
> + if (events & PCI_EXP_SLTSTA_DLLSC) {
> ctrl_info(ctrl, "Slot(%s): Link Down\n",
> slot_name(ctrl));
> - if (events & PCI_EXP_SLTSTA_PDC)
> + trace_pci_hp_event(pci_name(ctrl->pcie->port),
> + slot_name(ctrl),
> + PCI_HOTPLUG_LINK_DOWN);
Hmm, can't you just pass in the ctrl pointer to the tracepoint?
trace_pci_hp_event(ctrl, PCI_HOTPLUG_LINK_DOWN);
> + }
> + if (events & PCI_EXP_SLTSTA_PDC) {
> ctrl_info(ctrl, "Slot(%s): Card not present\n",
> slot_name(ctrl));
> + trace_pci_hp_event(pci_name(ctrl->pcie->port),
> + slot_name(ctrl),
> + PCI_HOTPLUG_CARD_NOT_PRESENT);
> + }
> pciehp_disable_slot(ctrl, SURPRISE_REMOVAL);
> break;
> default:
> @@ -269,6 +280,9 @@ void pciehp_handle_presence_or_link_change(struct
> controller *ctrl, u32 events)
> INDICATOR_NOOP);
> ctrl_info(ctrl, "Slot(%s): Card not present\n",
> slot_name(ctrl));
> + trace_pci_hp_event(pci_name(ctrl->pcie->port),
> + slot_name(ctrl),
> + PCI_HOTPLUG_CARD_NOT_PRESENT);
> }
> mutex_unlock(&ctrl->state_lock);
> return;
> @@ -281,12 +295,19 @@ void pciehp_handle_presence_or_link_change(struct
> controller *ctrl, u32 events)
> case OFF_STATE:
> ctrl->state = POWERON_STATE;
> mutex_unlock(&ctrl->state_lock);
> - if (present)
> + if (present) {
> ctrl_info(ctrl, "Slot(%s): Card present\n",
> slot_name(ctrl));
> - if (link_active)
> - ctrl_info(ctrl, "Slot(%s): Link Up\n",
> - slot_name(ctrl));
> + trace_pci_hp_event(pci_name(ctrl->pcie->port),
> + slot_name(ctrl),
> + PCI_HOTPLUG_CARD_PRESENT);
> + }
> + if (link_active) {
> + ctrl_info(ctrl, "Slot(%s): Link Up\n", slot_name(ctrl));
> + trace_pci_hp_event(pci_name(ctrl->pcie->port),
> + slot_name(ctrl),
> + PCI_HOTPLUG_LINK_UP);
> + }
> ctrl->request_result = pciehp_enable_slot(ctrl);
> break;
> default:
> diff --git a/drivers/pci/hotplug/trace.h b/drivers/pci/hotplug/trace.h
> new file mode 100644
> index 000000000000..1415ac505cb5
> --- /dev/null
> +++ b/drivers/pci/hotplug/trace.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#if !defined(_TRACE_HW_EVENT_PCI_HP_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_HW_EVENT_PCI_HP_H
> +
> +#include <linux/tracepoint.h>
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM pci
> +
> +TRACE_EVENT(pci_hp_event,
> +
> + TP_PROTO(const char *port_name,
> + const char *slot,
> + const int event),
> +
> + TP_ARGS(port_name, slot, event),
> +
> + TP_STRUCT__entry(
> + __string( port_name, port_name )
> + __string( slot, slot )
> + __field( int, event )
Then the above would be:
TP_PROTO(struct controller *ctrl, int event),
// don't really need a const int there
TP_ARGS(ctrl, event),
TP_STRUCT__entry(
__string( port_name, pci_name(ctrl->pcie->port)
)
__string( slot, slot_name(ctrl)
)
__field( int, event
)
and everything else could be the same.
-- Steve
> + ),
> +
> + TP_fast_assign(
> + __assign_str(port_name);
> + __assign_str(slot);
> + __entry->event = event;
> + ),
> +
> + TP_printk("%s slot:%s, event:%s\n",
> + __get_str(port_name),
> + __get_str(slot),
> + __print_symbolic(__entry->event, PCI_HOTPLUG_EVENT)
> + )
> +);
