netdev - Re: [PATCH iwl-next v2 1/3] ice: Extend Sideband Queue command to support dynamic flag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c3eadbc5-2585-4154-a429-d3b16726020b@intel.com>
Date: Thu, 23 May 2024 17:43:29 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Anil Samal <anil.samal@...el.com>, <intel-wired-lan@...ts.osuosl.org>
CC: <netdev@...r.kernel.org>, <--cc=jesse.brandeburg@...el.com>,
	<--cc=leszek.pepiak@...el.com>, <--cc=przemyslaw.kitszel@...el.com>,
	<--cc=lukasz.czapnik@...el.com>, <--cc=anthony.l.nguyen@...el.com>, "Anthony
 L Nguyen" <anthony.l.nguyen@...el.com>, Jesse Brandeburg
	<jesse.brandeburg@...el.com>
Subject: Re: [PATCH iwl-next v2 1/3] ice: Extend Sideband Queue command to
 support dynamic flag



On 5/9/2024 11:50 PM, Anil Samal wrote:
> Current driver implementation for Sideband Queue supports a
> fixed flag (ICE_AQ_FLAG_RD). To retrieve FEC statistics from
> firmware, Sideband Queue command is used with a different flag.
> 
> Extend API for Sideband Queue command to use 'flag' as input
> argument.
> 
> Reviewed-by: Anthony L Nguyen <anthony.l.nguyen@...el.com>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
> Signed-off-by: Anil Samal <anil.samal@...el.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_common.c |  5 +++--
>  drivers/net/ethernet/intel/ice/ice_common.h |  2 +-
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 16 ++++++++--------
>  3 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index 5649b257e631..9a0a533613ff 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -1473,8 +1473,9 @@ ice_sbq_send_cmd(struct ice_hw *hw, struct ice_sbq_cmd_desc *desc,
>   * ice_sbq_rw_reg - Fill Sideband Queue command
>   * @hw: pointer to the HW struct
>   * @in: message info to be filled in descriptor
> + * @flag: flag to fill desc structure

I would say:

@flags: control queue descriptor flags

>   */
> -int ice_sbq_rw_reg(struct ice_hw *hw, struct ice_sbq_msg_input *in)
> +int ice_sbq_rw_reg(struct ice_hw *hw, struct ice_sbq_msg_input *in, u16 flag)
>  {
>  	struct ice_sbq_cmd_desc desc = {0};
>  	struct ice_sbq_msg_req msg = {0};
> @@ -1498,7 +1499,7 @@ int ice_sbq_rw_reg(struct ice_hw *hw, struct ice_sbq_msg_input *in)
>  		 */
>  		msg_len -= sizeof(msg.data);
>  
> -	desc.flags = cpu_to_le16(ICE_AQ_FLAG_RD);
> +	desc.flags = cpu_to_le16(flag);

IMHO it makes more sense to call this "flags" instead of flag, since you
could pass arbitrary flags not just one flag.

Additionally, this patch did not apply cleanly to next-queue, due to
conflicts with other work on the tree.

Would you mind renaming this field and rebasing this series?

Thanks,
Jake