linux-kernel - Re: [PATCH net-next v3 02/11] net: phy: introduce phy_has_c45

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e7ecdbc54d55316735cd1d39dd31cc77@kernel.org>
Date:   Tue, 05 Sep 2023 10:22:40 +0200
From:   Michael Walle <mwalle@...nel.org>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Yisen Zhuang <yisen.zhuang@...wei.com>,
        Salil Mehta <salil.mehta@...wei.com>,
        Florian Fainelli <florian.fainelli@...adcom.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@...adcom.com>,
        Marek Behún <kabel@...nel.org>,
        Xu Liang <lxu@...linear.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Simon Horman <simon.horman@...igine.com>
Subject: Re: [PATCH net-next v3 02/11] net: phy: introduce
 phy_has_c45_registers()

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