| 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
| ||
|
Message-ID: <20230809043626.GG5736@pengutronix.de>
Date: Wed, 9 Aug 2023 06:36:26 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Wei Fang <wei.fang@....com>
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
On Wed, Aug 09, 2023 at 02:27:12AM +0000, Wei Fang wrote:
> > >> 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.
> > >>
> > >> Originally, this issue has been described by NXP in commit
> > >> 9ecf04016c87 ("net: phy: at803x: add disable hibernation mode
> > >> support") but this approach fully disables the hibernation support
> > >> and takes away any power saving benefit. This patch instead makes the
> > >> PHY generate the clock on start up for 10 seconds, which should be
> > >> long enough for the EQoS MAC to release DMA from reset.
> > >>
> > >> Before this patch on i.MX8M Plus board with AR8031 PHY:
> > >> "
> > >> $ ifconfig eth1 up
> > >> [ 25.576734] imx-dwmac 30bf0000.ethernet eth1: Register
> > >> MEM_TYPE_PAGE_POOL RxQ-0
> > >> [ 25.658916] imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:00]
> > >> driver [Qualcomm Atheros AR8031/AR8033] (irq=38)
> > >> [ 26.670276] imx-dwmac 30bf0000.ethernet: Failed to reset the dma
> > >> [ 26.676322] imx-dwmac 30bf0000.ethernet eth1: stmmac_hw_setup:
> > >> DMA engine initialization failed
> > >> [ 26.685103] imx-dwmac 30bf0000.ethernet eth1: __stmmac_open:
> > Hw
> > >> setup failed
> > >> ifconfig: SIOCSIFFLAGS: Connection timed out "
> > >>
> > >
> > > Have you reproduced this issue based on the upstream net-next or net
> > tree?
> >
> > On current linux-next next-20230808 so 6.5.0-rc5 . As far as I can tell,
> > net-next is merged into this tree too.
> >
>
> > > If so, can this issue be reproduced? The reason why I ask this is
> > > because when I tried to reproduce this problem on net-next 6.3.0
> > > version, I found that it could not be reproduced (I did not disable
> > > hibernation mode when I reproduced this issue ). So I guess maybe other
> > patches in eqos driver fixed the issue.
> >
> > This is what I use for testing:
> >
> > - Make sure "qca,disable-hibernation-mode" is NOT present in PHY DT node
> Yes, I deleted this property when I reproduced this issue.
>
> > - Boot the machine with NO ethernet cable plugged into the affected port
> > (i.e. the EQoS port), this is important
> > - Make sure the EQoS MAC is not brought up e.g. by systemd-networkd or
> > whatever other tool, I use busybox initramfs for testing with plain
> > script as init (it mounts the various filesystems and runs /bin/sh)
>
> It looks like something has been changed since I submitted the "disable hibernation
> mode " patch. In previous test, I only need to unplug the cable and then use ifconfig
> cmd to disable the interface, wait more than 10 seconds, then use ifconfig cmd to
> enable the interface.
>
> > - Wait longer than 10 seconds
> > - If possible, measure AR8031 PHY pin 33 RX_CLK, wait for the RX_CLK to
> > be turned OFF by the PHY (means PHY entered hibernation)
> > - ifconfig ethN up -- try to bring up the EQoS MAC <observe failure>
> >
> > [...]
>
> 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?
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 |
Powered by blists - more mailing lists