Re: [dpdk-dev] [PATCH v2 17/66] net/ice/base: add API to init FW logging

["Rong, Leyi" <leyi.rong@intel.com>]

> -----Original Message-----
> From: Stillwell Jr, Paul M
> Sent: Wednesday, June 12, 2019 12:24 AM
> To: Rong, Leyi <leyi.rong@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Nowlin, Dan <dan.nowlin@intel.com>
> Subject: RE: [PATCH v2 17/66] net/ice/base: add API to init FW logging
> 
> > -----Original Message-----
> > From: Rong, Leyi
> > Sent: Tuesday, June 11, 2019 8:52 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Rong, Leyi <leyi.rong@intel.com>; Nowlin, Dan
> > <dan.nowlin@intel.com>; Stillwell Jr, Paul M
> > <paul.m.stillwell.jr@intel.com>
> > Subject: [PATCH v2 17/66] net/ice/base: add API to init FW logging
> >
> > In order to initialize the current status of the FW logging, the api
> > ice_get_fw_log_cfg is added. The function retrieves the current
> > setting of the FW logging from HW and updates the ice_hw structure accordingly.
> >
> > Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
> > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> > Signed-off-by: Leyi Rong <leyi.rong@intel.com>
> > ---
> >  drivers/net/ice/base/ice_adminq_cmd.h |  1 +
> >  drivers/net/ice/base/ice_common.c     | 48
> > +++++++++++++++++++++++++++
> >  2 files changed, 49 insertions(+)
> >
> > diff --git a/drivers/net/ice/base/ice_adminq_cmd.h
> > b/drivers/net/ice/base/ice_adminq_cmd.h
> > index 7b0aa8aaa..739f79e88 100644
> > --- a/drivers/net/ice/base/ice_adminq_cmd.h
> > +++ b/drivers/net/ice/base/ice_adminq_cmd.h
> > @@ -2196,6 +2196,7 @@ enum ice_aqc_fw_logging_mod {
> >  	ICE_AQC_FW_LOG_ID_WATCHDOG,
> >  	ICE_AQC_FW_LOG_ID_TASK_DISPATCH,
> >  	ICE_AQC_FW_LOG_ID_MNG,
> > +	ICE_AQC_FW_LOG_ID_SYNCE,
> >  	ICE_AQC_FW_LOG_ID_MAX,
> >  };
> >
> > diff --git a/drivers/net/ice/base/ice_common.c
> > b/drivers/net/ice/base/ice_common.c
> > index 62c7fad0d..7093ee4f4 100644
> > --- a/drivers/net/ice/base/ice_common.c
> > +++ b/drivers/net/ice/base/ice_common.c
> > @@ -582,6 +582,49 @@ static void ice_cleanup_fltr_mgmt_struct(struct
> > ice_hw *hw)
> >  #define ICE_FW_LOG_DESC_SIZE_MAX	\
> >  	ICE_FW_LOG_DESC_SIZE(ICE_AQC_FW_LOG_ID_MAX)
> >
> > +/**
> > + * ice_get_fw_log_cfg - get FW logging configuration
> > + * @hw: pointer to the HW struct
> > + */
> > +static enum ice_status ice_get_fw_log_cfg(struct ice_hw *hw) {
> > +	struct ice_aqc_fw_logging_data *config;
> > +	struct ice_aq_desc desc;
> > +	enum ice_status status;
> > +	u16 size;
> > +
> > +	size = ICE_FW_LOG_DESC_SIZE_MAX;
> > +	config = (struct ice_aqc_fw_logging_data *)ice_malloc(hw, size);
> > +	if (!config)
> > +		return ICE_ERR_NO_MEMORY;
> > +
> > +	ice_fill_dflt_direct_cmd_desc(&desc,
> > ice_aqc_opc_fw_logging_info);
> > +
> > +	desc.flags |= CPU_TO_LE16(ICE_AQ_FLAG_BUF);
> > +	desc.flags |= CPU_TO_LE16(ICE_AQ_FLAG_RD);
> > +
> > +	status = ice_aq_send_cmd(hw, &desc, config, size, NULL);
> > +	if (!status) {
> > +		u16 i;
> > +
> > +		/* Save fw logging information into the HW structure */
> > +		for (i = 0; i < ICE_AQC_FW_LOG_ID_MAX; i++) {
> > +			u16 v, m, flgs;
> > +
> > +			v = LE16_TO_CPU(config->entry[i]);
> > +			m = (v & ICE_AQC_FW_LOG_ID_M) >>
> > ICE_AQC_FW_LOG_ID_S;
> > +			flgs = (v & ICE_AQC_FW_LOG_EN_M) >>
> > ICE_AQC_FW_LOG_EN_S;
> > +
> > +			if (m < ICE_AQC_FW_LOG_ID_MAX)
> > +				hw->fw_log.evnts[m].cur = flgs;
> > +		}
> > +	}
> > +
> > +	ice_free(hw, config);
> > +
> > +	return status;
> > +}
> > +
> >  /**
> >   * ice_cfg_fw_log - configure FW logging
> >   * @hw: pointer to the HW struct
> > @@ -636,6 +679,11 @@ static enum ice_status ice_cfg_fw_log(struct
> > ice_hw *hw, bool enable)
> 
> Is there code in DPDK that calls ice_cfg_fw_log()? If not then I would drop this patch.
> 

Yes, ice_cfg_fw_log() can be called indirectly.
ice_dev_init() -> ice_init_hw() -> ice_cfg_fw_log()