[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <MW4PR11MB57768A5C5E5670CFE680C5D7FD1FA@MW4PR11MB5776.namprd11.prod.outlook.com>
Date: Tue, 22 Aug 2023 09:39:45 +0000
From: "Drewek, Wojciech" <wojciech.drewek@...el.com>
To: "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH iwl-next] ice: Disable Cage Max Power override
> -----Original Message-----
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>
> Sent: wtorek, 22 sierpnia 2023 10:17
> To: Drewek, Wojciech <wojciech.drewek@...el.com>; intel-wired-lan@...ts.osuosl.org
> Cc: netdev@...r.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@...el.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
Powered by blists - more mailing lists