Add the tables documenting the DPLL pin MUX states and behavior to the driver. Signed-off-by: Sergey Temerkhanov --- drivers/net/ethernet/intel/ice/ice_dpll.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index 382fbe98f2b0..9b3584fa43a9 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -92,6 +92,29 @@ static const struct dpll_pin_frequency ice_esync_range[] = { DPLL_PIN_FREQUENCY_RANGE(0, DPLL_PIN_FREQUENCY_1_HZ), }; +/* + * SMA1/U.FL1 MUX table + *+----------------+-------------+------------+ + *| | SMA1_DIR_EN | SMA1_TX_EN | + *+----------------+-------------+------------+ + *|SMA1 RX Enable | 0 | x | + *|u.FL1 TX Enable | 0 | 0 | + *|SMA1 TX Enable | 1 | 0 | + *|All Disable | 1 | 1 | + *+----------------+-------------+------------+ + * + * SMA2/U.FL2 MUX table + *+----------------+-------------+------------+------------+ + *| | SMA2_DIR_EN | SMA2_TX_EN | UFL2_RX_EN | + *+----------------+-------------+------------+------------+ + *|SMA2 RX Enable | 0 | x | x | + *|u.FL2 RX Enable | 1 | x | 0 | + *|SMA2 TX Enable | 1 | 0 | x | + *|SMA2 Disable | 1 | 1 | x | + *|All Disable | 1 | 1 | 1 | + *+----------------+-------------+------------+------------+ + */ + /** * ice_dpll_is_sw_pin - check if given pin shall be controlled by SW * @pf: private board structure -- 2.53.0 Use a pointer to the struct ice_dpll_pin to link multiplexed pins. This allows to simplify the selection logic. Signed-off-by: Sergey Temerkhanov Reviewed-by: Aleksandr Loktionov Reviewed-by: Przemyslaw Korba --- drivers/net/ethernet/intel/ice/ice_dpll.c | 60 +++++++++++------------ drivers/net/ethernet/intel/ice/ice_dpll.h | 1 + 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index 9b3584fa43a9..4dcbbabee7aa 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -564,6 +564,29 @@ ice_dpll_pin_disable(struct ice_hw *hw, struct ice_dpll_pin *pin, return ret; } +/** + * ice_dpll_sw_pin_notify_peer - notify the paired SW pin after a state change + * @changed: the SW pin that was explicitly changed (already notified by dpll core) + * + * SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1 and + * SMA2/U.FL2). When one pin's routing changes via the PCA9575 GPIO + * expander, the paired pin's state may also change. Send a change + * notification for the peer pin so userspace consumers monitoring the + * peer via dpll netlink learn about the update. + * + * Context: Called from dpll_pin_ops callbacks after pf->dplls.lock is + * released. Uses __dpll_pin_change_ntf() because dpll_lock is + * still held by the dpll netlink layer. + */ +static void ice_dpll_sw_pin_notify_peer(struct ice_dpll_pin *changed) +{ + struct ice_dpll_pin *peer; + + peer = changed->muxed; + if (peer->pin) + __dpll_pin_change_ntf(peer->pin); +} + /** * ice_dpll_pin_store_state - updates the state of pin in SW bookkeeping * @pin: pointer to a pin @@ -1189,32 +1212,6 @@ ice_dpll_input_state_get(const struct dpll_pin *pin, void *pin_priv, extack, ICE_DPLL_PIN_TYPE_INPUT); } -/** - * ice_dpll_sw_pin_notify_peer - notify the paired SW pin after a state change - * @d: pointer to dplls struct - * @changed: the SW pin that was explicitly changed (already notified by dpll core) - * - * SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1 and - * SMA2/U.FL2). When one pin's routing changes via the PCA9575 GPIO - * expander, the paired pin's state may also change. Send a change - * notification for the peer pin so userspace consumers monitoring the - * peer via dpll netlink learn about the update. - * - * Context: Called from dpll_pin_ops callbacks after pf->dplls.lock is - * released. Uses __dpll_pin_change_ntf() because dpll_lock is - * still held by the dpll netlink layer. - */ -static void ice_dpll_sw_pin_notify_peer(struct ice_dplls *d, - struct ice_dpll_pin *changed) -{ - struct ice_dpll_pin *peer; - - peer = (changed >= d->sma && changed < d->sma + ICE_DPLL_PIN_SW_NUM) ? - &d->ufl[changed->idx] : &d->sma[changed->idx]; - if (peer->pin) - __dpll_pin_change_ntf(peer->pin); -} - /** * ice_dpll_sma_direction_set - set direction of SMA pin * @p: pointer to a pin @@ -1276,7 +1273,7 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p, * backing pin when U.FL becomes inactive because the SMA pin may * still be using it. */ - peer = &d->ufl[p->idx]; + peer = p->muxed; if (peer->active) { struct ice_dpll_pin *target; enum ice_dpll_pin_type type; @@ -1406,7 +1403,7 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, unlock: mutex_unlock(&pf->dplls.lock); if (!ret) - ice_dpll_sw_pin_notify_peer(&pf->dplls, p); + ice_dpll_sw_pin_notify_peer(p); return ret; } @@ -1526,7 +1523,7 @@ ice_dpll_sma_pin_state_set(const struct dpll_pin *pin, void *pin_priv, unlock: mutex_unlock(&pf->dplls.lock); if (!ret) - ice_dpll_sw_pin_notify_peer(&pf->dplls, sma); + ice_dpll_sw_pin_notify_peer(sma); return ret; } @@ -1723,7 +1720,7 @@ ice_dpll_pin_sma_direction_set(const struct dpll_pin *pin, void *pin_priv, ret = ice_dpll_sma_direction_set(p, direction, extack); mutex_unlock(&pf->dplls.lock); if (!ret) - ice_dpll_sw_pin_notify_peer(&pf->dplls, p); + ice_dpll_sw_pin_notify_peer(p); return ret; } @@ -4909,6 +4906,8 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf) if (pin->input->ref_sync) pin->ref_sync = pin->input->ref_sync - pin_abs_idx; pin->output = &d->outputs[ICE_DPLL_PIN_SW_OUTPUT_ABS(i)]; + pin->muxed = &d->ufl[i]; + ice_dpll_phase_range_set(&pin->prop.phase_range, phase_adj_max); } for (i = 0; i < ICE_DPLL_PIN_SW_NUM; i++) { @@ -4942,6 +4941,7 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf) (DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE | caps); } + pin->muxed = &d->sma[i]; ice_dpll_phase_range_set(&pin->prop.phase_range, phase_adj_max); } diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h index bce0bea9a664..f7e6680d124c 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.h +++ b/drivers/net/ethernet/intel/ice/ice_dpll.h @@ -76,6 +76,7 @@ struct ice_dpll_pin { s32 phase_adjust; struct ice_dpll_pin *input; struct ice_dpll_pin *output; + struct ice_dpll_pin *muxed; enum dpll_pin_direction direction; s64 phase_offset; u8 status; -- 2.53.0 Use switch statements to handle pin states to make the code more readable. This also makes this code more future-proof, should any new states appear. This also changes how direction-mismatched state requests are handled in ice_dpll_sma_pin_state_set(). Previously, requesting CONNECTED on an INPUT-direction SMA pin or SELECTABLE on an OUTPUT-direction pin would fall through to ice_dpll_pin_disable() and return success. After this change those requests return -EINVAL without issuing a firmware command. Because the DPLL netlink core does not filter pin states by direction before calling the driver, this is a user-visible netlink API change. Signed-off-by: Sergey Temerkhanov Reviewed-by: Aleksandr Loktionov Reviewed-by: Przemyslaw Korba --- drivers/net/ethernet/intel/ice/ice_dpll.c | 72 ++++++++++++++++++----- 1 file changed, 57 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index 4dcbbabee7aa..f4d6c247e96e 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -1338,10 +1338,12 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, ret = -EINVAL; switch (p->idx) { case ICE_DPLL_PIN_SW_1_IDX: - if (state == DPLL_PIN_STATE_CONNECTED) { + switch (state) { + case DPLL_PIN_STATE_CONNECTED: data &= ~ICE_SMA1_MASK; enable = true; - } else if (state == DPLL_PIN_STATE_DISCONNECTED) { + break; + case DPLL_PIN_STATE_DISCONNECTED: /* Skip if U.FL1 is not active, setting TX_EN * while DIR_EN is set would also deactivate * the paired SMA1 output. @@ -1352,18 +1354,21 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, } data |= ICE_SMA1_TX_EN; enable = false; - } else { + break; + default: goto unlock; } target = p->output; type = ICE_DPLL_PIN_TYPE_OUTPUT; break; case ICE_DPLL_PIN_SW_2_IDX: - if (state == DPLL_PIN_STATE_SELECTABLE) { + switch (state) { + case DPLL_PIN_STATE_SELECTABLE: data |= ICE_SMA2_DIR_EN; data &= ~ICE_SMA2_UFL2_RX_DIS; enable = true; - } else if (state == DPLL_PIN_STATE_DISCONNECTED) { + break; + case DPLL_PIN_STATE_DISCONNECTED: /* Skip if U.FL2 is not active, setting * UFL2_RX_DIS could also disable the paired * SMA2 input. @@ -1375,7 +1380,8 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, } data |= ICE_SMA2_UFL2_RX_DIS; enable = false; - } else { + break; + default: goto unlock; } target = p->input; @@ -1497,19 +1503,49 @@ ice_dpll_sma_pin_state_set(const struct dpll_pin *pin, void *pin_priv, return -EBUSY; mutex_lock(&pf->dplls.lock); - if (!sma->active) { - ret = ice_dpll_sma_direction_set(sma, sma->direction, extack); - if (ret) + switch (state) { + case DPLL_PIN_STATE_SELECTABLE: + if (sma->direction == DPLL_PIN_DIRECTION_OUTPUT) { + enable = false; + ret = -EINVAL; + goto unlock; + } + enable = true; + break; + case DPLL_PIN_STATE_CONNECTED: + if (sma->direction == DPLL_PIN_DIRECTION_INPUT) { + enable = false; + ret = -EINVAL; goto unlock; + } + enable = true; + break; + case DPLL_PIN_STATE_DISCONNECTED: + enable = false; + break; + default: + ret = -EINVAL; + goto unlock; } - if (sma->direction == DPLL_PIN_DIRECTION_INPUT) { - enable = state == DPLL_PIN_STATE_SELECTABLE; + + switch (sma->direction) { + case DPLL_PIN_DIRECTION_INPUT: target = sma->input; type = ICE_DPLL_PIN_TYPE_INPUT; - } else { - enable = state == DPLL_PIN_STATE_CONNECTED; + break; + case DPLL_PIN_DIRECTION_OUTPUT: target = sma->output; type = ICE_DPLL_PIN_TYPE_OUTPUT; + break; + default: + ret = -EINVAL; + goto unlock; + } + + if (!sma->active) { + ret = ice_dpll_sma_direction_set(sma, sma->direction, extack); + if (ret) + goto unlock; } if (enable) @@ -4917,7 +4953,8 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf) pin->prop.capabilities = caps; pin->pf = pf; pin->prop.board_label = ice_dpll_sw_pin_ufl[i]; - if (i == ICE_DPLL_PIN_SW_1_IDX) { + switch (i) { + case ICE_DPLL_PIN_SW_1_IDX: pin->direction = DPLL_PIN_DIRECTION_OUTPUT; pin_abs_idx = ICE_DPLL_PIN_SW_OUTPUT_ABS(i); pin->prop.freq_supported = @@ -4927,7 +4964,8 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf) pin->prop.freq_supported_num = freq_supp_num; pin->input = NULL; pin->output = &d->outputs[pin_abs_idx]; - } else if (i == ICE_DPLL_PIN_SW_2_IDX) { + break; + case ICE_DPLL_PIN_SW_2_IDX: pin->direction = DPLL_PIN_DIRECTION_INPUT; pin_abs_idx = ICE_DPLL_PIN_SW_INPUT_ABS(i) + input_idx_offset; @@ -4940,6 +4978,10 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf) pin->prop.capabilities = (DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE | caps); + break; + default: + dev_err(ice_pf_to_dev(pf), "Invalid U.FL pin index: %d\n", i); + return -EINVAL; } pin->muxed = &d->sma[i]; ice_dpll_phase_range_set(&pin->prop.phase_range, phase_adj_max); -- 2.53.0 Add bounds checking code so that any potential out-of-bound array access is avoided Signed-off-by: Sergey Temerkhanov Reviewed-by: Aleksandr Loktionov --- drivers/net/ethernet/intel/ice/ice_dpll.c | 76 ++++++++++++++--------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index f4d6c247e96e..d0225097cd10 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -760,6 +760,9 @@ ice_dpll_pin_state_update(struct ice_pf *pf, struct ice_dpll_pin *pin, u8 parent, port_num = ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT; int ret; + for (int i = 0; i < ICE_DPLL_RCLK_NUM_MAX; i++) + pin->state[i] = DPLL_PIN_STATE_DISCONNECTED; + switch (pin_type) { case ICE_DPLL_PIN_TYPE_INPUT: ret = ice_aq_get_input_pin_cfg(&pf->hw, pin->idx, &pin->status, @@ -769,25 +772,31 @@ ice_dpll_pin_state_update(struct ice_pf *pf, struct ice_dpll_pin *pin, 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; + if (pf->dplls.eec.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + pin->state[pf->dplls.eec.dpll_idx] = + pin->pin == pf->dplls.eec.active_input ? + DPLL_PIN_STATE_CONNECTED : + DPLL_PIN_STATE_SELECTABLE; + if (pf->dplls.pps.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + 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; + if (pf->dplls.eec.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + pin->state[pf->dplls.eec.dpll_idx] = + DPLL_PIN_STATE_SELECTABLE; + if (pf->dplls.pps.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + 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; + if (pf->dplls.eec.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + pin->state[pf->dplls.eec.dpll_idx] = + DPLL_PIN_STATE_DISCONNECTED; + if (pf->dplls.pps.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + pin->state[pf->dplls.pps.dpll_idx] = + DPLL_PIN_STATE_DISCONNECTED; } break; case ICE_DPLL_PIN_TYPE_OUTPUT: @@ -799,19 +808,23 @@ ice_dpll_pin_state_update(struct ice_pf *pf, struct ice_dpll_pin *pin, parent &= ICE_AQC_GET_CGU_OUT_CFG_DPLL_SRC_SEL; if (ICE_AQC_GET_CGU_OUT_CFG_OUT_EN & pin->flags[0]) { - pin->state[pf->dplls.eec.dpll_idx] = - parent == pf->dplls.eec.dpll_idx ? - DPLL_PIN_STATE_CONNECTED : - DPLL_PIN_STATE_DISCONNECTED; - pin->state[pf->dplls.pps.dpll_idx] = - parent == pf->dplls.pps.dpll_idx ? - DPLL_PIN_STATE_CONNECTED : - DPLL_PIN_STATE_DISCONNECTED; + if (pf->dplls.eec.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + pin->state[pf->dplls.eec.dpll_idx] = + parent == pf->dplls.eec.dpll_idx ? + DPLL_PIN_STATE_CONNECTED : + DPLL_PIN_STATE_DISCONNECTED; + if (pf->dplls.pps.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + pin->state[pf->dplls.pps.dpll_idx] = + parent == pf->dplls.pps.dpll_idx ? + DPLL_PIN_STATE_CONNECTED : + DPLL_PIN_STATE_DISCONNECTED; } else { - pin->state[pf->dplls.eec.dpll_idx] = - DPLL_PIN_STATE_DISCONNECTED; - pin->state[pf->dplls.pps.dpll_idx] = - DPLL_PIN_STATE_DISCONNECTED; + if (pf->dplls.eec.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + pin->state[pf->dplls.eec.dpll_idx] = + DPLL_PIN_STATE_DISCONNECTED; + if (pf->dplls.pps.dpll_idx < ICE_DPLL_RCLK_NUM_MAX) + pin->state[pf->dplls.pps.dpll_idx] = + DPLL_PIN_STATE_DISCONNECTED; } break; case ICE_DPLL_PIN_TYPE_RCLK_INPUT: @@ -5180,6 +5193,13 @@ static int ice_dpll_init_info(struct ice_pf *pf, bool cgu) de->dpll_idx = abilities.eec_dpll_idx; dp->dpll_idx = abilities.pps_dpll_idx; + if (de->dpll_idx >= ICE_DPLL_RCLK_NUM_MAX || + dp->dpll_idx >= ICE_DPLL_RCLK_NUM_MAX) { + dev_err(ice_pf_to_dev(pf), + "invalid dpll_idx in cgu abilities: eec=%u, pps=%u\n", + de->dpll_idx, dp->dpll_idx); + return -EINVAL; + } 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) & -- 2.53.0 Rework multiplexed pin control to match the state change listed in the design requirements. Signed-off-by: Sergey Temerkhanov Reviewed-by: Aleksandr Loktionov Reviewed-by: Przemyslaw Korba --- drivers/net/ethernet/intel/ice/ice_dpll.c | 191 +++++++++++++++++++++- 1 file changed, 183 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index d0225097cd10..0081544b769c 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -469,6 +469,40 @@ ice_dpll_sw_pin_frequency_get(const struct dpll_pin *pin, void *pin_priv, return ret; } +static int ice_dpll_get_fallback_idx(struct ice_dplls *d, struct ice_dpll_pin *target) +{ + if (d->pps.dpll_idx < ICE_DPLL_RCLK_NUM_MAX && + target->state[d->pps.dpll_idx] == DPLL_PIN_STATE_CONNECTED) + return d->pps.dpll_idx; + + if (d->eec.dpll_idx < ICE_DPLL_RCLK_NUM_MAX && + target->state[d->eec.dpll_idx] == DPLL_PIN_STATE_CONNECTED) + return d->eec.dpll_idx; + + return -EINVAL; +} + +static void ice_dpll_get_peer_target(struct ice_dpll_pin *peer, + struct ice_dpll_pin **peer_target, + enum ice_dpll_pin_type *peer_type) +{ + *peer_target = NULL; + *peer_type = 0; + + switch (peer->direction) { + case DPLL_PIN_DIRECTION_OUTPUT: + *peer_target = peer->output; + *peer_type = ICE_DPLL_PIN_TYPE_OUTPUT; + break; + case DPLL_PIN_DIRECTION_INPUT: + *peer_target = peer->input; + *peer_type = ICE_DPLL_PIN_TYPE_INPUT; + break; + default: + break; + } +} + /** * ice_dpll_pin_enable - enable a pin on dplls * @hw: board private hw structure @@ -1225,6 +1259,87 @@ ice_dpll_input_state_get(const struct dpll_pin *pin, void *pin_priv, extack, ICE_DPLL_PIN_TYPE_INPUT); } +struct ice_dpll_pin_config { + bool enabled; + u8 dpll_idx; +}; + +static int +ice_dpll_pin_config_get(struct ice_pf *pf, struct ice_dpll_pin *target, + enum ice_dpll_pin_type type, + struct ice_dpll_pin_config *config, + struct netlink_ext_ack *extack) +{ + int ret; + + ret = ice_dpll_pin_state_update(pf, target, type, extack); + if (ret) + return ret; + + switch (type) { + case ICE_DPLL_PIN_TYPE_INPUT: + config->enabled = target->flags[0] & + ICE_AQC_GET_CGU_IN_CFG_FLG2_INPUT_EN; + config->dpll_idx = 0; + return 0; + case ICE_DPLL_PIN_TYPE_OUTPUT: + config->enabled = target->flags[0] & + ICE_AQC_GET_CGU_OUT_CFG_OUT_EN; + if (!config->enabled) { + config->dpll_idx = 0; + return 0; + } + + ret = ice_dpll_get_fallback_idx(&pf->dplls, target); + if (ret < 0) + return ret; + config->dpll_idx = ret; + return 0; + default: + return -EINVAL; + } +} + +static int +ice_dpll_pin_config_restore(struct ice_pf *pf, struct ice_dpll_pin *target, + enum ice_dpll_pin_type type, + const struct ice_dpll_pin_config *config, + struct netlink_ext_ack *extack, int init_err) +{ + int restore_err, update_err; + + if (config->enabled) + restore_err = ice_dpll_pin_enable(&pf->hw, target, + config->dpll_idx, type, extack); + else + restore_err = ice_dpll_pin_disable(&pf->hw, target, type, extack); + update_err = ice_dpll_pin_state_update(pf, target, type, extack); + if (restore_err || update_err) { + dev_warn(ice_pf_to_dev(pf), + "Failed to rollback pin state: err %d, restore err %d, update err %d\n", + init_err, restore_err, update_err); + return restore_err ? restore_err : update_err; + } + + return 0; +} + +static int ice_dpll_restore_sma_ctrl(struct ice_pf *pf, u8 old_data, + int init_err) +{ + int restore_err; + + restore_err = ice_write_sma_ctrl(&pf->hw, old_data); + if (!restore_err) + restore_err = ice_dpll_sw_pins_update(pf); + if (restore_err) + dev_warn(ice_pf_to_dev(pf), + "Failed to restore SMA control: err %d, restore err %d\n", + init_err, restore_err); + + return restore_err; +} + /** * ice_dpll_sma_direction_set - set direction of SMA pin * @p: pointer to a pin @@ -1290,6 +1405,7 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p, if (peer->active) { struct ice_dpll_pin *target; enum ice_dpll_pin_type type; + int peer_ret; if (peer->output) { target = peer->output; @@ -1298,8 +1414,11 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p, target = peer->input; type = ICE_DPLL_PIN_TYPE_INPUT; } - ret = ice_dpll_pin_enable(&p->pf->hw, target, - d->eec.dpll_idx, type, extack); + peer_ret = ice_dpll_get_fallback_idx(d, target); + if (peer_ret < 0) + peer_ret = d->eec.dpll_idx; + ret = ice_dpll_pin_enable(&p->pf->hw, target, peer_ret, type, + extack); if (!ret) ret = ice_dpll_pin_state_update(p->pf, target, type, extack); @@ -1331,13 +1450,17 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, struct netlink_ext_ack *extack) { struct ice_dpll_pin *p = pin_priv, *target; + struct ice_dpll_pin *peer_target = NULL; struct ice_dpll *d = dpll_priv; - enum ice_dpll_pin_type type; + enum ice_dpll_pin_type peer_type = 0, type; struct ice_pf *pf = p->pf; + struct ice_dpll_pin_config old_config; + struct ice_dpll_pin_config peer_config; struct ice_hw *hw; + u8 old_data = 0; bool enable; u8 data; - int ret; + int peer_dpll_idx, restore_err, ret; if (ice_dpll_is_reset(pf, extack)) return -EBUSY; @@ -1347,6 +1470,7 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, ret = ice_read_sma_ctrl(hw, &data); if (ret) goto unlock; + old_data = data; ret = -EINVAL; switch (p->idx) { @@ -1406,18 +1530,69 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, ret = ice_write_sma_ctrl(hw, data); if (ret) - goto unlock; + goto restore_sma_ctrl; ret = ice_dpll_pin_state_update(pf, p, ICE_DPLL_PIN_TYPE_SOFTWARE, extack); if (ret) - goto unlock; + goto restore_sma_ctrl; + + ret = ice_dpll_pin_config_get(pf, target, type, &old_config, extack); + if (ret) + goto restore_sma_ctrl; if (enable) ret = ice_dpll_pin_enable(hw, target, d->dpll_idx, type, extack); else ret = ice_dpll_pin_disable(hw, target, type, extack); - if (!ret) - ret = ice_dpll_pin_state_update(pf, target, type, extack); + if (ret) + goto restore_target; + ret = ice_dpll_pin_state_update(pf, target, type, extack); + if (ret) + goto restore_target; + + if (enable && p->muxed && p->muxed->active) { + ice_dpll_get_peer_target(p->muxed, &peer_target, &peer_type); + if (!peer_target) { + ret = -EINVAL; + goto restore_target; + } + ret = ice_dpll_pin_config_get(pf, peer_target, peer_type, + &peer_config, extack); + if (ret) + goto restore_target; + + peer_dpll_idx = ice_dpll_get_fallback_idx(&pf->dplls, + peer_target); + if (peer_dpll_idx < 0) + peer_dpll_idx = d->dpll_idx; + ret = ice_dpll_pin_enable(&pf->hw, peer_target, peer_dpll_idx, + peer_type, extack); + if (ret) + goto restore_peer; + ret = ice_dpll_pin_state_update(pf, peer_target, peer_type, + extack); + if (ret) + goto restore_peer; + } + + goto unlock; + +restore_peer: + restore_err = + ice_dpll_pin_config_restore(pf, peer_target, peer_type, + &peer_config, extack, ret); + if (restore_err) + ret = restore_err; +restore_target: + restore_err = + ice_dpll_pin_config_restore(pf, target, type, &old_config, + extack, ret); + if (restore_err) + ret = restore_err; +restore_sma_ctrl: + restore_err = ice_dpll_restore_sma_ctrl(pf, old_data, ret); + if (restore_err) + ret = restore_err; unlock: mutex_unlock(&pf->dplls.lock); -- 2.53.0 Make the SMA control logic match the requirements: Setting SMA1 as Rx automatically enables U.FL1 as Tx if U.FL1 is disconnected. Setting SMA1 as Tx automatically changes U.FL1 state to disconnected. Setting SMA2 as Tx automatically enables U.FL2 as Rx if U.FL2 is disconnected. Setting SMA2 as Rx automatically changes U.FL2 state to disconnected. Signed-off-by: Sergey Temerkhanov Reviewed-by: Przemyslaw Korba --- drivers/net/ethernet/intel/ice/ice_dpll.c | 172 ++++++++++++++++++---- 1 file changed, 146 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index 0081544b769c..9e3e69e523c9 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -1359,28 +1359,37 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p, { struct ice_dplls *d = &p->pf->dplls; struct ice_dpll_pin *peer; - u8 data; - int ret; + struct ice_dpll_pin *target = NULL; + struct ice_dpll_pin_config peer_config; + enum ice_dpll_pin_type type = 0; + u8 data, old_data = 0; + int peer_dpll_idx, restore_err, ret; if (p->direction == direction && p->active) return 0; ret = ice_read_sma_ctrl(&p->pf->hw, &data); if (ret) return ret; + old_data = data; switch (p->idx) { case ICE_DPLL_PIN_SW_1_IDX: - data &= ~ICE_SMA1_MASK; - if (direction == DPLL_PIN_DIRECTION_OUTPUT) + if (direction == DPLL_PIN_DIRECTION_OUTPUT) { + data &= ~ICE_SMA1_TX_EN; data |= ICE_SMA1_DIR_EN; + } else { + data &= ~ICE_SMA1_DIR_EN; + data &= ~ICE_SMA1_TX_EN; + } break; case ICE_DPLL_PIN_SW_2_IDX: if (direction == DPLL_PIN_DIRECTION_INPUT) { data &= ~ICE_SMA2_DIR_EN; - data |= ICE_SMA2_UFL2_RX_DIS; + data &= ~ICE_SMA2_TX_EN; } else { - data &= ~(ICE_SMA2_TX_EN | ICE_SMA2_UFL2_RX_DIS); data |= ICE_SMA2_DIR_EN; + data &= ~ICE_SMA2_UFL2_RX_DIS; + data &= ~ICE_SMA2_TX_EN; } break; default: @@ -1392,7 +1401,7 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p, ICE_DPLL_PIN_TYPE_SOFTWARE, extack); if (ret) - return ret; + goto restore_sma; /* When a direction change activates the paired U.FL pin, enable * its backing CGU pin so the pin reports as connected. Without @@ -1403,27 +1412,44 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p, */ peer = p->muxed; if (peer->active) { - struct ice_dpll_pin *target; - enum ice_dpll_pin_type type; - int peer_ret; - if (peer->output) { target = peer->output; type = ICE_DPLL_PIN_TYPE_OUTPUT; - } else { + } else if (peer->input) { target = peer->input; type = ICE_DPLL_PIN_TYPE_INPUT; + } else { + ret = -EINVAL; + goto restore_sma; } - peer_ret = ice_dpll_get_fallback_idx(d, target); - if (peer_ret < 0) - peer_ret = d->eec.dpll_idx; - ret = ice_dpll_pin_enable(&p->pf->hw, target, peer_ret, type, - extack); - if (!ret) - ret = ice_dpll_pin_state_update(p->pf, target, - type, extack); + + ret = ice_dpll_pin_config_get(p->pf, target, type, + &peer_config, extack); + if (ret) + goto restore_sma; + + peer_dpll_idx = ice_dpll_get_fallback_idx(d, target); + if (peer_dpll_idx < 0) + peer_dpll_idx = d->eec.dpll_idx; + ret = ice_dpll_pin_enable(&p->pf->hw, target, peer_dpll_idx, + type, extack); + if (ret) + goto restore_peer; + ret = ice_dpll_pin_state_update(p->pf, target, type, extack); + if (ret) + goto restore_peer; } + return 0; + +restore_peer: + restore_err = + ice_dpll_pin_config_restore(p->pf, target, type, &peer_config, + extack, ret); + if (restore_err) + ret = restore_err; +restore_sma: + ice_dpll_restore_sma_ctrl(p->pf, old_data, ret); return ret; } @@ -1681,11 +1707,15 @@ ice_dpll_sma_pin_state_set(const struct dpll_pin *pin, void *pin_priv, struct netlink_ext_ack *extack) { struct ice_dpll_pin *sma = pin_priv, *target; + struct ice_dpll_pin *peer_target = NULL; struct ice_dpll *d = dpll_priv; struct ice_pf *pf = sma->pf; - enum ice_dpll_pin_type type; + enum ice_dpll_pin_type peer_type = 0, type; + struct ice_dpll_pin_config old_config; + struct ice_dpll_pin_config peer_config; + u8 old_data = 0; bool enable; - int ret; + int peer_dpll_idx, restore_err, ret = -EINVAL; if (ice_dpll_is_reset(pf, extack)) return -EBUSY; @@ -1693,6 +1723,10 @@ ice_dpll_sma_pin_state_set(const struct dpll_pin *pin, void *pin_priv, mutex_lock(&pf->dplls.lock); switch (state) { case DPLL_PIN_STATE_SELECTABLE: + /* Reject direction-mismatched requests: SELECTABLE + OUTPUT + * is an invalid combination, return -EINVAL in this case + * immediately instead of disabling the pin. + */ if (sma->direction == DPLL_PIN_DIRECTION_OUTPUT) { enable = false; ret = -EINVAL; @@ -1701,6 +1735,10 @@ ice_dpll_sma_pin_state_set(const struct dpll_pin *pin, void *pin_priv, enable = true; break; case DPLL_PIN_STATE_CONNECTED: + /* Reject direction-mismatched requests: CONNECTED + INPUT + * is an invalid combination, return -EINVAL in this case + * immediately instead of disabling the pin. + */ if (sma->direction == DPLL_PIN_DIRECTION_INPUT) { enable = false; ret = -EINVAL; @@ -1736,13 +1774,95 @@ ice_dpll_sma_pin_state_set(const struct dpll_pin *pin, void *pin_priv, goto unlock; } - if (enable) + if (enable) { + u8 data; + + ret = ice_read_sma_ctrl(&pf->hw, &data); + if (ret) + goto unlock; + old_data = data; + switch (sma->idx) { + case ICE_DPLL_PIN_SW_1_IDX: + data &= ~ICE_SMA1_TX_EN; + break; + case ICE_DPLL_PIN_SW_2_IDX: + data &= ~ICE_SMA2_UFL2_RX_DIS; + data &= ~ICE_SMA2_TX_EN; + break; + default: + ret = -EINVAL; + goto unlock; + } + ret = ice_write_sma_ctrl(&pf->hw, data); + if (ret) + goto restore_sma_ctrl; + ret = ice_dpll_sw_pins_update(pf); + if (ret) + goto restore_sma_ctrl; + + ret = ice_dpll_pin_config_get(pf, target, type, &old_config, + extack); + if (ret) + goto restore_sma_ctrl; ret = ice_dpll_pin_enable(&pf->hw, target, d->dpll_idx, type, extack); - else - ret = ice_dpll_pin_disable(&pf->hw, target, type, extack); - if (!ret) + if (ret) + goto restore_target; + /* Refresh target state before peer-side operations. */ ret = ice_dpll_pin_state_update(pf, target, type, extack); + if (ret) + goto restore_target; + + if (sma->muxed && sma->muxed->active) { + ice_dpll_get_peer_target(sma->muxed, &peer_target, + &peer_type); + if (!peer_target) { + ret = -EINVAL; + goto restore_target; + } + + ret = ice_dpll_pin_config_get(pf, peer_target, peer_type, + &peer_config, extack); + if (ret) + goto restore_target; + + peer_dpll_idx = + ice_dpll_get_fallback_idx(&pf->dplls, peer_target); + if (peer_dpll_idx < 0) + peer_dpll_idx = d->dpll_idx; + ret = ice_dpll_pin_enable(&pf->hw, peer_target, + peer_dpll_idx, peer_type, extack); + if (ret) + goto restore_peer; + ret = ice_dpll_pin_state_update(pf, peer_target, peer_type, + extack); + if (ret) + goto restore_peer; + } + } else { + ret = ice_dpll_pin_disable(&pf->hw, target, type, extack); + if (!ret) + ret = ice_dpll_pin_state_update(pf, target, type, + extack); + } + goto unlock; + +restore_peer: + restore_err = + ice_dpll_pin_config_restore(pf, peer_target, peer_type, + &peer_config, extack, ret); + if (restore_err) + ret = restore_err; +restore_target: + restore_err = + ice_dpll_pin_config_restore(pf, target, type, &old_config, + extack, ret); + if (restore_err) + ret = restore_err; +restore_sma_ctrl: + restore_err = ice_dpll_restore_sma_ctrl(pf, old_data, ret); + if (restore_err) + ret = restore_err; unlock: mutex_unlock(&pf->dplls.lock); -- 2.53.0