Re: [PATCH v2 3/3] common/sfc_efx/base: fix reading advertised autoneg ability

[Ivan Malov <ivan.malov@arknetworks.am>]

Dear Stephen,

If I may, I should like to address the following:

- Backport conflict and 'Depends-on:':
   The stable team operates independently of the upstream review process; should the patch not apply cleanly, the stable maintainers will contact me directly (a well-established practice) and I am entirely willing to provide all the assistance at the appropriate juncture. In other words, the 'Cc: stable@dpdk.org' signals that a backport is desirable, not that it must apply without manual resolution. Upstream acceptance is not contingent on the mechanics of a stable backport, which falls outside the scope of this review.

   As to 'Depends-on:': the dependency here is purely contextual. It is one of hunk context, not of function; the fix is semantically correct regardless of whether the block precedes or follows the conversion call.

- Local variable ('port'): I am entirely mindful of the desire to keep code laconic, yet, high-quality future-proof code benefits from such local declarations as they make the actual usage sites easier on the eyes and can be re-used by later additions to the logic.

On these premises, I respectfully suggest that the series be put forward for reconsideration and integration.

Thank you.

On Wed, 12 Aug 2026, Stephen Hemminger wrote:

> On Wed, 12 Aug 2026 21:10:17 +0400
> Ivan Malov <ivan.malov@arknetworks.am> wrote:
>
>> The issue is that when the user disables auto-negotiation by removing
>> the capability bit from the 'advertised mask' (set method) and then
>> reads the resulting capabilities, which involves querying MCDI, the
>> bit reappears in the mask irrespective of the user's intent.
>>
>> Fix this by remembering the user's intent before any link-state queries.
>>
>> Fixes: 2a5cf77e6de8 ("common/sfc_efx/base: provide PHY link get method on Medford4")
>> Fixes: 06f569de6c06 ("common/sfc_efx/base: decode netport link state on probe path")
>> Cc: stable@dpdk.org
>>
>> Suggested-by: Andy Moreton <andy.moreton@amd.com>
>> Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
>> Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
>> Reviewed-by: Andy Moreton <andy.moreton@amd.com>
>
> This patch depends on the previous common series and therefore can
> not be easily backported to stable.
>
> There is a way to mark patch dependencies using Depends-on:
> please use that instead of instructions to AI.
>
> AI says that in its usual over the top wordy way...
>
> Patch 3/3 - common/sfc_efx/base: fix reading advertised autoneg ability
>
>  Warning: this patch depends on the pending common/sfc series for its
>  diff context only, and that dependency creates a backport conflict.
>
>  The efx_np.c hunk removes
>
>      if (lsp->enls_an_supported != B_FALSE)
>              lsp->enls_adv_cap_mask |= 1U << EFX_PHY_CAP_AN;
>
>  from a position after the LINK_STATE_OUT_ADVERTISED_ABILITIES
>  conversion.  On main and on every stable branch the block is still
>  before that conversion, where 06f569de6c06 originally put it.  Since
>  efx_np_cap_mask_hw_to_sw() ORs into *sw_cap_maskp (efx_np.c:197)
>  rather than assigning, the block's position has no effect on the
>  resulting mask - the pending series is only moving text.
>
>  Both Fixes: commits first appear in v25.07, so this needs to reach
>  25.11 LTS and 25.07, neither of which has the move.  Stable will hit
>  a conflict on a hunk whose resolution is not obvious from the diff.
>
>  Suggest sending this fix ahead of the series that moves the block,
>  or as a standalone patch against main.  It then applies unchanged
>  everywhere, and the other series absorbs a zero-cost rebase.  If the
>  current ordering has to stand, please post an explicit backport to
>  stable@dpdk.org rather than leaving the resolution to the stable
>  maintainer.
>
>  Info: the added local
>
>      const efx_port_t *port = &enp->en_port;
>
>  is used once, and the file otherwise reaches through enp->en_port
>  directly (line 33) or names the local "epp" (medford4_phy_reconfigure,
>  medford4_mac_poll).  Suggest:
>
>      preserve_an = enp->en_port.ep_adv_cap_mask &
>                    (1U << EFX_PHY_CAP_AN);
>