Re: [PATCH net] ice: fix missing SMA pin initialization in DPLL subsystem

[Ivan Vecera <ivecera@redhat.com>]

On 2/13/26 3:16 PM, Petr Oros wrote:
> The DPLL SMA/U.FL pin redesign introduced ice_dpll_sw_pin_frequency_get()
> which gates frequency reporting on the pin's active flag. This flag is
> determined by ice_dpll_sw_pins_update() from the PCA9575 GPIO expander
> state. Before the redesign, SMA pins were exposed as direct HW
> input/output pins and ice_dpll_frequency_get() returned the CGU
> frequency unconditionally — the PCA9575 state was never consulted.
> 
> The PCA9575 powers on with all outputs high, setting ICE_SMA1_DIR_EN,
> ICE_SMA1_TX_EN, ICE_SMA2_DIR_EN and ICE_SMA2_TX_EN. Nothing in the
> driver writes the register during initialization, so
> ice_dpll_sw_pins_update() sees all pins as inactive and
> ice_dpll_sw_pin_frequency_get() permanently returns 0 Hz for every
> SW pin.
> 
> Fix this by writing a default SMA configuration in
> ice_dpll_init_info_sw_pins(): clear all SMA bits, then set SMA1 and
> SMA2 as active inputs (DIR_EN=0) with U.FL1 output and U.FL2 input
> disabled. Each SMA/U.FL pair shares a physical signal path so only
> one pin per pair can be active at a time. U.FL pins still report
> frequency 0 after this fix: U.FL1 (output-only) is disabled by
> ICE_SMA1_TX_EN which keeps the TX output buffer off, and U.FL2
> (input-only) is disabled by ICE_SMA2_UFL2_RX_DIS. They can be
> activated by changing the corresponding SMA pin direction via dpll
> netlink.
> 
> Fixes: 2dd5d03c77e2 ("ice: redesign dpll sma/u.fl pins control")
> 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 53b54e395a2ed8..c2ad39bfe177db 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> @@ -3545,6 +3545,7 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
>   	struct ice_dpll_pin *pin;
>   	u32 phase_adj_max, caps;
>   	int i, ret;
> +	u8 data;
>   
>   	if (pf->hw.device_id == ICE_DEV_ID_E810C_QSFP)
>   		input_idx_offset = ICE_E810_RCLK_PINS_NUM;
> @@ -3604,6 +3605,22 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
>   		}
>   		ice_dpll_phase_range_set(&pin->prop.phase_range, phase_adj_max);
>   	}
> +
> +	/* Initialize the SMA control register to a known-good default state.
> +	 * Without this write the PCA9575 GPIO expander retains its power-on
> +	 * default (all outputs high) which makes all SW pins appear inactive.
> +	 * Set SMA1 and SMA2 as active inputs, disable U.FL1 output and
> +	 * U.FL2 input.
> +	 */
> +	ret = ice_read_sma_ctrl(&pf->hw, &data);
> +	if (ret)
> +		return ret;
> +	data &= ~ICE_ALL_SMA_MASK;
> +	data |= ICE_SMA1_TX_EN | ICE_SMA2_TX_EN | ICE_SMA2_UFL2_RX_DIS;
> +	ret = ice_write_sma_ctrl(&pf->hw, data);
> +	if (ret)
> +		return ret;
> +
>   	ret = ice_dpll_pin_state_update(pf, pin, ICE_DPLL_PIN_TYPE_SOFTWARE,
>   					NULL);
>   	if (ret)

Good catch... thanks for the quick fix.

Reported-by: Liang Li <liali@redhat.com>
Reviewed-by: Ivan Vecera <ivecera@redhat.com>