Re: [PATCH net v2 7/7] igc: fix link speed advertising

On Wed, 2021-01-27 at 08:04 +0100, Heiner Kallweit wrote:

On 26.01.2021 23:10, Tony Nguyen wrote:

quoted

From: Corinna Vinschen <redacted>

Link speed advertising in igc has two problems:

- When setting the advertisement via ethtool, the link speed is
converted
  to the legacy 32 bit representation for the intel PHY code.
  This inadvertently drops ETHTOOL_LINK_MODE_2500baseT_Full_BIT
(being
  beyond bit 31).  As a result, any call to `ethtool -s ...' drops
the
  2500Mbit/s link speed from the PHY settings.  Only reloading the
driver
  alleviates that problem.

  Fix this by converting the ETHTOOL_LINK_MODE_2500baseT_Full_BIT
to the
  Intel PHY ADVERTISE_2500_FULL bit explicitly.

- Rather than checking the actual PHY setting, the
.get_link_ksettings
  function always fills link_modes.advertising with all link speeds
  the device is capable of.

  Fix this by checking the PHY autoneg_advertised settings and
report
  only the actually advertised speeds up to ethtool.

Fixes: 8c5ad0dae93c ("igc: Add ethtool support")
Signed-off-by: Corinna Vinschen <redacted>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_ethtool.c | 24 +++++++++++++++-
----
 1 file changed, 18 insertions(+), 6 deletions(-)

Would switching to phylib be a mid-term option for you?
This could save quite some code and you'd get things like proper
2.5Gbps
handling out of the box. Or is there anything that prevents using
phylib?

Phylib is something we can look into though we have some device
specific quirks that we would need to see how it could be handled.
Since this is fixing a current problem and any transition to phylib
would not be immediate, I think this patch should be accepted while we
investigate phylib.

Thanks,
Tony