Re: [Intel-wired-lan] [PATCH v2 iwl-next 07/10] idpf: add Tx timestamp capabilities negotiation

[Simon Horman <horms@kernel.org>]

On Tue, Nov 26, 2024 at 04:58:53AM +0100, Milena Olech wrote:
> Tx timestamp capabilities are negotiated for the uplink Vport.
> Driver receives information about the number of available Tx timestamp
> latches, the size of Tx timestamp value and the set of indexes used
> for Tx timestamping.
> 
> Add function to get the Tx timestamp capabilities and parse the uplink
> vport flag.
> 
> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> Co-developed-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> Signed-off-by: Milena Olech <milena.olech@intel.com>
> ---
> v1 -> v2: change the idpf_for_each_vport macro

Hi Milena,

Some minor nits from my side.

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> index e7ccdcbdbd47..057d1c546417 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> @@ -83,6 +83,70 @@ struct idpf_ptp_secondary_mbx {
>  	bool valid:1;
>  };
>  
> +/**
> + * enum idpf_ptp_tx_tstamp_state - Tx timestamp states
> + * @IDPF_PTP_FREE: Tx timestamp index free to use
> + * @IDPF_PTP_REQUEST: Tx timestamp index set to the Tx descriptor
> + * @IDPF_PTP_READ_VALUE: Tx timestamp value ready to be read
> + */
> +enum idpf_ptp_tx_tstamp_state {
> +	IDPF_PTP_FREE,
> +	IDPF_PTP_REQUEST,
> +	IDPF_PTP_READ_VALUE,
> +};
> +
> +/**
> + * struct idpf_ptp_tx_tstamp_status - Parameters to track Tx timestamp
> + * @skb: the pointer to the SKB that received the completion tag
> + * @state: the state of the Tx timestamp
> + */
> +struct idpf_ptp_tx_tstamp_status {
> +	struct sk_buff *skb;
> +	enum idpf_ptp_tx_tstamp_state state;
> +};
> +
> +/**
> + * struct idpf_ptp_tx_tstamp - Parameters for Tx timestamping
> + * @list_member: the list member strutcure

nit: structure

     Flagged by checkpatch.pl --codespell

> + * @tx_latch_reg_offset_l: Tx tstamp latch low register offset
> + * @tx_latch_reg_offset_h: Tx tstamp latch high register offset
> + * @skb: the pointer to the SKB for this timestamp request
> + * @tstamp: the Tx tstamp value
> + * @idx: the index of the Tx tstamp
> + */
> +struct idpf_ptp_tx_tstamp {
> +	struct list_head list_member;
> +	u32 tx_latch_reg_offset_l;
> +	u32 tx_latch_reg_offset_h;
> +	struct sk_buff *skb;
> +	u64 tstamp;
> +	u32 idx;
> +};

...

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c

...

> @@ -3154,6 +3157,14 @@ void idpf_vport_init(struct idpf_vport *vport, struct idpf_vport_max_q *max_q)
>  	idpf_vport_alloc_vec_indexes(vport);
>  
>  	vport->crc_enable = adapter->crc_enable;
> +
> +	if (!(vport_msg->vport_flags &
> +	      le16_to_cpu(VIRTCHNL2_VPORT_UPLINK_PORT)))

I think this should be cpu_to_le16.

Flagged by Sparse.

> +		return;
> +
> +	err = idpf_ptp_get_vport_tstamps_caps(vport);
> +	if (err)
> +		pci_dbg(vport->adapter->pdev, "Tx timestamping not supported\n");
>  }
>  
>  /**

...

> diff --git a/drivers/net/ethernet/intel/idpf/virtchnl2.h b/drivers/net/ethernet/intel/idpf/virtchnl2.h
> index 44a5ee84ed60..fdeebc621bdb 100644
> --- a/drivers/net/ethernet/intel/idpf/virtchnl2.h
> +++ b/drivers/net/ethernet/intel/idpf/virtchnl2.h
> @@ -569,6 +569,14 @@ struct virtchnl2_queue_reg_chunks {
>  };
>  VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_queue_reg_chunks);
>  
> +/**
> + * enum virtchnl2_vport_flags - Vport flags that indicate vport capabilities.
> + * @VIRTCHNL2_VPORT_UPLINK_PORT: Representatives of underlying physical ports
> + */
> +enum virtchnl2_vport_flags {
> +	VIRTCHNL2_VPORT_UPLINK_PORT	= BIT(0),
> +};
> +
>  /**
>   * struct virtchnl2_create_vport - Create vport config info.
>   * @vport_type: See enum virtchnl2_vport_type.
> @@ -620,7 +628,7 @@ struct virtchnl2_create_vport {
>  	__le16 max_mtu;
>  	__le32 vport_id;
>  	u8 default_mac_addr[ETH_ALEN];
> -	__le16 pad;
> +	__le16 vport_flags;

The kernel doc for this structure, which is immediately above the
structure, should also be updated.

Flagged by ./scripts/kernel-doc -none

>  	__le64 rx_desc_ids;
>  	__le64 tx_desc_ids;
>  	u8 pad1[72];
> -- 
> 2.31.1
> 
>