On Tue, Aug 20, 2024 at 12:21:51PM +0200, Karol Kolacinski wrote:
> 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 <[email protected]>
> Signed-off-by: Karol Kolacinski <[email protected]>
...
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index cf3b02d14b19..861f6224540a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2760,6 +2760,65 @@ enum ice_tx_tstamp_work ice_ptp_process_ts(struct
> ice_pf *pf)
> }
> }
>
> +/**
> + * ice_ptp_ts_irq - Process the PTP Tx timestamps in IRQ context
> + * @pf: Board private structure
> + *
> + * Return: IRQ_WAKE_THREAD if Tx timestamp read has to be handled in the
> bottom
> + * 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->mac_type) {
> + case ICE_MAC_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;
> + u8 idx;
> +
> + if (!ice_pf_state_is_nominal(pf))
> + return IRQ_HANDLED;
> +
> + spin_lock(&tx->lock);
> + 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(&tx->lock);
> +
> + return IRQ_HANDLED;
> + }
> + fallthrough; /* non-LL_TS E810 */
> + case ICE_MAC_GENERIC:
> + case ICE_MAC_GENERIC_3K_E825:
> + /* 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_MAC_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;
I think it would be better to split out adding E830 support into a separate
patch, leaving this patch as strictly refactoring of existing code.
> + default:
> + return IRQ_HANDLED;
> + }
> +}
> +
> /**
> * ice_ptp_maybe_trigger_tx_interrupt - Trigger Tx timstamp interrupt
> * @pf: Board private structure
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h
> b/drivers/net/ethernet/intel/ice/ice_ptp.h
...
> @@ -360,6 +361,11 @@ static inline bool ice_ptp_process_ts(struct ice_pf *pf)
> return true;
> }
>
> +static inline irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
> +{
> + return IRQ_HANDLED;
> +}
This no-op implementation is in effect if CONFIG_PTP_1588_CLOCK is not
enabled. Whereas previously the fuller implementation would have run.
I think that deserves some coverage in the patch description.
> +
> static inline u64
> ice_ptp_get_rx_hwts(const union ice_32b_rx_flex_desc *rx_desc,
> const struct ice_pkt_ctx *pkt_ctx)
> --
> 2.46.0
>
>
