netdev - RE: [PATCH] net: phy: at803x: Improve hibernation support on start up

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]

Date: Wed, 9 Aug 2023 05:37:45 +0000
From: Wei Fang <wei.fang@....com>
To: Oleksij Rempel <o.rempel@...gutronix.de>
CC: Marek Vasut <marex@...x.de>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>, Andrew
 Lunn <andrew@...n.ch>, Eric Dumazet <edumazet@...gle.com>, Heiner Kallweit
	<hkallweit1@...il.com>, Jakub Kicinski <kuba@...nel.org>, Oleksij Rempel
	<linux@...pel-privat.de>, Paolo Abeni <pabeni@...hat.com>, Russell King
	<linux@...linux.org.uk>
Subject: RE: [PATCH] net: phy: at803x: Improve hibernation support on start up

> > For the patch, I think your approach is better than mine, but I have a
> > suggestion, is the following modification more appropriate?
> >
> > --- a/drivers/net/phy/at803x.c
> > +++ b/drivers/net/phy/at803x.c
> > @@ -991,12 +991,28 @@ static int at8031_pll_config(struct phy_device
> > *phydev)  static int at803x_hibernation_mode_config(struct phy_device
> > *phydev)  {
> >         struct at803x_priv *priv = phydev->priv;
> > +       int ret;
> >
> >         /* The default after hardware reset is hibernation mode enabled.
> After
> >          * software reset, the value is retained.
> >          */
> > -       if (!(priv->flags & AT803X_DISABLE_HIBERNATION_MODE))
> > -               return 0;
> > +       if (!(priv->flags & AT803X_DISABLE_HIBERNATION_MODE)) {
> > +               /* Toggle hibernation mode OFF and ON to wake the
> PHY up and
> > +                * make it generate clock on RX_CLK pin for about 10
> seconds.
> > +                * These clock are needed during start up by MACs like
> DWMAC
> > +                * in NXP i.MX8M Plus to release their DMA from reset.
> After
> > +                * the MAC has started up, the PHY can enter
> hibernation and
> > +                * disable the RX_CLK clock, this poses no problem for
> the MAC.
> > +                */
> > +               ret = at803x_debug_reg_mask(phydev,
> AT803X_DEBUG_REG_HIB_CTRL,
> > +
> AT803X_DEBUG_HIB_CTRL_PS_HIB_EN, 0);
> > +               if (ret < 0)
> > +                       return ret;
> > +
> > +               return at803x_debug_reg_mask(phydev,
> AT803X_DEBUG_REG_HIB_CTRL,
> > +
> AT803X_DEBUG_HIB_CTRL_PS_HIB_EN,
> > +
> AT803X_DEBUG_HIB_CTRL_PS_HIB_EN);
> > +       }
> >
> >         return at803x_debug_reg_mask(phydev,
> > AT803X_DEBUG_REG_HIB_CTRL,
> 
> Hm.. how about officially defining this PHY as the clock provider and disable
> PHY automatic hibernation as long as clock is acquired?
> 
Sorry, I don't know much about the clock provider/consumer, but I think there
will be more changes if we use clock provider/consume mechanism. Furthermore,
we would expect the hibernation mode is enabled when the ethernet interface
is brought up but the cable is not plugged, that is to say, we only need the PHY to
provide the clock for a while to make the MAC reset successfully. Therefore, I think
the current approach is more simple and effective, and it takes full advantage of the
characteristics of the hardware (The PHY will continue to provide the clock about
10 seconds after hibernation mode is enabled when the cable is not plugged and
automatically disable the clock after 10 seconds, so the 10 seconds is enough for
the MAC to reset successfully).