Re: [PATCH net-next v3 02/11] net: phy: introduce phy_has_c45_registers()

From: Michael Walle
Date: Tue Sep 05 2023 - 13:08:50 EST


Hi,

Hence why I disagree with your suggestion.

So how can we make progress here? I tried Russells suggestion using
phy_supports_c45_transfers() and phy_has_c22_registers(). You can find
a possible v4 of this series on my github [1].

Some uses of .is_c45 seem to be to test whether the c45_ids are valid. E.g.
from the phy.h:
* @c45_ids: 802.3-c45 Device Identifiers if is_c45.

But if you test if a bit is set, you can skip that. For the opposite test
you'd need to have some kind of indication whether c45_ids was populated
in the first place.

If you convert all these uses of is_c45, there will only be a handful
of uses left. And replacing these by phy_supports_c45_transfers() sounds
sane to me except for for these two:
(1) drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
(2) drivers/net/phy/mxl-gpy.c

(2) will eventually replaced by my phy_promote_to_c45(). I might reorder
the patches, so I'll get rid of the .is_c45 use there before the
conversion to phy_supports_c45_transfers().

But honestly, for (1) I don't have any idea whats going on. If I look
at the very first commit, it seems that the is_c45 property is used
to distinguish loopback handling between a gigabit and a 10g PHY (?).

Btw, Russell, I've noticed that phy_restart_aneg() and phy_config_aneg()
will test for c22 registers, but phy_aneg_done() tests just for .is_c45.
Is that correct?

-michael

[1] https://github.com/mwalle/linux/tree/feature-c45-over-c22-v4
[2] https://git.kernel.org/torvalds/c/b5996f11ea5496d0445078f47d22c987888ed467