* [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups
@ 2026-05-04 14:24 Aleksandr Loktionov
2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 1/5] ice: fix ETH56G Rx offset type description in kernel-doc comment Aleksandr Loktionov
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Aleksandr Loktionov @ 2026-05-04 14:24 UTC (permalink / raw)
To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov; +Cc: netdev
Three correctness fixes and two cleanups for the ice driver.
Patch 1 corrects a kernel-doc comment in ice_ptp_hw.h that described the
ETH56G MAC Rx offset field as unsigned when it is signed (trivial doc fix,
no functional change).
Patch 2 removes the PF_SB_REM_DEV_CTL sideband register write from
ice_ptp_init_phc_e82x(). PHY access is enabled by default on E82X and
the register write was a leftover from an earlier SWITCH_MODE workaround
that is no longer needed.
Patch 3 renames ICE_SMA2_UFL2_RX_DIS to ICE_SMA2_UFL2_RX_EN to match
the actual active-high hardware semantics and inverts the three use sites
in ice_dpll.c so that the logic remains correct.
Patch 4 replaces the static per-type frequency tables for CGU pins with a
single DPLL_PIN_FREQUENCY_RANGE(1, 25 MHz) entry. The firmware defines
an any_freq capability for configurable CGU inputs, but the old tables
restricted users to 1 PPS or 10 MHz. GNSS pins retain a 1 PPS-only
entry since they are physically constrained.
Patch 5 exports ice_dcb_need_recfg() and calls it in the four SW LLDP
netlink setters instead of memcmp() on a non-packed struct, which is
undefined behaviour due to uninitialised padding bytes. The redundant
memcmp in ice_pf_dcb_cfg() is removed since callers now guard it.
Aleksandr Loktionov (2):
ice: add correct handling of SMA/u.FL states
ice: use element-by-element comparison for DCB config changes
Arkadiusz Kubalewski (1):
ice: fix DPLL pin frequency range in CGU pin descriptors
Karol Kolacinski (2):
ice: fix ETH56G Rx offset type description in kernel-doc comment
ice: remove unnecessary PF_SB_REM_DEV_CTL write for E82X
v1 -> v2 updated mail subject with PATCH iwl-next
drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 13 +-
drivers/net/ethernet/intel/ice/ice_dcb_lib.h | 2 +
drivers/net/ethernet/intel/ice/ice_dcb_nl.c | 30 +++-
drivers/net/ethernet/intel/ice/ice_dpll.c | 6 +-
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 141 ++++++++++---------
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 8 +-
6 files changed, 113 insertions(+), 87 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [Intel-wired-lan] [PATCH iwl-next v2 1/5] ice: fix ETH56G Rx offset type description in kernel-doc comment 2026-05-04 14:24 [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Aleksandr Loktionov @ 2026-05-04 14:24 ` Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 2/5] ice: remove unnecessary PF_SB_REM_DEV_CTL write for E82X Aleksandr Loktionov ` (4 subsequent siblings) 5 siblings, 0 replies; 9+ messages in thread From: Aleksandr Loktionov @ 2026-05-04 14:24 UTC (permalink / raw) To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov; +Cc: netdev From: Karol Kolacinski <karol.kolacinski@intel.com> The ETH56G MAC register configuration Rx offset field stores a signed integer, not an unsigned one. Correct the struct comment that incorrectly described it as '11 bit unsigned int'. Also update 'unsigned ints' to 'unsigned integers' for consistency. Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> --- drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h index 9bfd3e7..c1aa408 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h @@ -144,9 +144,9 @@ struct ice_vernier_info_e82x { * @tx_offset: total Tx offset, fixed point * @rx_offset: total Rx offset, contains value for bitslip/deskew, fixed point * - * All fixed point registers except Rx offset are 23 bit unsigned ints with + * All fixed point registers except Rx offset are 23 bit unsigned integers with * a 9 bit fractional. - * Rx offset is 11 bit unsigned int with a 9 bit fractional. + * Rx offset is 11 bit signed integer with a 9 bit fractional. */ struct ice_eth56g_mac_reg_cfg { struct { -- 2.52.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH iwl-next v2 2/5] ice: remove unnecessary PF_SB_REM_DEV_CTL write for E82X 2026-05-04 14:24 [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 1/5] ice: fix ETH56G Rx offset type description in kernel-doc comment Aleksandr Loktionov @ 2026-05-04 14:24 ` Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: add correct handling of SMA/u.FL states Aleksandr Loktionov ` (3 subsequent siblings) 5 siblings, 0 replies; 9+ messages in thread From: Aleksandr Loktionov @ 2026-05-04 14:24 UTC (permalink / raw) To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov; +Cc: netdev From: Karol Kolacinski <karol.kolacinski@intel.com> Remove the PF_SB_REM_DEV_CTL register write from ice_ptp_init_phc_e82x(). PHY access is enabled by default on E82X devices and the driver does not need to configure switch device access. The register write was a remnant of an earlier SWITCH_MODE workaround for a FIFO issue and is no longer needed. Also update the kernel-doc comment to refer to the E82X family rather than E822. Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> --- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index 61c0a0d..7b1b402 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -2767,22 +2767,13 @@ static int ice_ptp_set_vernier_wl(struct ice_hw *hw) } /** - * ice_ptp_init_phc_e82x - Perform E822 specific PHC initialization + * ice_ptp_init_phc_e82x - Perform E82X specific PHC initialization * @hw: pointer to HW struct * - * Perform PHC initialization steps specific to E822 devices. + * Perform PHC initialization steps specific to E82X devices. */ static int ice_ptp_init_phc_e82x(struct ice_hw *hw) { - u32 val; - - /* Enable reading switch and PHY registers over the sideband queue */ -#define PF_SB_REM_DEV_CTL_SWITCH_READ BIT(1) -#define PF_SB_REM_DEV_CTL_PHY0 BIT(2) - val = rd32(hw, PF_SB_REM_DEV_CTL); - val |= (PF_SB_REM_DEV_CTL_SWITCH_READ | PF_SB_REM_DEV_CTL_PHY0); - wr32(hw, PF_SB_REM_DEV_CTL, val); - /* Set window length for all the ports */ return ice_ptp_set_vernier_wl(hw); } -- 2.52.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: add correct handling of SMA/u.FL states 2026-05-04 14:24 [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 1/5] ice: fix ETH56G Rx offset type description in kernel-doc comment Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 2/5] ice: remove unnecessary PF_SB_REM_DEV_CTL write for E82X Aleksandr Loktionov @ 2026-05-04 14:24 ` Aleksandr Loktionov 2026-05-07 11:45 ` Simon Horman 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 4/5] ice: fix DPLL pin frequency range in CGU pin descriptors Aleksandr Loktionov ` (2 subsequent siblings) 5 siblings, 1 reply; 9+ messages in thread From: Aleksandr Loktionov @ 2026-05-04 14:24 UTC (permalink / raw) To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov; +Cc: netdev The ICE_SMA2_UFL2_RX_DIS bit name is wrong: the bit is active high (setting it *enables* RX for u.FL2 / SMA2), not active low. Rename it to ICE_SMA2_UFL2_RX_EN and invert the use sites in ice_dpll.c so that enabling the u.FL2 pin clears the bit (as it used to do) and disabling sets it. Fixes: 2dd5d03c77e2 ("ice: redesign dpll sma/u.fl pins control") Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> --- drivers/net/ethernet/intel/ice/ice_dpll.c | 6 +++--- drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index 62f75701d..7e8bb63 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -672,7 +672,7 @@ ice_dpll_sw_pins_update(struct ice_pf *pf) p->active = false; p = &d->ufl[ICE_DPLL_PIN_SW_2_IDX]; - p->active = (data & ICE_SMA2_DIR_EN) && !(data & ICE_SMA2_UFL2_RX_DIS); + p->active = (data & ICE_SMA2_DIR_EN) && !(data & ICE_SMA2_UFL2_RX_EN); d->sma_data = data; return 0; @@ -1264,10 +1264,10 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, case ICE_DPLL_PIN_SW_2_IDX: if (state == DPLL_PIN_STATE_SELECTABLE) { data |= ICE_SMA2_DIR_EN; - data &= ~ICE_SMA2_UFL2_RX_DIS; + data &= ~ICE_SMA2_UFL2_RX_EN; enable = true; } else if (state == DPLL_PIN_STATE_DISCONNECTED) { - data |= ICE_SMA2_UFL2_RX_DIS; + data |= ICE_SMA2_UFL2_RX_EN; enable = false; } else { goto unlock; diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h index c1aa408..278d757 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h @@ -655,12 +655,12 @@ static inline u64 ice_get_base_incval(struct ice_hw *hw) /* SMA controller pin control */ #define ICE_SMA1_DIR_EN BIT(4) #define ICE_SMA1_TX_EN BIT(5) -#define ICE_SMA2_UFL2_RX_DIS BIT(3) +#define ICE_SMA2_UFL2_RX_EN BIT(3) #define ICE_SMA2_DIR_EN BIT(6) #define ICE_SMA2_TX_EN BIT(7) #define ICE_SMA1_MASK (ICE_SMA1_DIR_EN | ICE_SMA1_TX_EN) -#define ICE_SMA2_MASK (ICE_SMA2_UFL2_RX_DIS | ICE_SMA2_DIR_EN | \ +#define ICE_SMA2_MASK (ICE_SMA2_UFL2_RX_EN | ICE_SMA2_DIR_EN | \ ICE_SMA2_TX_EN) #define ICE_SMA2_INACTIVE_MASK (ICE_SMA2_DIR_EN | ICE_SMA2_TX_EN) #define ICE_ALL_SMA_MASK (ICE_SMA1_MASK | ICE_SMA2_MASK) -- 2.52.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: add correct handling of SMA/u.FL states 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: add correct handling of SMA/u.FL states Aleksandr Loktionov @ 2026-05-07 11:45 ` Simon Horman 0 siblings, 0 replies; 9+ messages in thread From: Simon Horman @ 2026-05-07 11:45 UTC (permalink / raw) To: aleksandr.loktionov Cc: 'Simon Horman', intel-wired-lan, anthony.l.nguyen, netdev From: 'Simon Horman' <horms@kernel.org> This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev Simon says: I would also like to add that as a fix the patch description should describe the user-visible effects of the bug, and how the fix addresses them. Thanks! --- ice: add correct handling of SMA/u.FL states This commit renames ICE_SMA2_UFL2_RX_DIS to ICE_SMA2_UFL2_RX_EN, stating that the hardware bit is actually active high for enabling RX. It updates the uses of the macro in ice_dpll.c to reflect the new name. > The ICE_SMA2_UFL2_RX_DIS bit name is wrong: the bit is active high > (setting it *enables* RX for u.FL2 / SMA2), not active low. Rename > it to ICE_SMA2_UFL2_RX_EN and invert the use sites in ice_dpll.c so > that enabling the u.FL2 pin clears the bit (as it used to do) and > disabling sets it. The commit message claims that setting the bit enables RX (active high), but then states that enabling the pin clears the bit (active low). Could this description be clarified to explain exactly whether the hardware uses active high or active low logic? > diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c > index 62f75701d6520..7e8bb63b262a0 100644 > --- a/drivers/net/ethernet/intel/ice/ice_dpll.c > +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c > @@ -672,7 +672,7 @@ ice_dpll_sw_pins_update(struct ice_pf *pf) > p->active = false; > > p = &d->ufl[ICE_DPLL_PIN_SW_2_IDX]; > - p->active = (data & ICE_SMA2_DIR_EN) && !(data & ICE_SMA2_UFL2_RX_DIS); > + p->active = (data & ICE_SMA2_DIR_EN) && !(data & ICE_SMA2_UFL2_RX_EN); If the bit is renamed to an _EN suffix and is active high, should the logic be inverted here? This code still treats the bit as active low by checking for it being cleared (!(data & ICE_SMA2_UFL2_RX_EN)). [ ... ] > @@ -1264,10 +1264,10 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv, > case ICE_DPLL_PIN_SW_2_IDX: > if (state == DPLL_PIN_STATE_SELECTABLE) { > data |= ICE_SMA2_DIR_EN; > - data &= ~ICE_SMA2_UFL2_RX_DIS; > + data &= ~ICE_SMA2_UFL2_RX_EN; > enable = true; > } else if (state == DPLL_PIN_STATE_DISCONNECTED) { > - data |= ICE_SMA2_UFL2_RX_DIS; > + data |= ICE_SMA2_UFL2_RX_EN; > enable = false; > } else { Similarly, does this code still apply active low logic by clearing the _EN bit when enable is true, and setting it when enable is false? If the bit is truly active high as the commit message suggests ("setting it enables RX"), shouldn't the bitwise operations be inverted to match? ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH iwl-next v2 4/5] ice: fix DPLL pin frequency range in CGU pin descriptors 2026-05-04 14:24 [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Aleksandr Loktionov ` (2 preceding siblings ...) 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: add correct handling of SMA/u.FL states Aleksandr Loktionov @ 2026-05-04 14:24 ` Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 5/5] ice: use element-by-element comparison for DCB config changes Aleksandr Loktionov 2026-05-06 22:53 ` [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Jacob Keller 5 siblings, 0 replies; 9+ messages in thread From: Aleksandr Loktionov @ 2026-05-04 14:24 UTC (permalink / raw) To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov Cc: netdev, Arkadiusz Kubalewski From: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> Replace the per-type frequency tables (ice_cgu_pin_freq_1_hz and ice_cgu_pin_freq_10_mhz) and the two-entry ice_cgu_pin_freq_common array with a named range constant ICE_CGU_MAX_FREQ_HZ (25 MHz), a new ice_cgu_pin_freq_range array containing DPLL_PIN_FREQUENCY_RANGE(1, ICE_CGU_MAX_FREQ_HZ), and a separate ice_cgu_pin_freq_gnss array that retains DPLL_PIN_FREQUENCY_1PPS for GNSS input pins. The hardware firmware spec defines an any_freq capability for CGU inputs (ICE_AQC_GET_CGU_IN_CFG_FLG1_ANYFREQ), but the static pin descriptor tables constrained configurable pins to 1PPS or 10MHz, preventing users from setting valid intermediate frequencies. Use a range entry so the DPLL netlink interface correctly reflects what the firmware will accept. The firmware validates the actual value and rejects out-of-range requests. MUX, SyncE ETH port, and configurable EXT pins now advertise the full frequency range, matching the hardware capability. GNSS input pins retain the 1PPS-only advertisement since a GNSS receiver is physically constrained to 1 Hz. Fixes: 6db5f2cd9ebb ("ice: dpll: fix output pin capabilities") Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> --- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 128 +++++++++++--------- 1 file changed, 72 insertions(+), 56 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index 7b1b402..3949138 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -7,127 +7,143 @@ #include "ice_ptp_hw.h" #include "ice_ptp_consts.h" -static struct dpll_pin_frequency ice_cgu_pin_freq_common[] = { - DPLL_PIN_FREQUENCY_1PPS, - DPLL_PIN_FREQUENCY_10MHZ, -}; +/* Maximum frequency supported by CGU pins, in Hz */ +#define ICE_CGU_MAX_FREQ_HZ 25000000 -static struct dpll_pin_frequency ice_cgu_pin_freq_1_hz[] = { - DPLL_PIN_FREQUENCY_1PPS, +static struct dpll_pin_frequency ice_cgu_pin_freq_range[] = { + DPLL_PIN_FREQUENCY_RANGE(1, ICE_CGU_MAX_FREQ_HZ), }; -static struct dpll_pin_frequency ice_cgu_pin_freq_10_mhz[] = { - DPLL_PIN_FREQUENCY_10MHZ, +static struct dpll_pin_frequency ice_cgu_pin_freq_gnss[] = { + DPLL_PIN_FREQUENCY_1PPS, }; static const struct ice_cgu_pin_desc ice_e810t_sfp_cgu_inputs[] = { { "CVL-SDP22", ZL_REF0P, DPLL_PIN_TYPE_INT_OSCILLATOR, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "CVL-SDP20", ZL_REF0N, DPLL_PIN_TYPE_INT_OSCILLATOR, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, - { "C827_0-RCLKA", ZL_REF1P, DPLL_PIN_TYPE_MUX, 0, }, - { "C827_0-RCLKB", ZL_REF1N, DPLL_PIN_TYPE_MUX, 0, }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "C827_0-RCLKA", ZL_REF1P, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "C827_0-RCLKB", ZL_REF1N, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "SMA1", ZL_REF3P, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "SMA2/U.FL2", ZL_REF3N, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "GNSS-1PPS", ZL_REF4P, DPLL_PIN_TYPE_GNSS, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, + ARRAY_SIZE(ice_cgu_pin_freq_gnss), ice_cgu_pin_freq_gnss }, }; static const struct ice_cgu_pin_desc ice_e810t_qsfp_cgu_inputs[] = { { "CVL-SDP22", ZL_REF0P, DPLL_PIN_TYPE_INT_OSCILLATOR, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "CVL-SDP20", ZL_REF0N, DPLL_PIN_TYPE_INT_OSCILLATOR, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, - { "C827_0-RCLKA", ZL_REF1P, DPLL_PIN_TYPE_MUX, }, - { "C827_0-RCLKB", ZL_REF1N, DPLL_PIN_TYPE_MUX, }, - { "C827_1-RCLKA", ZL_REF2P, DPLL_PIN_TYPE_MUX, }, - { "C827_1-RCLKB", ZL_REF2N, DPLL_PIN_TYPE_MUX, }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "C827_0-RCLKA", ZL_REF1P, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "C827_0-RCLKB", ZL_REF1N, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "C827_1-RCLKA", ZL_REF2P, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "C827_1-RCLKB", ZL_REF2N, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "SMA1", ZL_REF3P, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "SMA2/U.FL2", ZL_REF3N, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "GNSS-1PPS", ZL_REF4P, DPLL_PIN_TYPE_GNSS, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, + ARRAY_SIZE(ice_cgu_pin_freq_gnss), ice_cgu_pin_freq_gnss }, }; static const struct ice_cgu_pin_desc ice_e810t_sfp_cgu_outputs[] = { { "REF-SMA1", ZL_OUT0, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "REF-SMA2/U.FL2", ZL_OUT1, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, - { "PHY-CLK", ZL_OUT2, DPLL_PIN_TYPE_SYNCE_ETH_PORT, }, - { "MAC-CLK", ZL_OUT3, DPLL_PIN_TYPE_SYNCE_ETH_PORT, }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "PHY-CLK", ZL_OUT2, DPLL_PIN_TYPE_SYNCE_ETH_PORT, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "MAC-CLK", ZL_OUT3, DPLL_PIN_TYPE_SYNCE_ETH_PORT, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "CVL-SDP21", ZL_OUT4, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "CVL-SDP23", ZL_OUT5, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, }; static const struct ice_cgu_pin_desc ice_e810t_qsfp_cgu_outputs[] = { { "REF-SMA1", ZL_OUT0, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "REF-SMA2/U.FL2", ZL_OUT1, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, - { "PHY-CLK", ZL_OUT2, DPLL_PIN_TYPE_SYNCE_ETH_PORT, 0 }, - { "PHY2-CLK", ZL_OUT3, DPLL_PIN_TYPE_SYNCE_ETH_PORT, 0 }, - { "MAC-CLK", ZL_OUT4, DPLL_PIN_TYPE_SYNCE_ETH_PORT, 0 }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "PHY-CLK", ZL_OUT2, DPLL_PIN_TYPE_SYNCE_ETH_PORT, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "PHY2-CLK", ZL_OUT3, DPLL_PIN_TYPE_SYNCE_ETH_PORT, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "MAC-CLK", ZL_OUT4, DPLL_PIN_TYPE_SYNCE_ETH_PORT, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "CVL-SDP21", ZL_OUT5, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "CVL-SDP23", ZL_OUT6, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, }; static const struct ice_cgu_pin_desc ice_e823_si_cgu_inputs[] = { { "NONE", SI_REF0P, 0, 0 }, { "NONE", SI_REF0N, 0, 0 }, - { "SYNCE0_DP", SI_REF1P, DPLL_PIN_TYPE_MUX, 0 }, - { "SYNCE0_DN", SI_REF1N, DPLL_PIN_TYPE_MUX, 0 }, + { "SYNCE0_DP", SI_REF1P, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "SYNCE0_DN", SI_REF1N, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "EXT_CLK_SYNC", SI_REF2P, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "NONE", SI_REF2N, 0, 0 }, { "EXT_PPS_OUT", SI_REF3, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "INT_PPS_OUT", SI_REF4, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, }; static const struct ice_cgu_pin_desc ice_e823_si_cgu_outputs[] = { { "1588-TIME_SYNC", SI_OUT0, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, - { "PHY-CLK", SI_OUT1, DPLL_PIN_TYPE_SYNCE_ETH_PORT, 0 }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "PHY-CLK", SI_OUT1, DPLL_PIN_TYPE_SYNCE_ETH_PORT, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "10MHZ-SMA2", SI_OUT2, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_10_mhz), ice_cgu_pin_freq_10_mhz }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "PPS-SMA1", SI_OUT3, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, }; static const struct ice_cgu_pin_desc ice_e823_zl_cgu_inputs[] = { { "NONE", ZL_REF0P, 0, 0 }, { "INT_PPS_OUT", ZL_REF0N, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, - { "SYNCE0_DP", ZL_REF1P, DPLL_PIN_TYPE_MUX, 0 }, - { "SYNCE0_DN", ZL_REF1N, DPLL_PIN_TYPE_MUX, 0 }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "SYNCE0_DP", ZL_REF1P, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "SYNCE0_DN", ZL_REF1N, DPLL_PIN_TYPE_MUX, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "NONE", ZL_REF2P, 0, 0 }, { "NONE", ZL_REF2N, 0, 0 }, { "EXT_CLK_SYNC", ZL_REF3P, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "NONE", ZL_REF3N, 0, 0 }, { "EXT_PPS_OUT", ZL_REF4P, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "OCXO", ZL_REF4N, DPLL_PIN_TYPE_INT_OSCILLATOR, 0 }, }; static const struct ice_cgu_pin_desc ice_e823_zl_cgu_outputs[] = { { "PPS-SMA1", ZL_OUT0, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "10MHZ-SMA2", ZL_OUT1, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_10_mhz), ice_cgu_pin_freq_10_mhz }, - { "PHY-CLK", ZL_OUT2, DPLL_PIN_TYPE_SYNCE_ETH_PORT, 0 }, - { "1588-TIME_REF", ZL_OUT3, DPLL_PIN_TYPE_SYNCE_ETH_PORT, 0 }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "PHY-CLK", ZL_OUT2, DPLL_PIN_TYPE_SYNCE_ETH_PORT, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, + { "1588-TIME_REF", ZL_OUT3, DPLL_PIN_TYPE_SYNCE_ETH_PORT, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "CPK-TIME_SYNC", ZL_OUT4, DPLL_PIN_TYPE_EXT, - ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common }, + ARRAY_SIZE(ice_cgu_pin_freq_range), ice_cgu_pin_freq_range }, { "NONE", ZL_OUT5, 0, 0 }, }; -- 2.52.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH iwl-next v2 5/5] ice: use element-by-element comparison for DCB config changes 2026-05-04 14:24 [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Aleksandr Loktionov ` (3 preceding siblings ...) 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 4/5] ice: fix DPLL pin frequency range in CGU pin descriptors Aleksandr Loktionov @ 2026-05-04 14:24 ` Aleksandr Loktionov 2026-05-07 11:46 ` Simon Horman 2026-05-06 22:53 ` [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Jacob Keller 5 siblings, 1 reply; 9+ messages in thread From: Aleksandr Loktionov @ 2026-05-04 14:24 UTC (permalink / raw) To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov; +Cc: netdev Comparing two ice_dcbx_cfg structs with memcmp() is unreliable on non-packed structs due to uninitialised padding bytes. The HW DCB path already has ice_dcb_need_recfg() that compares fields individually; export it and use it in the SW DCB netlink setters (setets, setpfc, setapp, cee_set_all) instead of the unsafe memcmp. Remove the now-redundant memcmp check from ice_pf_dcb_cfg() so that function always attempts the HW reconfiguration when called. Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> --- drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 13 ++------- drivers/net/ethernet/intel/ice/ice_dcb_lib.h | 2 ++ drivers/net/ethernet/intel/ice/ice_dcb_nl.c | 30 ++++++++++++++++++-- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c index bd77f1c..2e85fae 100644 --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c @@ -353,15 +353,11 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked) struct ice_dcbx_cfg *old_cfg, *curr_cfg; struct device *dev = ice_pf_to_dev(pf); struct iidc_rdma_event *event; - int ret = ICE_DCB_NO_HW_CHG; struct ice_vsi *pf_vsi; + int ret; curr_cfg = &pf->hw.port_info->qos_cfg.local_dcbx_cfg; - /* FW does not care if change happened */ - if (!pf->hw.port_info->qos_cfg.is_sw_lldp) - ret = ICE_DCB_HW_CHG_RST; - /* Enable DCB tagging only when more than one TC */ if (ice_dcb_get_num_tc(new_cfg) > 1) { dev_dbg(dev, "DCB tagging enabled (num TC > 1)\n"); @@ -377,11 +373,6 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked) clear_bit(ICE_FLAG_DCB_ENA, pf->flags); } - if (!memcmp(new_cfg, curr_cfg, sizeof(*new_cfg))) { - dev_dbg(dev, "No change in DCB config required\n"); - return ret; - } - if (ice_dcb_bwchk(pf, new_cfg)) return -EINVAL; @@ -481,7 +472,7 @@ static void ice_cfg_etsrec_defaults(struct ice_port_info *pi) * @old_cfg: current DCB config * @new_cfg: new DCB config */ -static bool +bool ice_dcb_need_recfg(struct ice_pf *pf, struct ice_dcbx_cfg *old_cfg, struct ice_dcbx_cfg *new_cfg) { diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.h b/drivers/net/ethernet/intel/ice/ice_dcb_lib.h index da9ba81..a7eaa2f9 100644 --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.h +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.h @@ -20,6 +20,8 @@ u8 ice_dcb_get_num_tc(struct ice_dcbx_cfg *dcbcfg); void ice_vsi_set_dcb_tc_cfg(struct ice_vsi *vsi); bool ice_is_pfc_causing_hung_q(struct ice_pf *pf, unsigned int txqueue); u8 ice_dcb_get_tc(struct ice_vsi *vsi, int queue_index); +bool ice_dcb_need_recfg(struct ice_pf *pf, struct ice_dcbx_cfg *old_cfg, + struct ice_dcbx_cfg *new_cfg); int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked); int ice_dcb_bwchk(struct ice_pf *pf, struct ice_dcbx_cfg *dcbcfg); diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c index a10c1c8d..13a52c1 100644 --- a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c +++ b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c @@ -108,11 +108,17 @@ static int ice_dcbnl_setets(struct net_device *netdev, struct ieee_ets *ets) if (!bwrec) new_cfg->etsrec.tcbwtable[0] = 100; + if (!ice_dcb_need_recfg(pf, &pf->hw.port_info->qos_cfg.local_dcbx_cfg, + new_cfg)) { + err = ICE_DCB_NO_HW_CHG; + goto ets_out; + } + err = ice_pf_dcb_cfg(pf, new_cfg, true); /* return of zero indicates new cfg applied */ - if (err == ICE_DCB_HW_CHG_RST) + if (!err) ice_dcbnl_devreset(netdev); - if (err == ICE_DCB_NO_HW_CHG) + else if (err == ICE_DCB_NO_HW_CHG) err = ICE_DCB_HW_CHG_RST; ets_out: @@ -287,11 +293,18 @@ static int ice_dcbnl_setpfc(struct net_device *netdev, struct ieee_pfc *pfc) new_cfg->pfc.pfcena = pfc->pfc_en; + if (!ice_dcb_need_recfg(pf, &pf->hw.port_info->qos_cfg.local_dcbx_cfg, + new_cfg)) { + err = ICE_DCB_NO_HW_CHG; + goto pfc_out; + } + err = ice_pf_dcb_cfg(pf, new_cfg, true); if (err == ICE_DCB_HW_CHG_RST) ice_dcbnl_devreset(netdev); if (err == ICE_DCB_NO_HW_CHG) err = ICE_DCB_HW_CHG_RST; +pfc_out: mutex_unlock(&pf->tc_mutex); return err; } @@ -845,6 +858,12 @@ static int ice_dcbnl_setapp(struct net_device *netdev, struct dcb_app *app) new_cfg->dscp_map[app->protocol] = app->priority; new_cfg->app[new_cfg->numapps++] = new_app; + if (!ice_dcb_need_recfg(pf, &pf->hw.port_info->qos_cfg.local_dcbx_cfg, + new_cfg)) { + ret = ICE_DCB_NO_HW_CHG; + goto setapp_out; + } + ret = ice_pf_dcb_cfg(pf, new_cfg, true); /* return of zero indicates new cfg applied */ if (ret == ICE_DCB_HW_CHG_RST) @@ -991,8 +1010,15 @@ static u8 ice_dcbnl_cee_set_all(struct net_device *netdev) mutex_lock(&pf->tc_mutex); + if (!ice_dcb_need_recfg(pf, &pf->hw.port_info->qos_cfg.local_dcbx_cfg, + new_cfg)) { + err = ICE_DCB_NO_HW_CHG; + goto out; + } + err = ice_pf_dcb_cfg(pf, new_cfg, true); +out: mutex_unlock(&pf->tc_mutex); return (err != ICE_DCB_HW_CHG_RST) ? ICE_DCB_NO_HW_CHG : err; } -- 2.52.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-next v2 5/5] ice: use element-by-element comparison for DCB config changes 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 5/5] ice: use element-by-element comparison for DCB config changes Aleksandr Loktionov @ 2026-05-07 11:46 ` Simon Horman 0 siblings, 0 replies; 9+ messages in thread From: Simon Horman @ 2026-05-07 11:46 UTC (permalink / raw) To: aleksandr.loktionov Cc: 'Simon Horman', intel-wired-lan, anthony.l.nguyen, netdev From: 'Simon Horman' <horms@kernel.org> This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev --- Subject: ice: use element-by-element comparison for DCB config changes This commit replaces the use of memcmp() for DCB configurations with an element-by-element comparison using ice_dcb_need_recfg(). It exports this function and applies it to the SW DCB netlink setters to prevent issues with uninitialized padding bytes. > diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c > index a10c1c8d86977..13a52c1a4da3e 100644 > --- a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c > +++ b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c > @@ -108,11 +108,17 @@ static int ice_dcbnl_setets(struct net_device *netdev, struct ieee_ets *ets) > if (!bwrec) > new_cfg->etsrec.tcbwtable[0] = 100; > > + if (!ice_dcb_need_recfg(pf, &pf->hw.port_info->qos_cfg.local_dcbx_cfg, > + new_cfg)) { > + err = ICE_DCB_NO_HW_CHG; > + goto ets_out; > + } Does ice_dcb_need_recfg() correctly detect bandwidth changes? Looking at the implementation of ice_dcb_need_recfg(), it only logs a debug message but doesn't set need_reconfig to true when only the ETS Bandwidth Table or TSA Table changes: drivers/net/ethernet/intel/ice/ice_dcb_lib.c:ice_dcb_need_recfg() { ... if (memcmp(&new_cfg->etscfg.tcbwtable, &old_cfg->etscfg.tcbwtable, sizeof(new_cfg->etscfg.tcbwtable))) dev_dbg(dev, "ETS TC BW Table changed.\n"); ... } Since the function returns false, will this cause ice_dcbnl_setets() to abort early and silently drop user-requested bandwidth modifications? Also, since new_cfg points to the shared desired_dcbx_cfg state, does this early return leave uncommitted modifications in the software state? If a subsequent unrelated operation (like setting PFC) correctly returns true from ice_dcb_need_recfg(), it appears it could unintentionally push these abandoned ETS bandwidth changes to the hardware. > err = ice_pf_dcb_cfg(pf, new_cfg, true); > /* return of zero indicates new cfg applied */ > - if (err == ICE_DCB_HW_CHG_RST) > + if (!err) > ice_dcbnl_devreset(netdev); This isn't a bug introduced by this patch, but can ice_dcbnl_devreset() cause a deadlock against hardware reset tasks? The ice_dcbnl_devreset() function spins in a usleep_range() loop while ice_is_reset_in_progress() is true. Since DCBNL callbacks like ice_dcbnl_setets() run with the global rtnl_lock held, and a concurrent hardware or PCIe reset worker likely requires rtnl_lock to complete the reset, will the reset task block forever while this thread waits for it? > - if (err == ICE_DCB_NO_HW_CHG) > + else if (err == ICE_DCB_NO_HW_CHG) > err = ICE_DCB_HW_CHG_RST; > > ets_out: Does this change cause the function to return a positive error code to userspace when applying an identical configuration? By bypassing the translation from ICE_DCB_NO_HW_CHG (which is 1) to ICE_DCB_HW_CHG_RST (which evaluates to 0) in the early goto added above, err remains 1 at the ets_out label. The DCBNL core evaluates any non-zero return value as an error: net/dcb/dcbnl.c:dcbnl_ieee_set() { ... err = netdev->dcbnl_ops->ieee_setets(netdev, ets); if (err) goto err; ... } Will this cause tools like lldptool to fail when configuring an existing setup? [ ... ] > @@ -845,6 +858,12 @@ static int ice_dcbnl_setapp(struct net_device *netdev, struct dcb_app *app) > new_cfg->dscp_map[app->protocol] = app->priority; > new_cfg->app[new_cfg->numapps++] = new_app; This isn't a bug introduced by this patch, but does this array append lack bounds checking against ICE_DCBX_MAX_APPS? The app array has a capacity of 64. The function restricts duplicate DSCP protocols using the dscp_mapped bitmap, but since DSCP_MAX is also 64, a user could map all 64 unique values. If numapps starts greater than 0 due to existing EtherType applications, adding 64 valid DSCP mappings would cause numapps to exceed 64, leading to an out-of-bounds write that overwrites the adjacent dscp_mapped bitmap. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups 2026-05-04 14:24 [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Aleksandr Loktionov ` (4 preceding siblings ...) 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 5/5] ice: use element-by-element comparison for DCB config changes Aleksandr Loktionov @ 2026-05-06 22:53 ` Jacob Keller 5 siblings, 0 replies; 9+ messages in thread From: Jacob Keller @ 2026-05-06 22:53 UTC (permalink / raw) To: Aleksandr Loktionov, intel-wired-lan, anthony.l.nguyen; +Cc: netdev On 5/4/2026 7:24 AM, Aleksandr Loktionov wrote: > Three correctness fixes and two cleanups for the ice driver. > > Patch 1 corrects a kernel-doc comment in ice_ptp_hw.h that described the > ETH56G MAC Rx offset field as unsigned when it is signed (trivial doc fix, > no functional change). > > Patch 2 removes the PF_SB_REM_DEV_CTL sideband register write from > ice_ptp_init_phc_e82x(). PHY access is enabled by default on E82X and > the register write was a leftover from an earlier SWITCH_MODE workaround > that is no longer needed. > > Patch 3 renames ICE_SMA2_UFL2_RX_DIS to ICE_SMA2_UFL2_RX_EN to match > the actual active-high hardware semantics and inverts the three use sites > in ice_dpll.c so that the logic remains correct. > > Patch 4 replaces the static per-type frequency tables for CGU pins with a > single DPLL_PIN_FREQUENCY_RANGE(1, 25 MHz) entry. The firmware defines > an any_freq capability for configurable CGU inputs, but the old tables > restricted users to 1 PPS or 10 MHz. GNSS pins retain a 1 PPS-only > entry since they are physically constrained. > > Patch 5 exports ice_dcb_need_recfg() and calls it in the four SW LLDP > netlink setters instead of memcmp() on a non-packed struct, which is > undefined behaviour due to uninitialised padding bytes. The redundant > memcmp in ice_pf_dcb_cfg() is removed since callers now guard it. > Some of these seem like they belong as net fixes, not cleanups targetting next. Specifically patch 3 and 4 I feel should be separated. Could you please either justify why those issues are not "fixes" worthy of net, or separate them into their own series? Thanks, Jake ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-07 11:47 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-04 14:24 [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 1/5] ice: fix ETH56G Rx offset type description in kernel-doc comment Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 2/5] ice: remove unnecessary PF_SB_REM_DEV_CTL write for E82X Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: add correct handling of SMA/u.FL states Aleksandr Loktionov 2026-05-07 11:45 ` Simon Horman 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 4/5] ice: fix DPLL pin frequency range in CGU pin descriptors Aleksandr Loktionov 2026-05-04 14:24 ` [Intel-wired-lan] [PATCH iwl-next v2 5/5] ice: use element-by-element comparison for DCB config changes Aleksandr Loktionov 2026-05-07 11:46 ` Simon Horman 2026-05-06 22:53 ` [Intel-wired-lan] [PATCH iwl-next v2 0/5] ice: five small fixes and cleanups Jacob Keller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox