netdev - Re: [net-next] net: ethernet: rtsn: Add support for Renesas Ethernet-TSN

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: <d5f6f31a-6ecc-48a9-a2ca-9d22fc6acb21@lunn.ch>
Date: Mon, 6 May 2024 03:51:45 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [net-next] net: ethernet: rtsn: Add support for Renesas
 Ethernet-TSN

> > phy_read_mmd():
> >   __phy_read_mmd():
> >       mmd_phy_read():
> > 
> > So is is_c45 is true?
> 
> Not sure, I never get that far. The function __mdiobus_c45_read() is 
> called directly from of_mdiobus_register() call-chain.
> 
> The call chain is:
> 
>   rtsn_open()
>     of_mdiobus_register() <- This fails and RTSN can't talk to the PHY
>       __of_mdiobus_register()
>         __of_mdiobus_parse_phys()
>           of_mdiobus_register_phy()
>             fwnode_mdiobus_register_phy() <- See [1]
>               get_phy_device()
>                 get_phy_c45_ids()
>                   mdiobus_c45_read()
>                     __mdiobus_c45_read() <- Returns -EOPNOTSUPP [2]
> 
> 1. Here is_c45 is set as it checks the compatible value is checked.
> 
>      is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");

Ah, O.K.

What PHY is this? Does it have C22 registers? Can it be identified via
C22 registers 2 and 3?

I suspect we in falling into the cracks with what is_c45 means. And
what ethernet-phy-ieee802.3-c45 means.

is_c45 is a bad mis-mash of responds to C45 bus transfers and has C45
registers.

Your MDIO bus master appears to be unable to perform C45 bus
transfers. So you probably don't want is_c45 set, so that C45 over C22
is performed. However, you are using ethernet-phy-ieee802.3-c45 which
suggests to me the PHY has C45 registers.

A PHY driver itself mostly does not care about is_c45. It knows the
device has C45 registers and will use phy_read_mmd() to access
them. If that uses C45 bus transfers or C45 over C22 is left to the
core, based on is_c45.

Where it gets a little problematic is determining the correct driver
for the PHY. By default it will look at registers 2 and 3 of C22. If
it finds an usable ID, that will be used to load the driver. If there
is no ID in C22 registers, and the MDIO bus supports C45 bus
transfers, it will try the various places there can be IDs in the C45
register space.

What the core will not do is use C45 over C22 to look for ID
registers, because it has no idea what is actually there, and C45 over
C22 means performing an write, which could destroy the configuration
of something which is not a PHY, e.g. a switch, or a GPIO controller
etc.

However, by specifying "ethernet-phy-ieee802.3-c45", you have short
cut the default, it goes direct to C45 bus transfers which your
hardware cannot actually do.

So i would drop the compatible. See if C22 is sufficient to get the
correct driver loaded.

	Andrew