[iwl-next,v5,2/3] ice: change SMA pins to SDP in PTP API

From: Karol Kolacinski <karol.kolacinski@intel.com>

This change aligns E810 PTP pin control to all other products.

Currently, SMA/U.FL port expanders are controlled together with SDP pins
connected to 1588 clock. To align this, separate this control by
exposing only SDP20..23 pins in PTP API on adapters with DPLL.

Clear error for all E810 on absent NVM pin section or other errors to
allow proper initialization on SMA E810 with NVM section.

Use ARRAY_SIZE for pin array instead of internal definition.

Reviewed-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
v5:
- no change.
---
 drivers/net/ethernet/intel/ice/ice_ptp.c | 254 ++++-------------------
 drivers/net/ethernet/intel/ice/ice_ptp.h |   3 -
 2 files changed, 39 insertions(+), 218 deletions(-)

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Arkadiusz Kubalewski
> Sent: Tuesday, April 22, 2025 6:02 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Kolacinski, Karol <karol.kolacinski@intel.com>;
> Olech, Milena <milena.olech@intel.com>; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v5 2/3] ice: change SMA pins to SDP
> in PTP API
> 
> From: Karol Kolacinski <karol.kolacinski@intel.com>
> 
> This change aligns E810 PTP pin control to all other products.
> 
> Currently, SMA/U.FL port expanders are controlled together with SDP pins
> connected to 1588 clock. To align this, separate this control by exposing only
> SDP20..23 pins in PTP API on adapters with DPLL.
> 
> Clear error for all E810 on absent NVM pin section or other errors to allow
> proper initialization on SMA E810 with NVM section.
> 
> Use ARRAY_SIZE for pin array instead of internal definition.
> 
> Reviewed-by: Milena Olech <milena.olech@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> ---
> v5:
> - no change.
> ---
>  drivers/net/ethernet/intel/ice/ice_ptp.c | 254 ++++-------------------
>  drivers/net/ethernet/intel/ice/ice_ptp.h |   3 -
>  2 files changed, 39 insertions(+), 218 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index b79a148ed0f2..b948a6d9226c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -40,21 +40,19 @@ static const struct ice_ptp_pin_desc
> ice_pin_desc_e810[] = {
>  	{  ONE_PPS,   { -1,  5 }, { 0, 1 }},
>  };
> 
> -static const char ice_pin_names_nvm[][64] = {
> -	"GNSS",
> -	"SMA1",
> -	"U.FL1",
> -	"SMA2",
> -	"U.FL2",
> +static const char ice_pin_names_dpll[][64] = {
> +	"SDP20",
> +	"SDP21",
> +	"SDP22",
> +	"SDP23",
>  };
> 
> -static const struct ice_ptp_pin_desc ice_pin_desc_e810_sma[] = {
> +static const struct ice_ptp_pin_desc ice_pin_desc_dpll[] = {
>  	/* name,   gpio,       delay */
> -	{  GNSS, {  1, -1 }, { 0, 0 }},
> -	{  SMA1, {  1,  0 }, { 0, 1 }},
> -	{  UFL1, { -1,  0 }, { 0, 1 }},
> -	{  SMA2, {  3,  2 }, { 0, 1 }},
> -	{  UFL2, {  3, -1 }, { 0, 0 }},
> +	{  SDP0, { -1,  0 }, { 0, 1 }},
> +	{  SDP1, {  1, -1 }, { 0, 0 }},
> +	{  SDP2, { -1,  2 }, { 0, 1 }},
> +	{  SDP3, {  3, -1 }, { 0, 0 }},
>  };
> 
>  static struct ice_pf *ice_get_ctrl_pf(struct ice_pf *pf) @@ -92,101 +90,6 @@
> static int ice_ptp_find_pin_idx(struct ice_pf *pf, enum ptp_pin_function func,
>  	return -1;
>  }
> 
> -/**
> - * ice_ptp_update_sma_data - update SMA pins data according to pins setup
> - * @pf: Board private structure
> - * @sma_pins: parsed SMA pins status
> - * @data: SMA data to update
> - */
> -static void ice_ptp_update_sma_data(struct ice_pf *pf, unsigned int
> sma_pins[],
> -				    u8 *data)
> -{
> -	const char *state1, *state2;
> -
> -	/* Set the right state based on the desired configuration.
> -	 * When bit is set, functionality is disabled.
> -	 */
> -	*data &= ~ICE_ALL_SMA_MASK;
> -	if (!sma_pins[UFL1 - 1]) {
> -		if (sma_pins[SMA1 - 1] == PTP_PF_EXTTS) {
> -			state1 = "SMA1 Rx, U.FL1 disabled";
> -			*data |= ICE_SMA1_TX_EN;
> -		} else if (sma_pins[SMA1 - 1] == PTP_PF_PEROUT) {
> -			state1 = "SMA1 Tx U.FL1 disabled";
> -			*data |= ICE_SMA1_DIR_EN;
> -		} else {
> -			state1 = "SMA1 disabled, U.FL1 disabled";
> -			*data |= ICE_SMA1_MASK;
> -		}
> -	} else {
> -		/* U.FL1 Tx will always enable SMA1 Rx */
> -		state1 = "SMA1 Rx, U.FL1 Tx";
> -	}
> -
> -	if (!sma_pins[UFL2 - 1]) {
> -		if (sma_pins[SMA2 - 1] == PTP_PF_EXTTS) {
> -			state2 = "SMA2 Rx, U.FL2 disabled";
> -			*data |= ICE_SMA2_TX_EN |
> ICE_SMA2_UFL2_RX_DIS;
> -		} else if (sma_pins[SMA2 - 1] == PTP_PF_PEROUT) {
> -			state2 = "SMA2 Tx, U.FL2 disabled";
> -			*data |= ICE_SMA2_DIR_EN |
> ICE_SMA2_UFL2_RX_DIS;
> -		} else {
> -			state2 = "SMA2 disabled, U.FL2 disabled";
> -			*data |= ICE_SMA2_MASK;
> -		}
> -	} else {
> -		if (!sma_pins[SMA2 - 1]) {
> -			state2 = "SMA2 disabled, U.FL2 Rx";
> -			*data |= ICE_SMA2_DIR_EN | ICE_SMA2_TX_EN;
> -		} else {
> -			state2 = "SMA2 Tx, U.FL2 Rx";
> -			*data |= ICE_SMA2_DIR_EN;
> -		}
> -	}
> -
> -	dev_dbg(ice_pf_to_dev(pf), "%s, %s\n", state1, state2);
> -}
> -
> -/**
> - * ice_ptp_set_sma_cfg - set the configuration of the SMA control logic
> - * @pf: Board private structure
> - *
> - * Return: 0 on success, negative error code otherwise
> - */
> -static int ice_ptp_set_sma_cfg(struct ice_pf *pf) -{
> -	const struct ice_ptp_pin_desc *ice_pins = pf->ptp.ice_pin_desc;
> -	struct ptp_pin_desc *pins = pf->ptp.pin_desc;
> -	unsigned int sma_pins[ICE_SMA_PINS_NUM] = {};
> -	int err;
> -	u8 data;
> -
> -	/* Read initial pin state value */
> -	err = ice_read_sma_ctrl(&pf->hw, &data);
> -	if (err)
> -		return err;
> -
> -	/* Get SMA/U.FL pins states */
> -	for (int i = 0; i < pf->ptp.info.n_pins; i++)
> -		if (pins[i].func) {
> -			int name_idx = ice_pins[i].name_idx;
> -
> -			switch (name_idx) {
> -			case SMA1:
> -			case UFL1:
> -			case SMA2:
> -			case UFL2:
> -				sma_pins[name_idx - 1] = pins[i].func;
> -				break;
> -			default:
> -				continue;
> -			}
> -		}
> -
> -	ice_ptp_update_sma_data(pf, sma_pins, &data);
> -	return ice_write_sma_ctrl(&pf->hw, data);
> -}
> -
>  /**
>   * ice_ptp_cfg_tx_interrupt - Configure Tx timestamp interrupt for the device
>   * @pf: Board private structure
> @@ -1878,63 +1781,6 @@ static void ice_ptp_enable_all_perout(struct
> ice_pf *pf)
>  					   true);
>  }
> 
> -/**
> - * ice_ptp_disable_shared_pin - Disable enabled pin that shares GPIO
> - * @pf: Board private structure
> - * @pin: Pin index
> - * @func: Assigned function
> - *
> - * Return: 0 on success, negative error code otherwise
> - */
> -static int ice_ptp_disable_shared_pin(struct ice_pf *pf, unsigned int pin,
> -				      enum ptp_pin_function func)
> -{
> -	unsigned int gpio_pin;
> -
> -	switch (func) {
> -	case PTP_PF_PEROUT:
> -		gpio_pin = pf->ptp.ice_pin_desc[pin].gpio[1];
> -		break;
> -	case PTP_PF_EXTTS:
> -		gpio_pin = pf->ptp.ice_pin_desc[pin].gpio[0];
> -		break;
> -	default:
> -		return -EOPNOTSUPP;
> -	}
> -
> -	for (unsigned int i = 0; i < pf->ptp.info.n_pins; i++) {
> -		struct ptp_pin_desc *pin_desc = &pf->ptp.pin_desc[i];
> -		unsigned int chan = pin_desc->chan;
> -
> -		/* Skip pin idx from the request */
> -		if (i == pin)
> -			continue;
> -
> -		if (pin_desc->func == PTP_PF_PEROUT &&
> -		    pf->ptp.ice_pin_desc[i].gpio[1] == gpio_pin) {
> -			pf->ptp.perout_rqs[chan].period.sec = 0;
> -			pf->ptp.perout_rqs[chan].period.nsec = 0;
> -			pin_desc->func = PTP_PF_NONE;
> -			pin_desc->chan = 0;
> -			dev_dbg(ice_pf_to_dev(pf), "Disabling pin %u with
> shared output GPIO pin %u\n",
> -				i, gpio_pin);
> -			return ice_ptp_cfg_perout(pf, &pf-
> >ptp.perout_rqs[chan],
> -						  false);
> -		} else if (pf->ptp.pin_desc->func == PTP_PF_EXTTS &&
> -			   pf->ptp.ice_pin_desc[i].gpio[0] == gpio_pin) {
> -			pf->ptp.extts_rqs[chan].flags &=
> ~PTP_ENABLE_FEATURE;
> -			pin_desc->func = PTP_PF_NONE;
> -			pin_desc->chan = 0;
> -			dev_dbg(ice_pf_to_dev(pf), "Disabling pin %u with
> shared input GPIO pin %u\n",
> -				i, gpio_pin);
> -			return ice_ptp_cfg_extts(pf, &pf-
> >ptp.extts_rqs[chan],
> -						 false);
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>  /**
>   * ice_verify_pin - verify if pin supports requested pin function
>   * @info: the driver's PTP info structure @@ -1969,14 +1815,6 @@ static int
> ice_verify_pin(struct ptp_clock_info *info, unsigned int pin,
>  		return -EOPNOTSUPP;
>  	}
> 
> -	/* On adapters with SMA_CTRL disable other pins that share same
> GPIO */
> -	if (ice_is_feature_supported(pf, ICE_F_SMA_CTRL)) {
> -		ice_ptp_disable_shared_pin(pf, pin, func);
> -		pf->ptp.pin_desc[pin].func = func;
> -		pf->ptp.pin_desc[pin].chan = chan;
> -		return ice_ptp_set_sma_cfg(pf);
> -	}
> -
>  	return 0;
>  }
> 
> @@ -2499,14 +2337,14 @@ static void ice_ptp_setup_pin_cfg(struct ice_pf
> *pf)
>  	for (unsigned int i = 0; i < pf->ptp.info.n_pins; i++) {
>  		const struct ice_ptp_pin_desc *desc = &pf-
> >ptp.ice_pin_desc[i];
>  		struct ptp_pin_desc *pin = &pf->ptp.pin_desc[i];
> -		const char *name = NULL;
> +		const char *name;
> 
>  		if (!ice_is_feature_supported(pf, ICE_F_SMA_CTRL))
>  			name = ice_pin_names[desc->name_idx];
> -		else if (desc->name_idx != GPIO_NA)
> -			name = ice_pin_names_nvm[desc->name_idx];
> -		if (name)
> -			strscpy(pin->name, name, sizeof(pin->name));
> +		else
> +			name = ice_pin_names_dpll[desc->name_idx];
> +
> +		strscpy(pin->name, name, sizeof(pin->name));
> 
>  		pin->index = i;
>  	}
> @@ -2518,8 +2356,8 @@ static void ice_ptp_setup_pin_cfg(struct ice_pf *pf)
>   * ice_ptp_disable_pins - Disable PTP pins
>   * @pf: pointer to the PF structure
>   *
> - * Disable the OS access to the SMA pins. Called to clear out the OS
> - * indications of pin support when we fail to setup the SMA control register.
> + * Disable the OS access to the pins. Called to clear out the OS
> + * indications of pin support when we fail to setup pin array.
>   */
>  static void ice_ptp_disable_pins(struct ice_pf *pf)  { @@ -2560,40 +2398,30
> @@ static int ice_ptp_parse_sdp_entries(struct ice_pf *pf, __le16 *entries,
>  	for (i = 0; i < num_entries; i++) {
>  		u16 entry = le16_to_cpu(entries[i]);
>  		DECLARE_BITMAP(bitmap, GPIO_NA);
> -		unsigned int bitmap_idx;
> +		unsigned int idx;
>  		bool dir;
>  		u16 gpio;
> 
>  		*bitmap = FIELD_GET(ICE_AQC_NVM_SDP_AC_PIN_M,
> entry);
> +
> +		/* Check if entry's pin bitmap is valid. */
> +		if (bitmap_empty(bitmap, GPIO_NA))
> +			continue;
> +
>  		dir = !!FIELD_GET(ICE_AQC_NVM_SDP_AC_DIR_M, entry);
>  		gpio = FIELD_GET(ICE_AQC_NVM_SDP_AC_SDP_NUM_M,
> entry);
> -		for_each_set_bit(bitmap_idx, bitmap, GPIO_NA + 1) {
> -			unsigned int idx;
> 
> -			/* Check if entry's pin bit is valid */
> -			if (bitmap_idx >= NUM_PTP_PINS_NVM &&
> -			    bitmap_idx != GPIO_NA)
> -				continue;
> -
> -			/* Check if pin already exists */
> -			for (idx = 0; idx < ICE_N_PINS_MAX; idx++)
> -				if (pins[idx].name_idx == bitmap_idx)
> -					break;
> -
> -			if (idx == ICE_N_PINS_MAX) {
> -				/* Pin not found, setup its entry and name */
> -				idx = n_pins++;
> -				pins[idx].name_idx = bitmap_idx;
> -				if (bitmap_idx == GPIO_NA)
> -					strscpy(pf->ptp.pin_desc[idx].name,
> -						ice_pin_names[gpio],
> -						sizeof(pf->ptp.pin_desc[idx]
> -							       .name));
> -			}
> +		for (idx = 0; idx < ICE_N_PINS_MAX; idx++) {
> +			if (pins[idx].name_idx == gpio)
> +				break;
> +		}
> 
> -			/* Setup in/out GPIO number */
> -			pins[idx].gpio[dir] = gpio;
> +		if (idx == ICE_N_PINS_MAX) {
> +			/* Pin not found, setup its entry and name */
> +			idx = n_pins++;
> +			pins[idx].name_idx = gpio;
>  		}
> +		pins[idx].gpio[dir] = gpio;
>  	}
> 
>  	for (i = 0; i < n_pins; i++) {
> @@ -2621,10 +2449,10 @@ static void ice_ptp_set_funcs_e82x(struct ice_pf
> *pf)
> 
>  	if (pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825) {
>  		pf->ptp.ice_pin_desc = ice_pin_desc_e825c;
> -		pf->ptp.info.n_pins =
> ICE_PIN_DESC_ARR_LEN(ice_pin_desc_e825c);
> +		pf->ptp.info.n_pins = ARRAY_SIZE(ice_pin_desc_e825c);
>  	} else {
>  		pf->ptp.ice_pin_desc = ice_pin_desc_e82x;
> -		pf->ptp.info.n_pins =
> ICE_PIN_DESC_ARR_LEN(ice_pin_desc_e82x);
> +		pf->ptp.info.n_pins = ARRAY_SIZE(ice_pin_desc_e82x);
>  	}
>  	ice_ptp_setup_pin_cfg(pf);
>  }
> @@ -2650,15 +2478,13 @@ static void ice_ptp_set_funcs_e810(struct ice_pf
> *pf)
>  	if (err) {
>  		/* SDP section does not exist in NVM or is corrupted */
>  		if (ice_is_feature_supported(pf, ICE_F_SMA_CTRL)) {
> -			ptp->ice_pin_desc = ice_pin_desc_e810_sma;
> -			ptp->info.n_pins =
> -
> 	ICE_PIN_DESC_ARR_LEN(ice_pin_desc_e810_sma);
> +			ptp->ice_pin_desc = ice_pin_desc_dpll;
> +			ptp->info.n_pins = ARRAY_SIZE(ice_pin_desc_dpll);
>  		} else {
>  			pf->ptp.ice_pin_desc = ice_pin_desc_e810;
> -			pf->ptp.info.n_pins =
> -				ICE_PIN_DESC_ARR_LEN(ice_pin_desc_e810);
> -			err = 0;
> +			pf->ptp.info.n_pins =
> ARRAY_SIZE(ice_pin_desc_e810);
>  		}
> +		err = 0;
>  	} else {
>  		desc = devm_kcalloc(ice_pf_to_dev(pf), ICE_N_PINS_MAX,
>  				    sizeof(struct ice_ptp_pin_desc), @@ -
> 2676,8 +2502,6 @@ static void ice_ptp_set_funcs_e810(struct ice_pf *pf)
>  	ptp->info.pin_config = ptp->pin_desc;
>  	ice_ptp_setup_pin_cfg(pf);
> 
> -	if (ice_is_feature_supported(pf, ICE_F_SMA_CTRL))
> -		err = ice_ptp_set_sma_cfg(pf);
>  err:
>  	if (err) {
>  		devm_kfree(ice_pf_to_dev(pf), desc);
> @@ -2703,7 +2527,7 @@ static void ice_ptp_set_funcs_e830(struct ice_pf
> *pf)  #endif /* CONFIG_ICE_HWTS */
>  	/* Rest of the config is the same as base E810 */
>  	pf->ptp.ice_pin_desc = ice_pin_desc_e810;
> -	pf->ptp.info.n_pins = ICE_PIN_DESC_ARR_LEN(ice_pin_desc_e810);
> +	pf->ptp.info.n_pins = ARRAY_SIZE(ice_pin_desc_e810);
>  	ice_ptp_setup_pin_cfg(pf);
>  }
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h
> b/drivers/net/ethernet/intel/ice/ice_ptp.h
> index 3b769a0cad00..c8dac5a5bcd9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
> @@ -202,9 +202,6 @@ enum ice_ptp_pin_nvm {
> 
>  /* Pin definitions for PTP */
>  #define ICE_N_PINS_MAX			6
> -#define ICE_SMA_PINS_NUM		4
> -#define ICE_PIN_DESC_ARR_LEN(_arr)	(sizeof(_arr) / \
> -					 sizeof(struct ice_ptp_pin_desc))
> 
>  /**
>   * struct ice_ptp_pin_desc - hardware pin description data
> --
> 2.38.1

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Arkadiusz Kubalewski
> Sent: 22 April 2025 21:32
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Kolacinski, Karol <karol.kolacinski@intel.com>; Olech, Milena <milena.olech@intel.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v5 2/3] ice: change SMA pins to SDP in PTP API
>
> From: Karol Kolacinski <karol.kolacinski@intel.com>
>
> This change aligns E810 PTP pin control to all other products.
>
> Currently, SMA/U.FL port expanders are controlled together with SDP pins connected to 1588 clock. To align this, separate this control by exposing only SDP20..23 pins in PTP API on adapters with DPLL.
>
> Clear error for all E810 on absent NVM pin section or other errors to allow proper initialization on SMA E810 with NVM section.
>
> Use ARRAY_SIZE for pin array instead of internal definition.
>
> Reviewed-by: Milena Olech <milena.olech@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> ---
> v5:
> - no change.
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 254 ++++-------------------
> drivers/net/ethernet/intel/ice/ice_ptp.h |   3 -
> 2 files changed, 39 insertions(+), 218 deletions(-)
>

Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)