subject:"\[PATCH\] ixgbe\: Wait for 1ms, not 1us, after RST"

Re: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

2015-10-27 Thread Peter Hurley

Hi Dan,

On 10/26/2015 08:16 PM, dan.street...@canonical.com wrote:
> From: Dan Streetman 
> 
> The driver currently waits 1us after issuing a RST, but the spec
> requires it to wait 1ms.
> 
> Signed-off-by: Dan Streetman 
> Signed-off-by: Dan Streetman 
> ---
>  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);

Since ixgbe_reset_hw_x540() goes on to msleep(100) immediately after this
busy-wait loop, this should instead be:

msleep(1);

Regards,
Peter Hurley


>   ctrl = IXGBE_READ_REG(hw, IXGBE_CTRL);
>   if (!(ctrl & IXGBE_CTRL_RST_MASK))
>   break;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

2015-10-27 Thread Dan Streetman

On Tue, Oct 27, 2015 at 1:03 PM, Skidmore, Donald C
 wrote:
>
>
>> -Original Message-
>> From: dan.street...@canonical.com
>> [mailto:dan.street...@canonical.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-
>> l...@lists.osuosl.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> Dan Streetman; Dan Streetman
>> Subject: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST
>>
>> From: Dan Streetman 
>>
>> The driver currently waits 1us after issuing a RST, but the spec requires it 
>> to
>> wait 1ms.
>>
>> Signed-off-by: Dan Streetman 
>> Signed-off-by: Dan Streetman 
>> ---
>>  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. :)

I don't know the hw details of course, I was just going on the spec's
use of "must" when stating how long the driver should wait before
talking to the hw.  If the hw doesn't actually care, then no need for
this patch (although the spec should probably be changed to not use
"must").

Thanks!

>
> 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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2015-10-27 Thread Skidmore, Donald C



> -Original Message-
> From: dan.street...@canonical.com
> [mailto:dan.street...@canonical.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-
> l...@lists.osuosl.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> Dan Streetman; Dan Streetman
> Subject: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST
> 
> From: Dan Streetman 
> 
> The driver currently waits 1us after issuing a RST, but the spec requires it 
> to
> wait 1ms.
> 
> Signed-off-by: Dan Streetman 
> Signed-off-by: Dan Streetman 
> ---
>  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 linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2015-10-27 Thread Skidmore, Donald C



> -Original Message-
> From: dan.street...@canonical.com
> [mailto:dan.street...@canonical.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-
> l...@lists.osuosl.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> Dan Streetman; Dan Streetman
> Subject: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST
> 
> From: Dan Streetman <dan.street...@canonical.com>
> 
> The driver currently waits 1us after issuing a RST, but the spec requires it 
> to
> wait 1ms.
> 
> Signed-off-by: Dan Streetman <dan.street...@canonical.com>
> Signed-off-by: Dan Streetman <ddstr...@ieee.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 linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

2015-10-27 Thread Dan Streetman

On Tue, Oct 27, 2015 at 1:03 PM, Skidmore, Donald C
<donald.c.skidm...@intel.com> wrote:
>
>
>> -Original Message-
>> From: dan.street...@canonical.com
>> [mailto:dan.street...@canonical.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-
>> l...@lists.osuosl.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> Dan Streetman; Dan Streetman
>> Subject: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST
>>
>> From: Dan Streetman <dan.street...@canonical.com>
>>
>> The driver currently waits 1us after issuing a RST, but the spec requires it 
>> to
>> wait 1ms.
>>
>> Signed-off-by: Dan Streetman <dan.street...@canonical.com>
>> Signed-off-by: Dan Streetman <ddstr...@ieee.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. :)

I don't know the hw details of course, I was just going on the spec's
use of "must" when stating how long the driver should wait before
talking to the hw.  If the hw doesn't actually care, then no need for
this patch (although the spec should probably be changed to not use
"must").

Thanks!

>
> 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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

2015-10-27 Thread Peter Hurley

Hi Dan,

On 10/26/2015 08:16 PM, dan.street...@canonical.com wrote:
> From: Dan Streetman 
> 
> The driver currently waits 1us after issuing a RST, but the spec
> requires it to wait 1ms.
> 
> Signed-off-by: Dan Streetman 
> Signed-off-by: Dan Streetman 
> ---
>  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);

Since ixgbe_reset_hw_x540() goes on to msleep(100) immediately after this
busy-wait loop, this should instead be:

msleep(1);

Regards,
Peter Hurley


>   ctrl = IXGBE_READ_REG(hw, IXGBE_CTRL);
>   if (!(ctrl & IXGBE_CTRL_RST_MASK))
>   break;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2015-10-26 Thread dan . streetman

From: Dan Streetman 

The driver currently waits 1us after issuing a RST, but the spec
requires it to wait 1ms.

Signed-off-by: Dan Streetman 
Signed-off-by: Dan Streetman 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2015-10-26 Thread dan . streetman

From: Dan Streetman 

The driver currently waits 1us after issuing a RST, but the spec
requires it to wait 1ms.

Signed-off-by: Dan Streetman 
Signed-off-by: Dan Streetman 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

Re: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

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

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

Re: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

Re: [PATCH] ixgbe: Wait for 1ms, not 1us, after RST

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

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

8 matches

Site Navigation

Mail list logo

Footer information