>From: Petr Oros <[email protected]>
>Sent: Wednesday, April 8, 2026 1:05 PM
>
>SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1 and
>SMA2/U.FL2) controlled by the PCA9575 GPIO expander.  Each pair can
>only have one active pin at a time: SMA1 output and U.FL1 output share
>the same CGU output, SMA2 input and U.FL2 input share the same CGU
>input.  The PCA9575 register bits determine which connector in each
>pair owns the signal path.
>
>The driver does not account for this pairing in two places:
>
>ice_dpll_ufl_pin_state_set() modifies PCA9575 bits and disables the
>backing CGU pin without checking whether the U.FL pin is currently
>active.  Disconnecting an already inactive U.FL pin flips bits that
>the paired SMA pin relies on, breaking its connection.
>
>ice_dpll_sma_direction_set() does not propagate direction changes to
>the paired U.FL pin.  For SMA2/U.FL2 the ICE_SMA2_UFL2_RX_DIS bit is
>never managed, so U.FL2 stays disconnected after SMA2 switches to
>output.  For both pairs the backing CGU pin of the U.FL side is never
>enabled when a direction change activates it, so userspace sees the
>pin as disconnected even though the routing is correct.
>
>Fix by guarding the U.FL disconnect path against inactive pins and by
>updating the paired U.FL pin fully on SMA direction changes: manage
>ICE_SMA2_UFL2_RX_DIS for the SMA2/U.FL2 pair and enable the backing
>CGU pin whenever the peer becomes active.
>
>Fixes: 2dd5d03c77e2 ("ice: redesign dpll sma/u.fl pins control")

LGTM,

Reviewed-by: Arkadiusz Kubalewski <[email protected]>

>Signed-off-by: Petr Oros <[email protected]>
>---
>v2:
> - fix ice_dpll_sma_direction_set() to manage ICE_SMA2_UFL2_RX_DIS
>   when SMA2 direction changes
> - enable paired U.FL backing CGU pin when direction change makes
>   it active, so it reports as connected immediately
> - (both reported by Intel test on the SMA init and notification
>   patch threads)
>v1: https://lore.kernel.org/all/[email protected]/
>---
> drivers/net/ethernet/intel/ice/ice_dpll.c | 50 ++++++++++++++++++++++-
> 1 file changed, 49 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
>b/drivers/net/ethernet/intel/ice/ice_dpll.c
>index 498ec2c045f384..3f8cd5b8298b57 100644
>--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>@@ -1171,6 +1171,8 @@ static int ice_dpll_sma_direction_set(struct
>ice_dpll_pin *p,
>                                     enum dpll_pin_direction direction,
>                                     struct netlink_ext_ack *extack)
> {
>+      struct ice_dplls *d = &p->pf->dplls;
>+      struct ice_dpll_pin *peer;
>       u8 data;
>       int ret;
>
>@@ -1189,8 +1191,9 @@ static int ice_dpll_sma_direction_set(struct
>ice_dpll_pin *p,
>       case ICE_DPLL_PIN_SW_2_IDX:
>               if (direction == DPLL_PIN_DIRECTION_INPUT) {
>                       data &= ~ICE_SMA2_DIR_EN;
>+                      data |= ICE_SMA2_UFL2_RX_DIS;
>               } else {
>-                      data &= ~ICE_SMA2_TX_EN;
>+                      data &= ~(ICE_SMA2_TX_EN | ICE_SMA2_UFL2_RX_DIS);
>                       data |= ICE_SMA2_DIR_EN;
>               }
>               break;
>@@ -1202,6 +1205,34 @@ static int ice_dpll_sma_direction_set(struct
>ice_dpll_pin *p,
>               ret = ice_dpll_pin_state_update(p->pf, p,
>                                               ICE_DPLL_PIN_TYPE_SOFTWARE,
>                                               extack);
>+      if (ret)
>+              return ret;
>+
>+      /* When a direction change activates the paired U.FL pin, enable
>+       * its backing CGU pin so the pin reports as connected. Without
>+       * this the U.FL routing is correct but the CGU pin stays disabled
>+       * and userspace sees the pin as disconnected.  Do not disable the
>+       * backing pin when U.FL becomes inactive because the SMA pin may
>+       * still be using it.
>+       */
>+      peer = &d->ufl[p->idx];
>+      if (peer->active) {
>+              struct ice_dpll_pin *target;
>+              enum ice_dpll_pin_type type;
>+
>+              if (peer->output) {
>+                      target = peer->output;
>+                      type = ICE_DPLL_PIN_TYPE_OUTPUT;
>+              } else {
>+                      target = peer->input;
>+                      type = ICE_DPLL_PIN_TYPE_INPUT;
>+              }
>+              ret = ice_dpll_pin_enable(&p->pf->hw, target,
>+                                        d->eec.dpll_idx, type, extack);
>+              if (!ret)
>+                      ret = ice_dpll_pin_state_update(p->pf, target,
>+                                                      type, extack);
>+      }
>
>       return ret;
> }
>@@ -1253,6 +1284,14 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin
>*pin, void *pin_priv,
>                       data &= ~ICE_SMA1_MASK;
>                       enable = true;
>               } else if (state == 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.
>+                       */
>+                      if (data & (ICE_SMA1_DIR_EN | ICE_SMA1_TX_EN)) {
>+                              ret = 0;
>+                              goto unlock;
>+                      }
>                       data |= ICE_SMA1_TX_EN;
>                       enable = false;
>               } else {
>@@ -1267,6 +1306,15 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin
>*pin, void *pin_priv,
>                       data &= ~ICE_SMA2_UFL2_RX_DIS;
>                       enable = true;
>               } else if (state == DPLL_PIN_STATE_DISCONNECTED) {
>+                      /* Skip if U.FL2 is not active, setting
>+                       * UFL2_RX_DIS could also disable the paired
>+                       * SMA2 input.
>+                       */
>+                      if (!(data & ICE_SMA2_DIR_EN) ||
>+                          (data & ICE_SMA2_UFL2_RX_DIS)) {
>+                              ret = 0;
>+                              goto unlock;
>+                      }
>                       data |= ICE_SMA2_UFL2_RX_DIS;
>                       enable = false;
>               } else {
>--
>2.52.0

Reply via email to