> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Arkadiusz Kubalewski
> Sent: Tuesday, April 22, 2025 6:02 PM
> To: [email protected]
> Cc: [email protected]; Kolacinski, Karol <[email protected]>;
> Olech, Milena <[email protected]>; Kubalewski, Arkadiusz
> <[email protected]>
> Subject: [Intel-wired-lan] [PATCH iwl-next v5 2/3] ice: change SMA pins to SDP
> in PTP API
>
> From: Karol Kolacinski <[email protected]>
>
> 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 <[email protected]>
Reviewed-by: Aleksandr Loktionov <[email protected]>
> Signed-off-by: Karol Kolacinski <[email protected]>
> Signed-off-by: Arkadiusz Kubalewski <[email protected]>
> ---
> 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
