[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <401c13fc-9fbb-a126-f05a-6468a563404a@amd.com>
Date: Wed, 14 Jun 2023 14:23:58 -0700
From: Brett Creeley <bcreeley@....com>
To: Dave Ertman <david.m.ertman@...el.com>, intel-wired-lan@...ts.osuosl.org
Cc: daniel.machon@...rochip.com, simon.horman@...igine.com,
netdev@...r.kernel.org
Subject: Re: [PATCH iwl-next v4 02/10] ice: Add driver support for firmware
changes for LAG
On 6/9/2023 2:16 PM, Dave Ertman wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> Add the defines, fields, and detection code for FW support of LAG for
> SRIOV. Also exposes some previously static functions to allow access
> in the lag code.
>
> Clean up code that is unused or not needed for LAG support. Also add
> an ordered workqueue for processing LAG events.
>
> Reviewed-by: Daniel Machon <daniel.machon@...rochip.com>
> Signed-off-by: Dave Ertman <david.m.ertman@...el.com>
> ---
> drivers/net/ethernet/intel/ice/ice.h | 5 ++
> .../net/ethernet/intel/ice/ice_adminq_cmd.h | 3 ++
> drivers/net/ethernet/intel/ice/ice_common.c | 8 +++
> drivers/net/ethernet/intel/ice/ice_lag.c | 53 ++++++++++---------
> drivers/net/ethernet/intel/ice/ice_lib.c | 2 +-
> drivers/net/ethernet/intel/ice/ice_lib.h | 1 +
> drivers/net/ethernet/intel/ice/ice_main.c | 12 +++++
> drivers/net/ethernet/intel/ice/ice_type.h | 2 +
> 8 files changed, 59 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 9109006336f0..5ac0ad12f9f1 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -200,6 +200,8 @@ enum ice_feature {
> ICE_F_PTP_EXTTS,
> ICE_F_SMA_CTRL,
> ICE_F_GNSS,
> + ICE_F_ROCE_LAG,
> + ICE_F_SRIOV_LAG,
> ICE_F_MAX
> };
>
> @@ -569,6 +571,7 @@ struct ice_pf {
> struct mutex sw_mutex; /* lock for protecting VSI alloc flow */
> struct mutex tc_mutex; /* lock to protect TC changes */
> struct mutex adev_mutex; /* lock to protect aux device access */
> + struct mutex lag_mutex; /* protect ice_lag struct in PF */
> u32 msg_enable;
> struct ice_ptp ptp;
> struct gnss_serial *gnss_serial;
> @@ -639,6 +642,8 @@ struct ice_pf {
> struct ice_agg_node vf_agg_node[ICE_MAX_VF_AGG_NODES];
> };
>
> +extern struct workqueue_struct *ice_lag_wq;
> +
> struct ice_netdev_priv {
> struct ice_vsi *vsi;
> struct ice_repr *repr;
> diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> index 6ea0d4c017f0..6b5ba9a02bad 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -120,6 +120,9 @@ struct ice_aqc_list_caps_elem {
> #define ICE_AQC_CAPS_PCIE_RESET_AVOIDANCE 0x0076
> #define ICE_AQC_CAPS_POST_UPDATE_RESET_RESTRICT 0x0077
> #define ICE_AQC_CAPS_NVM_MGMT 0x0080
> +#define ICE_AQC_CAPS_FW_LAG_SUPPORT 0x0092
> +#define ICE_AQC_BIT_ROCEV2_LAG 0x01
> +#define ICE_AQC_BIT_SRIOV_LAG 0x02
>
> u8 major_ver;
> u8 minor_ver;
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index 09e2e38d538e..6ddf607d2edf 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -2241,6 +2241,14 @@ ice_parse_common_caps(struct ice_hw *hw, struct ice_hw_common_caps *caps,
> "%s: reset_restrict_support = %d\n", prefix,
> caps->reset_restrict_support);
> break;
> + case ICE_AQC_CAPS_FW_LAG_SUPPORT:
> + caps->roce_lag = !!(number & ICE_AQC_BIT_ROCEV2_LAG);
> + ice_debug(hw, ICE_DBG_INIT, "%s: roce_lag = %u\n",
> + prefix, caps->roce_lag);
> + caps->sriov_lag = !!(number & ICE_AQC_BIT_SRIOV_LAG);
> + ice_debug(hw, ICE_DBG_INIT, "%s: sriov_lag = %u\n",
> + prefix, caps->sriov_lag);
> + break;
> default:
> /* Not one of the recognized common capabilities */
> found = false;
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
> index 5a7753bda324..73bfc5cd8b37 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
> @@ -4,8 +4,12 @@
> /* Link Aggregation code */
>
> #include "ice.h"
> +#include "ice_lib.h"
> #include "ice_lag.h"
>
> +#define ICE_LAG_RES_SHARED BIT(14)
> +#define ICE_LAG_RES_VALID BIT(15)
> +
> /**
> * ice_lag_set_primary - set PF LAG state as Primary
> * @lag: LAG info struct
> @@ -225,6 +229,26 @@ static void ice_lag_unregister(struct ice_lag *lag, struct net_device *netdev)
> lag->role = ICE_LAG_NONE;
> }
>
> +/**
> + * ice_lag_check_nvm_support - Check for NVM support for LAG
> + * @pf: PF struct
> + */
> +static void ice_lag_check_nvm_support(struct ice_pf *pf)
Nit, but this name is a bit misleading to me. I would expect it to be
called something like "ice_lag_init_feature_support_flag()" or something
similar that better describes what the function is doing.
> +{
> + struct ice_hw_dev_caps *caps;
> +
> + caps = &pf->hw.dev_caps;
Nit, but since you are already creating a local variable you could go
one level further to the common_cap, so it could be:
common_cap->roce_lag and common_cap->sriov_lag
> + if (caps->common_cap.roce_lag)
> + ice_set_feature_support(pf, ICE_F_ROCE_LAG);
> + else
> + ice_clear_feature_support(pf, ICE_F_ROCE_LAG);
> +
> + if (caps->common_cap.sriov_lag)
> + ice_set_feature_support(pf, ICE_F_SRIOV_LAG);
> + else
> + ice_clear_feature_support(pf, ICE_F_SRIOV_LAG);
> +}
> +
[...]
Powered by blists - more mailing lists