diff mbox series

[iwl-next] ice: Disable Cage Max Power override

Message ID 20230822073452.28446-1-wojciech.drewek@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [iwl-next] ice: Disable Cage Max Power override | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1330 this patch: 1330
netdev/cc_maintainers warning 6 maintainers not CCed: kuba@kernel.org jesse.brandeburg@intel.com davem@davemloft.net anthony.l.nguyen@intel.com pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1353 this patch: 1353
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 80 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Wojciech Drewek Aug. 22, 2023, 7:34 a.m. UTC
NVM module called "Cage Max Power override" allows to
change max power in the cage. This can be achieved
using external tools. The responsibility of the ice driver is to
go back to the default settings whenever port split is done.
This is achieved by clearing Override Enable bit in the
NVM module. Override of the max power is disabled so the
default value will be used.

Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  9 +++++
 drivers/net/ethernet/intel/ice/ice_devlink.c  | 35 +++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_nvm.c      |  2 +-
 drivers/net/ethernet/intel/ice/ice_nvm.h      |  4 +++
 4 files changed, 49 insertions(+), 1 deletion(-)

Comments

Przemek Kitszel Aug. 22, 2023, 8:16 a.m. UTC | #1
On 8/22/23 09:34, Wojciech Drewek wrote:
> NVM module called "Cage Max Power override" allows to
> change max power in the cage. This can be achieved
> using external tools. The responsibility of the ice driver is to
> go back to the default settings whenever port split is done.
> This is achieved by clearing Override Enable bit in the
> NVM module. Override of the max power is disabled so the
> default value will be used.
> 
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
>   .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  9 +++++
>   drivers/net/ethernet/intel/ice/ice_devlink.c  | 35 +++++++++++++++++++
>   drivers/net/ethernet/intel/ice/ice_nvm.c      |  2 +-
>   drivers/net/ethernet/intel/ice/ice_nvm.h      |  4 +++
>   4 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> index ffbe9d3a5d77..a3a49d922650 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -1569,6 +1569,15 @@ struct ice_aqc_nvm {
>   	__le32 addr_low;
>   };
>   
> +#define ICE_AQC_NVM_CMPO_MOD_ID			0x153
> +
> +/* Cage Max Power override NVM module */
> +struct ice_aqc_nvm_cmpo {
> +	__le16 length;
> +#define ICE_AQC_NVM_CMPO_ENABLE	BIT(8)
> +	__le16 cages_cfg[8];

[1] here

> +};
> +
>   #define ICE_AQC_NVM_START_POINT			0
>   
>   /* NVM Checksum Command (direct, 0x0706) */
> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> index 80dc5445b50d..e9300df9ef40 100644
> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> @@ -591,6 +591,36 @@ static void ice_devlink_port_options_print(struct ice_pf *pf)
>   	kfree(options);
>   }
>   
> +#define ICE_NUM_OF_CAGES 8

perhaps move this define to ice_adminq_cmd.h to benefit from it in 
struct definition just few lines above [1]
> +
> +/**
> + * ice_devlink_aq_clear_cmpo - clear Cage Max Power override
> + * @hw: pointer to the HW struct
> + *
> + * Read Cage Max Power override NVM module, clear override
> + * enable bit for each of the cages. Write the settings back to
> + * the NVM.

Read+clear+write is "how" or algorithm here, but doc should just stick 
to "what" most of the time. So I would just:
"Clear Cage Max Power override enable bit for each of the cages".

"how" part could be inside the function, just above "read" call.

> + */
> +static int
> +ice_devlink_aq_clear_cmpo(struct ice_hw *hw)
> +{
> +	struct ice_aqc_nvm_cmpo data;
> +	int ret, i;
> +
> +	ret = ice_aq_read_nvm(hw, ICE_AQC_NVM_CMPO_MOD_ID, 0, sizeof(data),
> +			      &data, true, false, NULL);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < ICE_NUM_OF_CAGES; i++)
> +		data.cages_cfg[i] &= ~cpu_to_le16(ICE_AQC_NVM_CMPO_ENABLE);
> +
> +	/* Do not update the length word since it is not permitted */
> +	return ice_aq_update_nvm(hw, ICE_AQC_NVM_CMPO_MOD_ID, 2,
> +				 sizeof(data.cages_cfg), data.cages_cfg,
> +				 false, 0, NULL);
> +}
> +
>   /**
>    * ice_devlink_aq_set_port_option - Send set port option admin queue command
>    * @pf: the PF to print split port options
> @@ -623,6 +653,11 @@ ice_devlink_aq_set_port_option(struct ice_pf *pf, u8 option_idx,
>   		return -EIO;
>   	}
>   
> +	status = ice_devlink_aq_clear_cmpo(&pf->hw);
> +	if (status)
> +		dev_dbg(dev, "Failed to clear Cage Max Power override, err %d aq_err %d\n",
> +			status, pf->hw.adminq.sq_last_status);
> +
>   	status = ice_nvm_write_activate(&pf->hw, ICE_AQC_NVM_ACTIV_REQ_EMPR, NULL);
>   	if (status) {
>   		dev_dbg(dev, "ice_nvm_write_activate failed, err %d aq_err %d\n",
> diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
> index f6f52a248066..745f2459943f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_nvm.c
> +++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
> @@ -18,7 +18,7 @@
>    *
>    * Read the NVM using the admin queue commands (0x0701)
>    */
> -static int
> +int
>   ice_aq_read_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset, u16 length,
>   		void *data, bool last_command, bool read_shadow_ram,
>   		struct ice_sq_cd *cd)
> diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.h b/drivers/net/ethernet/intel/ice/ice_nvm.h
> index 774c2317967d..90f36e19e06b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_nvm.h
> +++ b/drivers/net/ethernet/intel/ice/ice_nvm.h
> @@ -15,6 +15,10 @@ struct ice_orom_civd_info {
>   int ice_acquire_nvm(struct ice_hw *hw, enum ice_aq_res_access_type access);
>   void ice_release_nvm(struct ice_hw *hw);
>   int
> +ice_aq_read_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset, u16 length,
> +		void *data, bool last_command, bool read_shadow_ram,
> +		struct ice_sq_cd *cd);
> +int
>   ice_read_flat_nvm(struct ice_hw *hw, u32 offset, u32 *length, u8 *data,
>   		  bool read_shadow_ram);
>   int
Wojciech Drewek Aug. 22, 2023, 9:39 a.m. UTC | #2
> -----Original Message-----
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Sent: wtorek, 22 sierpnia 2023 10:17
> To: Drewek, Wojciech <wojciech.drewek@intel.com>; intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH iwl-next] ice: Disable Cage Max Power override
> 
> On 8/22/23 09:34, Wojciech Drewek wrote:
> > NVM module called "Cage Max Power override" allows to
> > change max power in the cage. This can be achieved
> > using external tools. The responsibility of the ice driver is to
> > go back to the default settings whenever port split is done.
> > This is achieved by clearing Override Enable bit in the
> > NVM module. Override of the max power is disabled so the
> > default value will be used.
> >
> > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> > ---
> >   .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  9 +++++
> >   drivers/net/ethernet/intel/ice/ice_devlink.c  | 35 +++++++++++++++++++
> >   drivers/net/ethernet/intel/ice/ice_nvm.c      |  2 +-
> >   drivers/net/ethernet/intel/ice/ice_nvm.h      |  4 +++
> >   4 files changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > index ffbe9d3a5d77..a3a49d922650 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > @@ -1569,6 +1569,15 @@ struct ice_aqc_nvm {
> >   	__le32 addr_low;
> >   };
> >
> > +#define ICE_AQC_NVM_CMPO_MOD_ID			0x153
> > +
> > +/* Cage Max Power override NVM module */
> > +struct ice_aqc_nvm_cmpo {
> > +	__le16 length;
> > +#define ICE_AQC_NVM_CMPO_ENABLE	BIT(8)
> > +	__le16 cages_cfg[8];
> 
> [1] here
> 
> > +};
> > +
> >   #define ICE_AQC_NVM_START_POINT			0
> >
> >   /* NVM Checksum Command (direct, 0x0706) */
> > diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> > index 80dc5445b50d..e9300df9ef40 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> > @@ -591,6 +591,36 @@ static void ice_devlink_port_options_print(struct ice_pf *pf)
> >   	kfree(options);
> >   }
> >
> > +#define ICE_NUM_OF_CAGES 8
> 
> perhaps move this define to ice_adminq_cmd.h to benefit from it in
> struct definition just few lines above [1]

Good point

> > +
> > +/**
> > + * ice_devlink_aq_clear_cmpo - clear Cage Max Power override
> > + * @hw: pointer to the HW struct
> > + *
> > + * Read Cage Max Power override NVM module, clear override
> > + * enable bit for each of the cages. Write the settings back to
> > + * the NVM.
> 
> Read+clear+write is "how" or algorithm here, but doc should just stick
> to "what" most of the time. So I would just:
> "Clear Cage Max Power override enable bit for each of the cages".
> 
> "how" part could be inside the function, just above "read" call.

I'll rethink that.

> 
> > + */
> > +static int
> > +ice_devlink_aq_clear_cmpo(struct ice_hw *hw)
> > +{
> > +	struct ice_aqc_nvm_cmpo data;
> > +	int ret, i;
> > +
> > +	ret = ice_aq_read_nvm(hw, ICE_AQC_NVM_CMPO_MOD_ID, 0, sizeof(data),
> > +			      &data, true, false, NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	for (i = 0; i < ICE_NUM_OF_CAGES; i++)
> > +		data.cages_cfg[i] &= ~cpu_to_le16(ICE_AQC_NVM_CMPO_ENABLE);
> > +
> > +	/* Do not update the length word since it is not permitted */
> > +	return ice_aq_update_nvm(hw, ICE_AQC_NVM_CMPO_MOD_ID, 2,
> > +				 sizeof(data.cages_cfg), data.cages_cfg,
> > +				 false, 0, NULL);
> > +}
> > +
> >   /**
> >    * ice_devlink_aq_set_port_option - Send set port option admin queue command
> >    * @pf: the PF to print split port options
> > @@ -623,6 +653,11 @@ ice_devlink_aq_set_port_option(struct ice_pf *pf, u8 option_idx,
> >   		return -EIO;
> >   	}
> >
> > +	status = ice_devlink_aq_clear_cmpo(&pf->hw);
> > +	if (status)
> > +		dev_dbg(dev, "Failed to clear Cage Max Power override, err %d aq_err %d\n",
> > +			status, pf->hw.adminq.sq_last_status);
> > +
> >   	status = ice_nvm_write_activate(&pf->hw, ICE_AQC_NVM_ACTIV_REQ_EMPR, NULL);
> >   	if (status) {
> >   		dev_dbg(dev, "ice_nvm_write_activate failed, err %d aq_err %d\n",
> > diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
> > index f6f52a248066..745f2459943f 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_nvm.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
> > @@ -18,7 +18,7 @@
> >    *
> >    * Read the NVM using the admin queue commands (0x0701)
> >    */
> > -static int
> > +int
> >   ice_aq_read_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset, u16 length,
> >   		void *data, bool last_command, bool read_shadow_ram,
> >   		struct ice_sq_cd *cd)
> > diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.h b/drivers/net/ethernet/intel/ice/ice_nvm.h
> > index 774c2317967d..90f36e19e06b 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_nvm.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_nvm.h
> > @@ -15,6 +15,10 @@ struct ice_orom_civd_info {
> >   int ice_acquire_nvm(struct ice_hw *hw, enum ice_aq_res_access_type access);
> >   void ice_release_nvm(struct ice_hw *hw);
> >   int
> > +ice_aq_read_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset, u16 length,
> > +		void *data, bool last_command, bool read_shadow_ram,
> > +		struct ice_sq_cd *cd);
> > +int
> >   ice_read_flat_nvm(struct ice_hw *hw, u32 offset, u32 *length, u8 *data,
> >   		  bool read_shadow_ram);
> >   int
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index ffbe9d3a5d77..a3a49d922650 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1569,6 +1569,15 @@  struct ice_aqc_nvm {
 	__le32 addr_low;
 };
 
+#define ICE_AQC_NVM_CMPO_MOD_ID			0x153
+
+/* Cage Max Power override NVM module */
+struct ice_aqc_nvm_cmpo {
+	__le16 length;
+#define ICE_AQC_NVM_CMPO_ENABLE	BIT(8)
+	__le16 cages_cfg[8];
+};
+
 #define ICE_AQC_NVM_START_POINT			0
 
 /* NVM Checksum Command (direct, 0x0706) */
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 80dc5445b50d..e9300df9ef40 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -591,6 +591,36 @@  static void ice_devlink_port_options_print(struct ice_pf *pf)
 	kfree(options);
 }
 
+#define ICE_NUM_OF_CAGES 8
+
+/**
+ * ice_devlink_aq_clear_cmpo - clear Cage Max Power override
+ * @hw: pointer to the HW struct
+ *
+ * Read Cage Max Power override NVM module, clear override
+ * enable bit for each of the cages. Write the settings back to
+ * the NVM.
+ */
+static int
+ice_devlink_aq_clear_cmpo(struct ice_hw *hw)
+{
+	struct ice_aqc_nvm_cmpo data;
+	int ret, i;
+
+	ret = ice_aq_read_nvm(hw, ICE_AQC_NVM_CMPO_MOD_ID, 0, sizeof(data),
+			      &data, true, false, NULL);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ICE_NUM_OF_CAGES; i++)
+		data.cages_cfg[i] &= ~cpu_to_le16(ICE_AQC_NVM_CMPO_ENABLE);
+
+	/* Do not update the length word since it is not permitted */
+	return ice_aq_update_nvm(hw, ICE_AQC_NVM_CMPO_MOD_ID, 2,
+				 sizeof(data.cages_cfg), data.cages_cfg,
+				 false, 0, NULL);
+}
+
 /**
  * ice_devlink_aq_set_port_option - Send set port option admin queue command
  * @pf: the PF to print split port options
@@ -623,6 +653,11 @@  ice_devlink_aq_set_port_option(struct ice_pf *pf, u8 option_idx,
 		return -EIO;
 	}
 
+	status = ice_devlink_aq_clear_cmpo(&pf->hw);
+	if (status)
+		dev_dbg(dev, "Failed to clear Cage Max Power override, err %d aq_err %d\n",
+			status, pf->hw.adminq.sq_last_status);
+
 	status = ice_nvm_write_activate(&pf->hw, ICE_AQC_NVM_ACTIV_REQ_EMPR, NULL);
 	if (status) {
 		dev_dbg(dev, "ice_nvm_write_activate failed, err %d aq_err %d\n",
diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
index f6f52a248066..745f2459943f 100644
--- a/drivers/net/ethernet/intel/ice/ice_nvm.c
+++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
@@ -18,7 +18,7 @@ 
  *
  * Read the NVM using the admin queue commands (0x0701)
  */
-static int
+int
 ice_aq_read_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset, u16 length,
 		void *data, bool last_command, bool read_shadow_ram,
 		struct ice_sq_cd *cd)
diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.h b/drivers/net/ethernet/intel/ice/ice_nvm.h
index 774c2317967d..90f36e19e06b 100644
--- a/drivers/net/ethernet/intel/ice/ice_nvm.h
+++ b/drivers/net/ethernet/intel/ice/ice_nvm.h
@@ -15,6 +15,10 @@  struct ice_orom_civd_info {
 int ice_acquire_nvm(struct ice_hw *hw, enum ice_aq_res_access_type access);
 void ice_release_nvm(struct ice_hw *hw);
 int
+ice_aq_read_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset, u16 length,
+		void *data, bool last_command, bool read_shadow_ram,
+		struct ice_sq_cd *cd);
+int
 ice_read_flat_nvm(struct ice_hw *hw, u32 offset, u32 *length, u8 *data,
 		  bool read_shadow_ram);
 int