RE: [Intel-wired-lan] [PATCH iwl-net] ice: fix U.FL pin state set affecting paired SMA pin

["Loktionov, Aleksandr" <aleksandr.loktionov@intel.com>]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Petr Oros
> Sent: Wednesday, March 25, 2026 4:11 PM
> To: netdev@vger.kernel.org
> Cc: Vadim Fedorenko <vadim.fedorenko@linux.dev>; Michal Michalik
> <michal.michalik@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; linux-kernel@vger.kernel.org; Eric
> Dumazet <edumazet@google.com>; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> Nguyen, Anthony L <anthony.l.nguyen@intel.com>; intel-wired-
> lan@lists.osuosl.org; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; David S. Miller <davem@davemloft.net>; Olech,
> Milena <milena.olech@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net] ice: fix U.FL pin state set
> affecting paired SMA pin
> 
> When setting a U.FL pin to disconnected state,
> ice_dpll_ufl_pin_state_set() blindly modifies SMA control register
> bits and disables the shared underlying output/input pin without
> checking whether the U.FL pin is currently active.
> 
> SMA1 and U.FL1 share the same physical output pin, controlled by
> ICE_SMA1_DIR_EN and ICE_SMA1_TX_EN bits. When SMA1 is in output mode
> (DIR_EN=1, TX_EN=0), U.FL1 is already inactive. Disconnecting U.FL1
> sets TX_EN=1, which combined with DIR_EN=1 causes
> ice_dpll_sw_pins_update() to mark SMA1 as inactive too. The subsequent
> ice_dpll_pin_disable() then disables the shared output pin entirely,
> breaking SMA1's connection.
> 
> Fix by checking whether U.FL1/U.FL2 is already inactive before
> proceeding with the disconnect. If the pin is not currently
> controlling the shared output/input, return success immediately
> without modifying the SMA control register or disabling the underlying
> pin.
> 
> Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_dpll.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
> b/drivers/net/ethernet/intel/ice/ice_dpll.c
> index 5cfa19da099bfc..76c68f54a1cc97 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> @@ -1253,6 +1253,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 +1275,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

I'd recommend adding Cc: stable@vger.kernel.org
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>