netdev - RE: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 27 Oct 2015 17:03:34 +0000
From:	"Skidmore, Donald C" <donald.c.skidmore@...el.com>
To:	"dan.streetman@...onical.com" <dan.streetman@...onical.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	"Nelson, Shannon" <shannon.nelson@...el.com>,
	"Wyborny, Carolyn" <carolyn.wyborny@...el.com>,
	"Vick, Matthew" <matthew.vick@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"Williams, Mitch A" <mitch.a.williams@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Dan Streetman <ddstreet@...e.org>
Subject: RE: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST



> -----Original Message-----
> From: dan.streetman@...onical.com
> [mailto:dan.streetman@...onical.com]
> Sent: Monday, October 26, 2015 5:16 PM
> To: Kirsher, Jeffrey T
> Cc: Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore,
> Donald C; Vick, Matthew; Ronciak, John; Williams, Mitch A; intel-wired-
> lan@...ts.osuosl.org; netdev@...r.kernel.org; linux-kernel@...r.kernel.org;
> Dan Streetman; Dan Streetman
> Subject: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST
> 
> From: Dan Streetman <dan.streetman@...onical.com>
> 
> The driver currently waits 1us after issuing a RST, but the spec requires it to
> wait 1ms.
> 
> Signed-off-by: Dan Streetman <dan.streetman@...onical.com>
> Signed-off-by: Dan Streetman <ddstreet@...e.org>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
> index 4e75843..147bc65 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
> @@ -113,7 +113,12 @@ mac_reset_top:
> 
>  	/* Poll for reset bit to self-clear indicating reset is complete */
>  	for (i = 0; i < 10; i++) {
> -		udelay(1);
> +		/* sec 8.2.4.1.1 :
> +		 * programmers must wait approximately 1 ms after setting
> before
> +		 * attempting to check if the bit has cleared or to access
> (read
> +		 * or write) any other device register.
> +		 */
> +		mdelay(1);
>  		ctrl = IXGBE_READ_REG(hw, IXGBE_CTRL);
>  		if (!(ctrl & IXGBE_CTRL_RST_MASK))
>  			break;
> --
> 2.5.0

While the Data Sheet does mention that this should take ~ 1ms, we are in a busy wait state so it probably isn't that big of a deal to check more frequently for our exit condition.  That said there are plenty of other delays later on in the reset path so keeping the udelay really isn't speeding things up much. :)

Also normally it isn't a good idea to reference a section number in the data sheet as they do seem to change with updates.  We are most likely a bit more safe here as it is one of the first of a list of register descriptions' and thus less like to move. 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html