From: Karol Kolacinski <karol.kolacin...@intel.com>
Date: Thu, 25 Jul 2024 11:34:49 +0200
> Simplify TSYN IRQ processing by moving it to a separate function and
> having appropriate behavior per PHY model, instead of multiple
> conditions not related to HW, but to specific timestamping modes.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kits...@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacin...@intel.com>
[...]
> +/**
> + * ice_ptp_ts_irq - Process the PTP Tx timestamps in IRQ context
> + * @pf: Board private structure
> + *
> + * Returns: IRQ_WAKE_THREAD if Tx timestamp read has to be handled in the
> bottom
In the previous commit, you used 'Return:', here you have 'Returns:'.
Perhaps you may want to keep the style consistent :)
> + * half of the interrupt and IRQ_HANDLED otherwise.
> + */
> +irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
> +{
> + struct ice_hw *hw = &pf->hw;
> +
> + switch (hw->ptp.phy_model) {
> + case ICE_PHY_E810:
> + /* E810 capable of low latency timestamping with interrupt can
> + * request a single timestamp in the top half and wait for
> + * a second LL TS interrupt from the FW when it's ready.
> + */
> + if (hw->dev_caps.ts_dev_info.ts_ll_int_read) {
> + struct ice_ptp_tx *tx = &pf->ptp.port.tx;
> + unsigned long flags;
> + u8 idx;
> +
> + if (!ice_pf_state_is_nominal(pf))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(&tx->lock, flags);
It's top half, right? If so, you don't need _irqsave versions, just
spin_lock/unlock().
_irqsave/restore are only to be used in either BH or process context to
temporarily disable interrupts to avoid concurrent access with TH (e.g.
this block).
> + idx = find_next_bit_wrap(tx->in_use, tx->len,
> + tx->last_ll_ts_idx_read + 1);
> + if (idx != tx->len)
> + ice_ptp_req_tx_single_tstamp(tx, idx);
> + spin_unlock_irqrestore(&tx->lock, flags);
> +
> + return IRQ_HANDLED;
> + }
> + fallthrough; /* non-LL_TS E810 */
> + case ICE_PHY_E82X:
> + case ICE_PHY_ETH56G:
> + /* All other devices process timestamps in the bottom half due
> + * to sleeping or polling.
> + */
> + if (!ice_ptp_pf_handles_tx_interrupt(pf))
> + return IRQ_HANDLED;
> +
> + set_bit(ICE_MISC_THREAD_TX_TSTAMP, pf->misc_thread);
> + return IRQ_WAKE_THREAD;
> + case ICE_PHY_E830:
> + /* E830 can read timestamps in the top half using rd32() */
> + if (ice_ptp_process_ts(pf) == ICE_TX_TSTAMP_WORK_PENDING) {
> + /* Process outstanding Tx timestamps. If there
> + * is more work, re-arm the interrupt to trigger again.
> + */
> + wr32(hw, PFINT_OICR, PFINT_OICR_TSYN_TX_M);
> + ice_flush(hw);
> + }
> + return IRQ_HANDLED;
> + default:
> + return IRQ_HANDLED;
> + }
> +}
[...]
Thanks,
Olek
