netdev - Re: [PATCH RFC net-next 0/5] net: phy: C45-over-C22 access

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YjuDbqZom8knPVpm@lunn.ch>
Date:   Wed, 23 Mar 2022 21:30:38 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Michael Walle <michael@...le.cc>
Cc:     Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        "David S . Miller" <davem@...emloft.net>,
        Xu Liang <lxu@...linear.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net-next 0/5] net: phy: C45-over-C22 access

On Wed, Mar 23, 2022 at 07:34:14PM +0100, Michael Walle wrote:
> Hi,
> 
> This is the result of this discussion:
> https://lore.kernel.org/netdev/240354b0a54b37e8b5764773711b8aa3@walle.cc/
> 
> The goal here is to get the GYP215 and LAN8814 running on the Microchip
> LAN9668 SoC. The LAN9668 suppports one external bus and unfortunately, the
> LAN8814 has a bug which makes it impossible to use C45 on that bus.
> Fortunately, it was the intention of the GPY215 driver to be used on a C22
> bus. But I think this could have never really worked, because the
> phy_get_c45_ids() will always do c45 accesses and thus on MDIO bus drivers
> which will correctly check for the MII_ADDR_C45 flag and return -EOPNOTSUPP
> the function call will fail and thus gpy_probe() will fail. This series
> tries to fix that and will lay the foundation to add a workaround for the
> LAN8814 bug by forcing an MDIO bus to be c22-only.
> 
> At the moment, the probe_capabilities is taken into account to decide if
> we have to use C45-over-C22. What is still missing from this series is the
> handling of a device tree property to restrict the probe_capabilities to
> c22-only.

We have a problem here with phydev->is_c45.

In phy-core.c, functions __phy_read_mmd() and __phy_write_mmd() it
means perform c45 transactions over the bus. We know we want to access
a register in c45 space because we are using an _mmd() function.

In phy.c, it means does this PHY have c45 registers and we should
access that register space, or should we use the c22 register
space. So far example phy_restart_aneg() decides to either call
genphy_c45_restart_aneg() or genphy_restart_aneg() depending on
is_c45.

So a PHY with C45 register space but only accessible by C45 over C22
is probably going to do the wrong thing with the current code.

For this patchset to work, we need to cleanly separate the concepts of
what sort of transactions to do over the bus, from what register
spaces the PHY has. We probably want something like phydev->has_c45 to
indicate the register space is implemented, and phydev->c45_over_c22
to indicate what sort of transaction should be used in the _mmd()
functions.

Your patches start in that direction, but i don't think it goes far
enough.

	Andrew