netdev - Re: net: asix: best way to handle orphan PHYs

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: <20220309144523.GE15680@pengutronix.de>
Date:   Wed, 9 Mar 2022 15:45:23 +0100
From:   Oleksij Rempel <o.rempel@...gutronix.de>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, kernel@...gutronix.de,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        netdev@...r.kernel.org, paskripkin@...il.com
Subject: Re: net: asix: best way to handle orphan PHYs

Hi Andrew,

On Wed, Mar 09, 2022 at 02:31:18PM +0100, Andrew Lunn wrote:
> On Wed, Mar 09, 2022 at 01:18:35PM +0100, Oleksij Rempel wrote:
> > Hello all,
> > 
> > I have ASIX based USB Ethernet adapter with two PHYs: internal and
> > external. The internal PHY is enabled by default and there seems to be
> > no way to disable internal PHY on the MAC level without affecting the
> > external PHY.
> > 
> > What is the preferred method to suspend internal PHY?
> > Currently I have following options:
> > - suspend PHY in the probe function of the PHY driver
> > - get the phydev in the MAC driver and call phy_suspend()
> > - whisper magic numbers from the MAC driver directly this the MDIO bus.
> > 
> > Are there other options?
> 
> Hi Oleksij
> 
> Can you unique identity this device? Does it have a custom VID:PID?

No, currently it has generic VID:PID.

> It seems like suspending it in the PHY driver would be messy. How do
> you identify the PHY is part of your devices and should be suspended?

EEPROM provides information, which PHY address should be used. If
address is 0x10, it is internal PHY. Different parts of ASIX driver use
this logic.

> Doing it from the MAC driver seems better, your identification
> information is close to hand.
> 
> I would avoid the magic numbers, since phy_suspend() makes it clear
> what you are doing.
> 
> Is there one MDIO bus with two devices, or two MDIO busses?  If there
> are two busses, you could maybe add an extra flag to the bus structure
> you pass to mdiobus_register() which indicates it should suspend all
> PHY it finds on the bus during enumeration of the bus. Generally we
> don't want this, if the PHY has link already we want to keep it, to
> avoid the 1.5s delay causes by autoneg. But if we know the PHYs on the
> bus are not going to be used, it would be a good point to suspend
> them.

It is one MDIO bus with multiple PHYs one of them is the internal PHY.
ax88772 seems to provide way to put the PHY to reset from one of MAC
register. See drivers/net/usb/asix_devices.c
ax88772_hw_reset()
  if (priv->embd_phy)
    ...
  else
    asix_sw_reset(dev, AX_SWRESET_IPPD | AX_SWRESET_PRL,

But this way is not working for the ax88772b variant.

Ok, so if phy_suspend() is the preffered way, I need to get phydev
without attaching it. Correct? Do we already have some helpers to do it?

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |