| 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
| ||
|
Message-ID: <ZL631F2MWdXVoM+y@corigine.com>
Date: Mon, 24 Jul 2023 19:41:40 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>,
Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
Jonathan Lemon <jonathan.lemon@...il.com>,
Paolo Abeni <pabeni@...hat.com>,
Milena Olech <milena.olech@...el.com>,
Michal Michalik <michal.michalik@...el.com>,
linux-arm-kernel@...ts.infradead.org, poros@...hat.com,
mschmidt@...hat.com, netdev@...r.kernel.org,
linux-clk@...r.kernel.org, Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH 09/11] ice: implement dpll interface to control cgu
On Thu, Jul 20, 2023 at 10:19:01AM +0100, Vadim Fedorenko wrote:
...
Hi Vadim,
> +/**
> + * ice_dpll_cb_unlock - unlock dplls mutex in callback context
> + * @pf: private board structure
> + *
> + * Unlock the mutex from the callback operations invoked by dpll subsystem.
> + */
> +static void ice_dpll_cb_unlock(struct ice_pf *pf)
> +{
> + mutex_unlock(&pf->dplls.lock);
> +}
> +
> +/**
> + * ice_dpll_pin_freq_set - set pin's frequency
> + * @pf: private board structure
> + * @pin: pointer to a pin
> + * @pin_type: type of pin being configured
> + * @freq: frequency to be set
> + * @extack: error reporting
> + *
> + * Set requested frequency on a pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error on AQ or wrong pin type given
> + */
> +static int
> +ice_dpll_pin_freq_set(struct ice_pf *pf, struct ice_dpll_pin *pin,
> + enum ice_dpll_pin_type pin_type, const u32 freq,
> + struct netlink_ext_ack *extack)
> +{
> + int ret;
> + u8 flags;
Please arrange local variable declarations for new Networking
code in reverse xmas tree order - longest line to shortest.
> +
> + switch (pin_type) {
> + case ICE_DPLL_PIN_TYPE_INPUT:
> + flags = ICE_AQC_SET_CGU_IN_CFG_FLG1_UPDATE_FREQ;
> + ret = ice_aq_set_input_pin_cfg(&pf->hw, pin->idx, flags,
> + pin->flags[0], freq, 0);
> + break;
> + case ICE_DPLL_PIN_TYPE_OUTPUT:
> + flags = ICE_AQC_SET_CGU_OUT_CFG_UPDATE_FREQ;
> + ret = ice_aq_set_output_pin_cfg(&pf->hw, pin->idx, flags,
> + 0, freq, 0);
> + break;
> + default:
> + return -EINVAL;
> + }
> + if (ret) {
> + NL_SET_ERR_MSG_FMT(extack,
> + "err:%d %s failed to set pin freq:%u on pin:%u\n",
> + ret,
> + ice_aq_str(pf->hw.adminq.sq_last_status),
> + freq, pin->idx);
> + return ret;
> + }
> + pin->freq = freq;
> +
> + return 0;
> +}
...
> +/**
> + * ice_dpll_pin_state_update - update pin's state
> + * @pf: private board struct
> + * @pin: structure with pin attributes to be updated
> + * @pin_type: type of pin being updated
> + * @extack: error reporting
> + *
> + * Determine pin current state and frequency, then update struct
> + * holding the pin info. For input pin states are separated for each
> + * dpll, for rclk pins states are separated for each parent.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - OK
> + * * negative - error
> + */
> +int
> +ice_dpll_pin_state_update(struct ice_pf *pf, struct ice_dpll_pin *pin,
> + enum ice_dpll_pin_type pin_type,
> + struct netlink_ext_ack *extack)
> +/**
> + * ice_dpll_frequency_set - wrapper for pin callback for set frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: frequency to be set
> + * @extack: error reporting
> + * @pin_type: type of pin being configured
> + *
> + * Wraps internal set frequency command on a pin.
> + *
> + * Context: Acquires pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't set in hw
> + */
> +static int
> +ice_dpll_frequency_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + const u32 frequency,
> + struct netlink_ext_ack *extack,
> + enum ice_dpll_pin_type pin_type)
> +{
> + struct ice_dpll_pin *p = pin_priv;
> + struct ice_dpll *d = dpll_priv;
> + struct ice_pf *pf = d->pf;
> + int ret;
> +
> + ret = ice_dpll_cb_lock(pf, extack);
> + if (ret)
> + return ret;
> + ret = ice_dpll_pin_freq_set(pf, p, pin_type, frequency, extack);
> + ice_dpll_cb_unlock(pf);
> +
> + return ret;
> +}
> +
> +/**
> + * ice_dpll_input_frequency_set - input pin callback for set frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: frequency to be set
> + * @extack: error reporting
> + *
> + * Wraps internal set frequency command on a pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't set in hw
> + */
> +static int
> +ice_dpll_input_frequency_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 frequency, struct netlink_ext_ack *extack)
> +{
> + return ice_dpll_frequency_set(pin, pin_priv, dpll, dpll_priv, frequency,
> + extack, ICE_DPLL_PIN_TYPE_INPUT);
> +}
> +
> +/**
> + * ice_dpll_output_frequency_set - output pin callback for set frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: frequency to be set
> + * @extack: error reporting
> + *
> + * Wraps internal set frequency command on a pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't set in hw
> + */
> +static int
> +ice_dpll_output_frequency_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 frequency, struct netlink_ext_ack *extack)
> +{
> + return ice_dpll_frequency_set(pin, pin_priv, dpll, dpll_priv, frequency,
> + extack, ICE_DPLL_PIN_TYPE_OUTPUT);
> +}
> +
> +/**
> + * ice_dpll_frequency_get - wrapper for pin callback for get frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: on success holds pin's frequency
> + * @extack: error reporting
> + * @pin_type: type of pin being configured
> + *
> + * Wraps internal get frequency command of a pin.
> + *
> + * Context: Acquires pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't get from hw
> + */
> +static int
> +ice_dpll_frequency_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 *frequency, struct netlink_ext_ack *extack,
> + enum ice_dpll_pin_type pin_type)
> +{
> + struct ice_dpll_pin *p = pin_priv;
> + struct ice_dpll *d = dpll_priv;
> + struct ice_pf *pf = d->pf;
> + int ret;
> +
> + ret = ice_dpll_cb_lock(pf, extack);
> + if (ret)
> + return ret;
> + *frequency = p->freq;
> + ice_dpll_cb_unlock(pf);
> +
> + return 0;
> +}
> +
> +/**
> + * ice_dpll_input_frequency_get - input pin callback for get frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: on success holds pin's frequency
> + * @extack: error reporting
> + *
> + * Wraps internal get frequency command of a input pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't get from hw
> + */
> +static int
> +ice_dpll_input_frequency_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 *frequency, struct netlink_ext_ack *extack)
> +{
> + return ice_dpll_frequency_get(pin, pin_priv, dpll, dpll_priv, frequency,
> + extack, ICE_DPLL_PIN_TYPE_INPUT);
> +}
> +
> +/**
> + * ice_dpll_output_frequency_get - output pin callback for get frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: on success holds pin's frequency
> + * @extack: error reporting
> + *
> + * Wraps internal get frequency command of a pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't get from hw
> + */
> +static int
> +ice_dpll_output_frequency_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 *frequency, struct netlink_ext_ack *extack)
> +{
> + return ice_dpll_frequency_get(pin, pin_priv, dpll, dpll_priv, frequency,
> + extack, ICE_DPLL_PIN_TYPE_OUTPUT);
> +}
> +
> +/**
> + * ice_dpll_pin_enable - enable a pin on dplls
> + * @hw: board private hw structure
> + * @pin: pointer to a pin
> + * @pin_type: type of pin being enabled
> + * @extack: error reporting
> + *
> + * Enable a pin on both dplls. Store current state in pin->flags.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - OK
> + * * negative - error
> + */
> +static int
> +ice_dpll_pin_enable(struct ice_hw *hw, struct ice_dpll_pin *pin,
> + enum ice_dpll_pin_type pin_type,
> + struct netlink_ext_ack *extack)
> +{
> + u8 flags = 0;
> + int ret;
> +
> + switch (pin_type) {
> + case ICE_DPLL_PIN_TYPE_INPUT:
> + if (pin->flags[0] & ICE_AQC_GET_CGU_IN_CFG_FLG2_ESYNC_EN)
> + flags |= ICE_AQC_SET_CGU_IN_CFG_FLG2_ESYNC_EN;
> + flags |= ICE_AQC_SET_CGU_IN_CFG_FLG2_INPUT_EN;
> + ret = ice_aq_set_input_pin_cfg(hw, pin->idx, 0, flags, 0, 0);
> + break;
> + case ICE_DPLL_PIN_TYPE_OUTPUT:
> + if (pin->flags[0] & ICE_AQC_GET_CGU_OUT_CFG_ESYNC_EN)
> + flags |= ICE_AQC_SET_CGU_OUT_CFG_ESYNC_EN;
> + flags |= ICE_AQC_SET_CGU_OUT_CFG_OUT_EN;
> + ret = ice_aq_set_output_pin_cfg(hw, pin->idx, flags, 0, 0, 0);
> + break;
> + default:
> + return -EINVAL;
> + }
> + if (ret)
> + NL_SET_ERR_MSG_FMT(extack,
> + "err:%d %s failed to enable %s pin:%u\n",
> + ret, ice_aq_str(hw->adminq.sq_last_status),
> + pin_type_name[pin_type], pin->idx);
> +
> + return ret;
> +}
> +
> +/**
> + * ice_dpll_pin_disable - disable a pin on dplls
> + * @hw: board private hw structure
> + * @pin: pointer to a pin
> + * @pin_type: type of pin being disabled
> + * @extack: error reporting
> + *
> + * Disable a pin on both dplls. Store current state in pin->flags.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - OK
> + * * negative - error
> + */
> +static int
> +ice_dpll_pin_disable(struct ice_hw *hw, struct ice_dpll_pin *pin,
> + enum ice_dpll_pin_type pin_type,
> + struct netlink_ext_ack *extack)
> +{
> + u8 flags = 0;
> + int ret;
> +
> + switch (pin_type) {
> + case ICE_DPLL_PIN_TYPE_INPUT:
> + if (pin->flags[0] & ICE_AQC_GET_CGU_IN_CFG_FLG2_ESYNC_EN)
> + flags |= ICE_AQC_SET_CGU_IN_CFG_FLG2_ESYNC_EN;
> + ret = ice_aq_set_input_pin_cfg(hw, pin->idx, 0, flags, 0, 0);
> + break;
> + case ICE_DPLL_PIN_TYPE_OUTPUT:
> + if (pin->flags[0] & ICE_AQC_GET_CGU_OUT_CFG_ESYNC_EN)
> + flags |= ICE_AQC_SET_CGU_OUT_CFG_ESYNC_EN;
> + ret = ice_aq_set_output_pin_cfg(hw, pin->idx, flags, 0, 0, 0);
> + break;
> + default:
> + return -EINVAL;
> + }
> + if (ret)
> + NL_SET_ERR_MSG_FMT(extack,
> + "err:%d %s failed to disable %s pin:%u\n",
> + ret, ice_aq_str(hw->adminq.sq_last_status),
> + pin_type_name[pin_type], pin->idx);
> +
> + return ret;
> +}
> +/**
> + * ice_dpll_frequency_set - wrapper for pin callback for set frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: frequency to be set
> + * @extack: error reporting
> + * @pin_type: type of pin being configured
> + *
> + * Wraps internal set frequency command on a pin.
> + *
> + * Context: Acquires pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't set in hw
> + */
> +static int
> +ice_dpll_frequency_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + const u32 frequency,
> + struct netlink_ext_ack *extack,
> + enum ice_dpll_pin_type pin_type)
> +{
> + struct ice_dpll_pin *p = pin_priv;
> + struct ice_dpll *d = dpll_priv;
> + struct ice_pf *pf = d->pf;
> + int ret;
> +
> + ret = ice_dpll_cb_lock(pf, extack);
> + if (ret)
> + return ret;
> + ret = ice_dpll_pin_freq_set(pf, p, pin_type, frequency, extack);
> + ice_dpll_cb_unlock(pf);
> +
> + return ret;
> +}
> +
> +/**
> + * ice_dpll_input_frequency_set - input pin callback for set frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: frequency to be set
> + * @extack: error reporting
> + *
> + * Wraps internal set frequency command on a pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't set in hw
> + */
> +static int
> +ice_dpll_input_frequency_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 frequency, struct netlink_ext_ack *extack)
> +{
> + return ice_dpll_frequency_set(pin, pin_priv, dpll, dpll_priv, frequency,
> + extack, ICE_DPLL_PIN_TYPE_INPUT);
> +}
> +
> +/**
> + * ice_dpll_output_frequency_set - output pin callback for set frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: frequency to be set
> + * @extack: error reporting
> + *
> + * Wraps internal set frequency command on a pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't set in hw
> + */
> +static int
> +ice_dpll_output_frequency_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 frequency, struct netlink_ext_ack *extack)
> +{
> + return ice_dpll_frequency_set(pin, pin_priv, dpll, dpll_priv, frequency,
> + extack, ICE_DPLL_PIN_TYPE_OUTPUT);
> +}
> +
> +/**
> + * ice_dpll_frequency_get - wrapper for pin callback for get frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: on success holds pin's frequency
> + * @extack: error reporting
> + * @pin_type: type of pin being configured
> + *
> + * Wraps internal get frequency command of a pin.
> + *
> + * Context: Acquires pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't get from hw
> + */
> +static int
> +ice_dpll_frequency_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 *frequency, struct netlink_ext_ack *extack,
> + enum ice_dpll_pin_type pin_type)
> +{
> + struct ice_dpll_pin *p = pin_priv;
> + struct ice_dpll *d = dpll_priv;
> + struct ice_pf *pf = d->pf;
> + int ret;
> +
> + ret = ice_dpll_cb_lock(pf, extack);
> + if (ret)
> + return ret;
> + *frequency = p->freq;
> + ice_dpll_cb_unlock(pf);
> +
> + return 0;
> +}
> +
> +/**
> + * ice_dpll_input_frequency_get - input pin callback for get frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: on success holds pin's frequency
> + * @extack: error reporting
> + *
> + * Wraps internal get frequency command of a input pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't get from hw
> + */
> +static int
> +ice_dpll_input_frequency_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 *frequency, struct netlink_ext_ack *extack)
> +{
> + return ice_dpll_frequency_get(pin, pin_priv, dpll, dpll_priv, frequency,
> + extack, ICE_DPLL_PIN_TYPE_INPUT);
> +}
> +
> +/**
> + * ice_dpll_output_frequency_get - output pin callback for get frequency
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: pointer to dpll
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @frequency: on success holds pin's frequency
> + * @extack: error reporting
> + *
> + * Wraps internal get frequency command of a pin.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error pin not found or couldn't get from hw
> + */
> +static int
> +ice_dpll_output_frequency_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u64 *frequency, struct netlink_ext_ack *extack)
> +{
> + return ice_dpll_frequency_get(pin, pin_priv, dpll, dpll_priv, frequency,
> + extack, ICE_DPLL_PIN_TYPE_OUTPUT);
> +}
> +
> +/**
> + * ice_dpll_pin_enable - enable a pin on dplls
> + * @hw: board private hw structure
> + * @pin: pointer to a pin
> + * @pin_type: type of pin being enabled
> + * @extack: error reporting
> + *
> + * Enable a pin on both dplls. Store current state in pin->flags.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - OK
> + * * negative - error
> + */
> +static int
> +ice_dpll_pin_enable(struct ice_hw *hw, struct ice_dpll_pin *pin,
> + enum ice_dpll_pin_type pin_type,
> + struct netlink_ext_ack *extack)
> +{
> + u8 flags = 0;
> + int ret;
> +
> + switch (pin_type) {
> + case ICE_DPLL_PIN_TYPE_INPUT:
> + if (pin->flags[0] & ICE_AQC_GET_CGU_IN_CFG_FLG2_ESYNC_EN)
> + flags |= ICE_AQC_SET_CGU_IN_CFG_FLG2_ESYNC_EN;
> + flags |= ICE_AQC_SET_CGU_IN_CFG_FLG2_INPUT_EN;
> + ret = ice_aq_set_input_pin_cfg(hw, pin->idx, 0, flags, 0, 0);
> + break;
> + case ICE_DPLL_PIN_TYPE_OUTPUT:
> + if (pin->flags[0] & ICE_AQC_GET_CGU_OUT_CFG_ESYNC_EN)
> + flags |= ICE_AQC_SET_CGU_OUT_CFG_ESYNC_EN;
> + flags |= ICE_AQC_SET_CGU_OUT_CFG_OUT_EN;
> + ret = ice_aq_set_output_pin_cfg(hw, pin->idx, flags, 0, 0, 0);
> + break;
> + default:
> + return -EINVAL;
> + }
> + if (ret)
> + NL_SET_ERR_MSG_FMT(extack,
> + "err:%d %s failed to enable %s pin:%u\n",
> + ret, ice_aq_str(hw->adminq.sq_last_status),
> + pin_type_name[pin_type], pin->idx);
> +
> + return ret;
> +}
> +
> +/**
> + * ice_dpll_pin_disable - disable a pin on dplls
> + * @hw: board private hw structure
> + * @pin: pointer to a pin
> + * @pin_type: type of pin being disabled
> + * @extack: error reporting
> + *
> + * Disable a pin on both dplls. Store current state in pin->flags.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - OK
> + * * negative - error
> + */
> +static int
> +ice_dpll_pin_disable(struct ice_hw *hw, struct ice_dpll_pin *pin,
> + enum ice_dpll_pin_type pin_type,
> + struct netlink_ext_ack *extack)
> +{
> + u8 flags = 0;
> + int ret;
> +
> + switch (pin_type) {
> + case ICE_DPLL_PIN_TYPE_INPUT:
> + if (pin->flags[0] & ICE_AQC_GET_CGU_IN_CFG_FLG2_ESYNC_EN)
> + flags |= ICE_AQC_SET_CGU_IN_CFG_FLG2_ESYNC_EN;
> + ret = ice_aq_set_input_pin_cfg(hw, pin->idx, 0, flags, 0, 0);
> + break;
> + case ICE_DPLL_PIN_TYPE_OUTPUT:
> + if (pin->flags[0] & ICE_AQC_GET_CGU_OUT_CFG_ESYNC_EN)
> + flags |= ICE_AQC_SET_CGU_OUT_CFG_ESYNC_EN;
> + ret = ice_aq_set_output_pin_cfg(hw, pin->idx, flags, 0, 0, 0);
> + break;
> + default:
> + return -EINVAL;
> + }
> + if (ret)
> + NL_SET_ERR_MSG_FMT(extack,
> + "err:%d %s failed to disable %s pin:%u\n",
> + ret, ice_aq_str(hw->adminq.sq_last_status),
> + pin_type_name[pin_type], pin->idx);
> +
> + return ret;
> +}
Should this function be static?
> +{
> + int ret;
> +
> + switch (pin_type) {
> + case ICE_DPLL_PIN_TYPE_INPUT:
> + ret = ice_aq_get_input_pin_cfg(&pf->hw, pin->idx, NULL, NULL,
> + NULL, &pin->flags[0],
> + &pin->freq, NULL);
> + if (ret)
> + goto err;
> + if (ICE_AQC_GET_CGU_IN_CFG_FLG2_INPUT_EN & pin->flags[0]) {
> + if (pin->pin) {
> + pin->state[pf->dplls.eec.dpll_idx] =
> + pin->pin == pf->dplls.eec.active_input ?
> + DPLL_PIN_STATE_CONNECTED :
> + DPLL_PIN_STATE_SELECTABLE;
> + pin->state[pf->dplls.pps.dpll_idx] =
> + pin->pin == pf->dplls.pps.active_input ?
> + DPLL_PIN_STATE_CONNECTED :
> + DPLL_PIN_STATE_SELECTABLE;
> + } else {
> + pin->state[pf->dplls.eec.dpll_idx] =
> + DPLL_PIN_STATE_SELECTABLE;
> + pin->state[pf->dplls.pps.dpll_idx] =
> + DPLL_PIN_STATE_SELECTABLE;
> + }
> + } else {
> + pin->state[pf->dplls.eec.dpll_idx] =
> + DPLL_PIN_STATE_DISCONNECTED;
> + pin->state[pf->dplls.pps.dpll_idx] =
> + DPLL_PIN_STATE_DISCONNECTED;
> + }
> + break;
> + case ICE_DPLL_PIN_TYPE_OUTPUT:
> + ret = ice_aq_get_output_pin_cfg(&pf->hw, pin->idx,
> + &pin->flags[0], NULL,
> + &pin->freq, NULL);
> + if (ret)
> + goto err;
> + if (ICE_AQC_SET_CGU_OUT_CFG_OUT_EN & pin->flags[0])
> + pin->state[0] = DPLL_PIN_STATE_CONNECTED;
> + else
> + pin->state[0] = DPLL_PIN_STATE_DISCONNECTED;
> + break;
> + case ICE_DPLL_PIN_TYPE_RCLK_INPUT:
clang-16 complains that:
drivers/net/ethernet/intel/ice/ice_dpll.c:461:3: error: expected expression
u8 parent, port_num = ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT;
Which, I think means, it wants this case to be enclosed in { }
> + u8 parent, port_num = ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT;
> +
> + for (parent = 0; parent < pf->dplls.rclk.num_parents;
> + parent++) {
> + u8 p = parent;
> +
> + ret = ice_aq_get_phy_rec_clk_out(&pf->hw, &p,
> + &port_num,
> + &pin->flags[parent],
> + NULL);
> + if (ret)
> + goto err;
> + if (ICE_AQC_GET_PHY_REC_CLK_OUT_OUT_EN &
> + pin->flags[parent])
> + pin->state[parent] = DPLL_PIN_STATE_CONNECTED;
> + else
> + pin->state[parent] =
> + DPLL_PIN_STATE_DISCONNECTED;
> + }
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +err:
> + if (extack)
> + NL_SET_ERR_MSG_FMT(extack,
> + "err:%d %s failed to update %s pin:%u\n",
> + ret,
> + ice_aq_str(pf->hw.adminq.sq_last_status),
> + pin_type_name[pin_type], pin->idx);
> + else
> + dev_err_ratelimited(ice_pf_to_dev(pf),
> + "err:%d %s failed to update %s pin:%u\n",
> + ret,
> + ice_aq_str(pf->hw.adminq.sq_last_status),
> + pin_type_name[pin_type], pin->idx);
> + return ret;
> +}
...
> +/**
> + * ice_dpll_update_state - update dpll state
> + * @pf: pf private structure
> + * @d: pointer to queried dpll device
> + * @init: if function called on initialization of ice dpll
> + *
> + * Poll current state of dpll from hw and update ice_dpll struct.
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - AQ failure
> + */
> +static int
> +ice_dpll_update_state(struct ice_pf *pf, struct ice_dpll *d, bool init)
> +{
> + struct ice_dpll_pin *p = NULL;
> + int ret;
> +
> + ret = ice_get_cgu_state(&pf->hw, d->dpll_idx, d->prev_dpll_state,
> + &d->input_idx, &d->ref_state, &d->eec_mode,
> + &d->phase_shift, &d->dpll_state, &d->mode);
> +
> + dev_dbg(ice_pf_to_dev(pf),
> + "update dpll=%d, prev_src_idx:%u, src_idx:%u, state:%d, prev:%d mode:%d\n",
> + d->dpll_idx, d->prev_input_idx, d->input_idx,
> + d->dpll_state, d->prev_dpll_state, d->mode);
> + if (ret) {
> + dev_err(ice_pf_to_dev(pf),
> + "update dpll=%d state failed, ret=%d %s\n",
> + d->dpll_idx, ret,
> + ice_aq_str(pf->hw.adminq.sq_last_status));
> + return ret;
> + }
> + if (init) {
> + if (d->dpll_state == DPLL_LOCK_STATUS_LOCKED &&
> + d->dpll_state == DPLL_LOCK_STATUS_LOCKED_HO_ACQ)
Should this be '||' rather than '&&' ?
Flagged by a clang-16 W=1 build, Sparse and Smatch.
> + d->active_input = pf->dplls.inputs[d->input_idx].pin;
> + p = &pf->dplls.inputs[d->input_idx];
> + return ice_dpll_pin_state_update(pf, p,
> + ICE_DPLL_PIN_TYPE_INPUT, NULL);
> + }
...
> +/**
> + * ice_dpll_init_info_direct_pins - initializes direct pins info
> + * @pf: board private structure
> + * @pin_type: type of pins being initialized
> + *
> + * Init information for directly connected pins, cache them in pf's pins
> + * structures.
> + *
> + * Context: Called under pf->dplls.lock.
> + * Return:
> + * * 0 - success
> + * * negative - init failure reason
> + */
> +static int
> +ice_dpll_init_info_direct_pins(struct ice_pf *pf,
> + enum ice_dpll_pin_type pin_type)
> +{
> + struct ice_dpll *de = &pf->dplls.eec, *dp = &pf->dplls.pps;
> + struct ice_hw *hw = &pf->hw;
> + struct ice_dpll_pin *pins;
> + int num_pins, i, ret;
> + u8 freq_supp_num;
> + bool input;
> +
> + switch (pin_type) {
> + case ICE_DPLL_PIN_TYPE_INPUT:
> + pins = pf->dplls.inputs;
> + num_pins = pf->dplls.num_inputs;
> + input = true;
> + break;
> + case ICE_DPLL_PIN_TYPE_OUTPUT:
> + pins = pf->dplls.outputs;
> + num_pins = pf->dplls.num_outputs;
> + input = false;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < num_pins; i++) {
> + pins[i].idx = i;
> + pins[i].prop.board_label = ice_cgu_get_pin_name(hw, i, input);
> + pins[i].prop.type = ice_cgu_get_pin_type(hw, i, input);
> + if (input) {
> + ret = ice_aq_get_cgu_ref_prio(hw, de->dpll_idx, i,
> + &de->input_prio[i]);
> + if (ret)
> + return ret;
> + ret = ice_aq_get_cgu_ref_prio(hw, dp->dpll_idx, i,
> + &dp->input_prio[i]);
> + if (ret)
> + return ret;
> + pins[i].prop.capabilities |=
> + DPLL_PIN_CAPS_PRIORITY_CAN_CHANGE;
> + }
> + pins[i].prop.capabilities |= DPLL_PIN_CAPS_STATE_CAN_CHANGE;
> + ret = ice_dpll_pin_state_update(pf, &pins[i], pin_type, NULL);
> + if (ret)
> + return ret;
> + pins[i].prop.freq_supported =
> + ice_cgu_get_pin_freq_supp(hw, i, input, &freq_supp_num);
> + pins[i].prop.freq_supported_num = freq_supp_num;
> + pins[i].pf = pf;
> + }
> +
I'm unsure if this can happen,
but if the for loop above iterates zero times
then ret will be null here.
Use of uninitialised variable flagged by Smatch.
> + return ret;
> +}
...
> +/**
> + * ice_dpll_init_info - prepare pf's dpll information structure
> + * @pf: board private structure
> + * @cgu: if cgu is present and controlled by this NIC
> + *
> + * Acquire (from HW) and set basic dpll information (on pf->dplls struct).
> + *
> + * Context: Called under pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - init failure reason
> + */
> +static int ice_dpll_init_info(struct ice_pf *pf, bool cgu)
> +{
> + struct ice_aqc_get_cgu_abilities abilities;
> + struct ice_dpll *de = &pf->dplls.eec;
> + struct ice_dpll *dp = &pf->dplls.pps;
> + struct ice_dplls *d = &pf->dplls;
> + struct ice_hw *hw = &pf->hw;
> + int ret, alloc_size, i;
> +
> + d->clock_id = ice_generate_clock_id(pf);
> + ret = ice_aq_get_cgu_abilities(hw, &abilities);
> + if (ret) {
> + dev_err(ice_pf_to_dev(pf),
> + "err:%d %s failed to read cgu abilities\n",
> + ret, ice_aq_str(hw->adminq.sq_last_status));
> + return ret;
> + }
> +
> + de->dpll_idx = abilities.eec_dpll_idx;
> + dp->dpll_idx = abilities.pps_dpll_idx;
> + d->num_inputs = abilities.num_inputs;
> + d->num_outputs = abilities.num_outputs;
> + d->input_phase_adj_max = le32_to_cpu(abilities.max_in_phase_adj);
> + d->output_phase_adj_max = le32_to_cpu(abilities.max_out_phase_adj);
> +
> + alloc_size = sizeof(*d->inputs) * d->num_inputs;
> + d->inputs = kzalloc(alloc_size, GFP_KERNEL);
> + if (!d->inputs)
> + return -ENOMEM;
> +
> + alloc_size = sizeof(*de->input_prio) * d->num_inputs;
> + de->input_prio = kzalloc(alloc_size, GFP_KERNEL);
> + if (!de->input_prio)
> + return -ENOMEM;
> +
> + dp->input_prio = kzalloc(alloc_size, GFP_KERNEL);
> + if (!dp->input_prio)
> + return -ENOMEM;
> +
> + ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_INPUT);
> + if (ret)
> + goto deinit_info;
> +
> + if (cgu) {
> + alloc_size = sizeof(*d->outputs) * d->num_outputs;
> + d->outputs = kzalloc(alloc_size, GFP_KERNEL);
> + if (!d->outputs)
Should ret be set to -ENOMEM here?
Flagged by Smatch.
> + goto deinit_info;
> +
> + ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_OUTPUT);
> + if (ret)
> + goto deinit_info;
> + }
> +
> + ret = ice_get_cgu_rclk_pin_info(&pf->hw, &d->base_rclk_idx,
> + &pf->dplls.rclk.num_parents);
> + if (ret)
> + return ret;
> + for (i = 0; i < pf->dplls.rclk.num_parents; i++)
> + pf->dplls.rclk.parent_idx[i] = d->base_rclk_idx + i;
> + ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_RCLK_INPUT);
> + if (ret)
> + return ret;
> + de->mode = DPLL_MODE_AUTOMATIC;
> + dp->mode = DPLL_MODE_AUTOMATIC;
> +
> + dev_dbg(ice_pf_to_dev(pf),
> + "%s - success, inputs:%u, outputs:%u rclk-parents:%u\n",
> + __func__, d->num_inputs, d->num_outputs, d->rclk.num_parents);
> +
> + return 0;
> +
> +deinit_info:
> + dev_err(ice_pf_to_dev(pf),
> + "%s - fail: d->inputs:%p, de->input_prio:%p, dp->input_prio:%p, d->outputs:%p\n",
> + __func__, d->inputs, de->input_prio,
> + dp->input_prio, d->outputs);
> + ice_dpll_deinit_info(pf);
> + return ret;
> +}
...
> +/**
> + * ice_dpll_init - initialize support for dpll subsystem
> + * @pf: board private structure
> + *
> + * Set up the device dplls, register them and pins connected within Linux dpll
> + * subsystem. Allow userpsace to obtain state of DPLL and handling of DPLL
nit: userpsace -> userspace
> + * configuration requests.
> + *
> + * Context: Function initializes and holds pf->dplls.lock mutex.
> + */
...
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
> new file mode 100644
> index 000000000000..975066b71c5e
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
> @@ -0,0 +1,104 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2022, Intel Corporation. */
> +
> +#ifndef _ICE_DPLL_H_
> +#define _ICE_DPLL_H_
> +
> +#include "ice.h"
> +
> +#define ICE_DPLL_PRIO_MAX 0xF
> +#define ICE_DPLL_RCLK_NUM_MAX 4
> +
> +/** ice_dpll_pin - store info about pins
> + * @pin: dpll pin structure
> + * @pf: pointer to pf, which has registered the dpll_pin
> + * @idx: ice pin private idx
> + * @num_parents: hols number of parent pins
> + * @parent_idx: hold indexes of parent pins
> + * @flags: pin flags returned from HW
> + * @state: state of a pin
> + * @prop: pin properities
nit: properities -> properties
> + * @freq: current frequency of a pin
> + */
> +struct ice_dpll_pin {
> + struct dpll_pin *pin;
> + struct ice_pf *pf;
> + u8 idx;
> + u8 num_parents;
> + u8 parent_idx[ICE_DPLL_RCLK_NUM_MAX];
> + u8 flags[ICE_DPLL_RCLK_NUM_MAX];
> + u8 state[ICE_DPLL_RCLK_NUM_MAX];
> + struct dpll_pin_properties prop;
> + u32 freq;
> +};
...
Powered by blists - more mailing lists