netdev - Re: [PATCH net v2 2/9] net: axienet: Wait for PhyRstCmplt after core reset

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: <d0b00b8c96be17e6ad636f5a74ebfb170a603eac.camel@calian.com>
Date:   Wed, 12 Jan 2022 19:25:58 +0000
From:   Robert Hancock <robert.hancock@...ian.com>
To:     "andrew@...n.ch" <andrew@...n.ch>
CC:     "daniel@...earbox.net" <daniel@...earbox.net>,
        "radhey.shyam.pandey@...inx.com" <radhey.shyam.pandey@...inx.com>,
        "michal.simek@...inx.com" <michal.simek@...inx.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "ariane.keller@....ee.ethz.ch" <ariane.keller@....ee.ethz.ch>
Subject: Re: [PATCH net v2 2/9] net: axienet: Wait for PhyRstCmplt after core
 reset

On Wed, 2022-01-12 at 20:15 +0100, Andrew Lunn wrote:
> On Wed, Jan 12, 2022 at 11:36:53AM -0600, Robert Hancock wrote:
> > When resetting the device, wait for the PhyRstCmplt bit to be set
> > in the interrupt status register before continuing initialization, to
> > ensure that the core is actually ready. The MgtRdy bit could also be
> > waited for, but unfortunately when using 7-series devices, the bit does
> > not appear to work as documented (it seems to behave as some sort of
> > link state indication and not just an indication the transceiver is
> > ready) so it can't really be relied on.
> > 
> > Fixes: 8a3b7a252dca9 ("drivers/net/ethernet/xilinx: added Xilinx AXI
> > Ethernet driver")
> > Signed-off-by: Robert Hancock <robert.hancock@...ian.com>
> > ---
> >  drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > index f950342f6467..f425a8404a9b 100644
> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > @@ -516,6 +516,16 @@ static int __axienet_device_reset(struct axienet_local
> > *lp)
> >  		return ret;
> >  	}
> >  
> > +	/* Wait for PhyRstCmplt bit to be set, indicating the PHY reset has
> > finished */
> > +	ret = read_poll_timeout(axienet_ior, value,
> > +				value & XAE_INT_PHYRSTCMPLT_MASK,
> > +				DELAY_OF_ONE_MILLISEC, 50000, false, lp,
> > +				XAE_IS_OFFSET);
> > +	if (ret) {
> > +		dev_err(lp->dev, "%s: timeout waiting for PhyRstCmplt\n",
> > __func__);
> > +		return ret;
> > +	}
> > +
> 
> Is this bit guaranteed to be clear before you start waiting for it?

The documentation for the IP core ( 
https://www.xilinx.com/content/dam/xilinx/support/documentation/ip_documentation/axi_ethernet/v7_2/pg138-axi-ethernet.pdf
 ) states for the phy_rst_n output signal: "This active-Low reset is held
active for 10 ms after power is applied and during any reset. After the reset
goes inactive, the PHY cannot be accessed for an additional 5 ms." The
PhyRstComplt bit definition mentions "This signal does not transition to 1 for
5 ms after PHY_RST_N transitions to 1". Given that a reset of the core has just
been completed above, the PHY reset should at least have been initiated as
well, so it should be sufficient to just wait for the bit to become 1 at this
point.

> 
>    Andrew