Hi Tony,
> On 15. Mar 2024, at 22:39, Tony Nguyen <[email protected]> wrote:
>
>
>
> On 3/14/2024 2:15 AM, Ulrich Weber wrote:
>> Current logic uses ICE_AQ_AN_COMPLETED information to
>> flag if autonegotiation is enabled or disabled.
>> Since new ethtool netlink interface checks if there is
>> a configuration change or not and ignores the call, if
>> there is no change, this makes is impossible to disable
>> autonegotiation on links without established autoneg.
>> This will change the logic to check the active phy
>> configuration if autoneg is enabled or not.
>
> Sounds like a bug fix, so you should target this to 'iwl-net' and also add a
> Fixes:
>
>> Signed-off-by: Ulrich Weber <[email protected]
>> <mailto:[email protected]>>
>> ---
>> src/ice_ethtool.c | 10 +++++-----
>
> What tree are you using? This is not a kernel path.
Wasn’t sure about where the patches should apply to,
So I used the latest sf release. Will adopt to net-next then.
>
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>> diff --git a/src/ice_ethtool.c b/src/ice_ethtool.c
>> index e1eeb16babb3..8fe475944f2c 100644
>> --- a/src/ice_ethtool.c
>> +++ b/src/ice_ethtool.c
>> @@ -2857,10 +2857,6 @@ ice_get_link_ksettings(struct net_device *netdev,
>> else
>> ice_get_settings_link_down(ks, netdev);
>> - /* set autoneg settings */
>> - ks->base.autoneg = (hw_link_info->an_info & ICE_AQ_AN_COMPLETED) ?
>> - AUTONEG_ENABLE : AUTONEG_DISABLE;
>> -
>> /* set media type settings */
>> switch (vsi->port_info->phy.media_type) {
>> @@ -2912,6 +2908,10 @@ ice_get_link_ksettings(struct net_device *netdev,
>> if (err)
>> goto done;
>> + /* set autoneg settings based on active configuration */
>> + ks->base.autoneg = ice_is_phy_caps_an_enabled(caps) ?
>> + AUTONEG_ENABLE : AUTONEG_DISABLE;
>
> Since this needs to move to after the PHY capabilities call anyways, it'd be
> nicer to put this with the rest of the autoneg code. You could probably
> utilize the existing ice_is_phy_caps_an_enabled check and not add a second
> call as well.
That’s what I tried first, but it didn’t work for me. That’s also the reason I
changed the comment on the second ice_is_phy_caps_an_enabled() call.
I disabled autoeng by explicitly setting config.low_power_ctrl_an to 0:
ICE_AQC_REPORT_ACTIVE_CFG shows autoneg as disabled then,
while ICE_AQC_REPORT_TOPO_CAP_MEDIA shows it still as enabled.
Chers
Ulrich
>
> Thanks,
> Tony
>
>> +
>> /* Set the advertised flow control based on the PHY capability */
>> if ((caps->caps & ICE_AQC_PHY_EN_TX_LINK_PAUSE) &&
>> (caps->caps & ICE_AQC_PHY_EN_RX_LINK_PAUSE)) {
>> @@ -2960,7 +2960,7 @@ ice_get_link_ksettings(struct net_device *netdev,
>> ethtool_link_ksettings_add_link_mode(ks, supported, FEC_RS);
>> #endif /* ETHTOOL_GFECPARAM */
>> - /* Set supported and advertised autoneg */
>> + /* Set supported and advertised autoneg based on media */
>> if (ice_is_phy_caps_an_enabled(caps)) {
>> ethtool_link_ksettings_add_link_mode(ks, supported, Autoneg);
>> ethtool_link_ksettings_add_link_mode(ks, advertising, Autoneg);
