Re: [PATCH v4 3/3] PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 24 Feb 2026 09:11:15 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> > +#ifdef CONFIG_TRACING
> > +static void rockchip_pcie_ltssm_trace_work(struct work_struct *work)
> > +{
> > +	struct rockchip_pcie *rockchip = container_of(work,
> > +						struct rockchip_pcie,
> > +						trace_work.work);
> > +	struct dw_pcie *pci = &rockchip->pci;
> > +	enum dw_pcie_ltssm state;
> > +	u32 i, l1ss, prev_val = DW_PCIE_LTSSM_UNKNOWN, rate, val;
> > +
> > +	if (!pci_ltssm_tp_enabled())
> > +		goto skip_trace;  
> 
> You can use:
> 
> 	if (!trace_pcie_ltssm_state_transition_enabled())
> 		goto skip_trace;
> 
> The above is a static branch. That means when tracing is disabled, it is
> basically:
> 
> 	goto skip_trace;
> 
> and when tracing is enabled it is a nop.
> 
> -- Steve
> 
> 
> > +
> > +	for (i = 0; i < PCIE_DBG_LTSSM_HISTORY_CNT; i++) {
> > +		val = rockchip_pcie_readl_apb(rockchip,
> > +				PCIE_CLIENT_DBG_FIFO_STATUS);
> > +		rate = FIELD_GET(PCIE_DBG_FIFO_RATE_MASK, val);
> > +		l1ss = FIELD_GET(PCIE_DBG_FIFO_L1SUB_MASK, val);
> > +		val = FIELD_GET(PCIE_LTSSM_STATUS_MASK, val);
> > +
> > +		/*
> > +		 * Hardware Mechanism: The ring FIFO employs two tracking
> > +		 * counters:
> > +		 * - 'last-read-point': maintains the user's last read position
> > +		 * - 'last-valid-point': tracks the HW's last state update
> > +		 *
> > +		 * Software Handling: When two consecutive LTSSM states are
> > +		 * identical, it indicates invalid subsequent data in the FIFO.
> > +		 * In this case, we skip the remaining entries. The dual counter
> > +		 * design ensures that on the next state transition, reading can
> > +		 * resume from the last user position.
> > +		 */
> > +		if ((i > 0 && val == prev_val) || val > DW_PCIE_LTSSM_RCVRY_EQ3)
> > +			break;
> > +
> > +		state = prev_val = val;
> > +		if (val == DW_PCIE_LTSSM_L1_IDLE) {
> > +			if (l1ss == 2)
> > +				state = DW_PCIE_LTSSM_L1_2;
> > +			else if (l1ss == 1)
> > +				state = DW_PCIE_LTSSM_L1_1;
> > +		}
> > +
> > +		trace_pcie_ltssm_state_transition(dev_name(pci->dev),
> > +				dw_pcie_ltssm_status_string(state),
> > +				((rate + 1) > pci->max_link_speed) ?
> > +				PCI_SPEED_UNKNOWN : PCIE_SPEED_2_5GT + rate);
> > +	}
> > +
> > +skip_trace:
> > +	schedule_delayed_work(&rockchip->trace_work, msecs_to_jiffies(5000));
> > +}
> > +

Hmm, so basically you only want to call the work when tracing is
enabled? That's what I was thinking should be enabled by the reg and
unreg functions. That is, the reg should enabled the delayed work, and
the unreg should disable it from being called.

This looks like it calls the work regardless of if tracing is enabled
or not. Why waste the cycles when tracing is disabled?

-- Steve