Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH NET] net: atlantic: Use readx_poll_timeout() for large timeout
@ 2020-08-18 16:14 Sebastian Andrzej Siewior
  2020-08-19 16:19 ` Guenter Roeck
  2020-08-19 23:25 ` David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-08-18 16:14 UTC (permalink / raw)
  To: Igor Russkikh, netdev, Mark Starovoytov
  Cc: David S. Miller, Jakub Kicinski, Thomas Gleixner

Commit
   8dcf2ad39fdb2 ("net: atlantic: add hwmon getter for MAC temperature")

implemented a read callback with an udelay(10000U). This fails to
compile on ARM because the delay is >1ms. I doubt that it is needed to
spin for 10ms even if possible on x86.

From looking at the code, the context appears to be preemptible so using
usleep() should work and avoid busy spinning.

Use readx_poll_timeout() in the poll loop.

Cc: Mark Starovoytov <mstarovoitov@marvell.com>
Cc: Igor Russkikh <irusskikh@marvell.com>
Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---

Could someone with hardware please verify it? It compiles, yes.

 drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 16a944707ba90..8941ac4df9e37 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -1631,8 +1631,8 @@ static int hw_atl_b0_get_mac_temp(struct aq_hw_s *self, u32 *temp)
 		hw_atl_ts_reset_set(self, 0);
 	}
 
-	err = readx_poll_timeout_atomic(hw_atl_b0_ts_ready_and_latch_high_get,
-					self, val, val == 1, 10000U, 500000U);
+	err = readx_poll_timeout(hw_atl_b0_ts_ready_and_latch_high_get, self,
+				 val, val == 1, 10000U, 500000U);
 	if (err)
 		return err;
 
-- 
2.28.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH NET] net: atlantic: Use readx_poll_timeout() for large timeout
  2020-08-18 16:14 [PATCH NET] net: atlantic: Use readx_poll_timeout() for large timeout Sebastian Andrzej Siewior
@ 2020-08-19 16:19 ` Guenter Roeck
  2020-08-20 10:17   ` [EXT] " Igor Russkikh
  2020-08-19 23:25 ` David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2020-08-19 16:19 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Igor Russkikh, netdev, Mark Starovoytov, David S. Miller,
	Jakub Kicinski, Thomas Gleixner

On Tue, Aug 18, 2020 at 06:14:39PM +0200, Sebastian Andrzej Siewior wrote:
> Commit
>    8dcf2ad39fdb2 ("net: atlantic: add hwmon getter for MAC temperature")
> 
> implemented a read callback with an udelay(10000U). This fails to
> compile on ARM because the delay is >1ms. I doubt that it is needed to
> spin for 10ms even if possible on x86.
> 
> >From looking at the code, the context appears to be preemptible so using
> usleep() should work and avoid busy spinning.
> 
> Use readx_poll_timeout() in the poll loop.
> 
> Cc: Mark Starovoytov <mstarovoitov@marvell.com>
> Cc: Igor Russkikh <irusskikh@marvell.com>
> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>

Fixes: 8dcf2ad39fdb2 ("net: atlantic: add hwmon getter for MAC temperature")
Acked-by: Guenter Roeck <linux@roeck-us.net>

As in: This patch does not cause any additional trouble and will fix the
observed compile failure. However, the submitter of 8dcf2ad39fdb2 might
consider adding a mutex either into hw_atl_b0_get_mac_temp() or into
the calling code.

Thanks,
Guenter

> ---
> 
> Could someone with hardware please verify it? It compiles, yes.
> 
>  drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
> index 16a944707ba90..8941ac4df9e37 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
> @@ -1631,8 +1631,8 @@ static int hw_atl_b0_get_mac_temp(struct aq_hw_s *self, u32 *temp)
>  		hw_atl_ts_reset_set(self, 0);
>  	}
>  
> -	err = readx_poll_timeout_atomic(hw_atl_b0_ts_ready_and_latch_high_get,
> -					self, val, val == 1, 10000U, 500000U);
> +	err = readx_poll_timeout(hw_atl_b0_ts_ready_and_latch_high_get, self,
> +				 val, val == 1, 10000U, 500000U);
>  	if (err)
>  		return err;
>  
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH NET] net: atlantic: Use readx_poll_timeout() for large timeout
  2020-08-18 16:14 [PATCH NET] net: atlantic: Use readx_poll_timeout() for large timeout Sebastian Andrzej Siewior
  2020-08-19 16:19 ` Guenter Roeck
@ 2020-08-19 23:25 ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2020-08-19 23:25 UTC (permalink / raw)
  To: bigeasy; +Cc: irusskikh, netdev, mstarovoitov, kuba, tglx

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: Tue, 18 Aug 2020 18:14:39 +0200

> Commit
>    8dcf2ad39fdb2 ("net: atlantic: add hwmon getter for MAC temperature")
> 
> implemented a read callback with an udelay(10000U). This fails to
> compile on ARM because the delay is >1ms. I doubt that it is needed to
> spin for 10ms even if possible on x86.
> 
> From looking at the code, the context appears to be preemptible so using
> usleep() should work and avoid busy spinning.
> 
> Use readx_poll_timeout() in the poll loop.
> 
> Cc: Mark Starovoytov <mstarovoitov@marvell.com>
> Cc: Igor Russkikh <irusskikh@marvell.com>
> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>

Applied, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [EXT] Re: [PATCH NET] net: atlantic: Use readx_poll_timeout() for large timeout
  2020-08-19 16:19 ` Guenter Roeck
@ 2020-08-20 10:17   ` Igor Russkikh
  0 siblings, 0 replies; 4+ messages in thread
From: Igor Russkikh @ 2020-08-20 10:17 UTC (permalink / raw)
  To: Guenter Roeck, Sebastian Andrzej Siewior
  Cc: netdev, Jakub Kicinski, Thomas Gleixner, Dmitry Bogdanov


>> implemented a read callback with an udelay(10000U). This fails to
>> compile on ARM because the delay is >1ms. I doubt that it is needed to
>> spin for 10ms even if possible on x86.
>>
>> >From looking at the code, the context appears to be preemptible so
> using
>> usleep() should work and avoid busy spinning.
>>
>> Use readx_poll_timeout() in the poll loop.
>>
>> Cc: Mark Starovoytov <mstarovoitov@marvell.com>
>> Cc: Igor Russkikh <irusskikh@marvell.com>
>> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> 
> Fixes: 8dcf2ad39fdb2 ("net: atlantic: add hwmon getter for MAC
> temperature")
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> 
> As in: This patch does not cause any additional trouble and will fix the
> observed compile failure. However, the submitter of 8dcf2ad39fdb2 might
> consider adding a mutex either into hw_atl_b0_get_mac_temp() or into
> the calling code.

Hi Sebastian, Guenter, thanks for catching and taking care of this,
Looks good for me so far.

>> Could someone with hardware please verify it? It compiles, yes.
>>

We'll verify this on our side, sure.

Regards,
  Igor

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-08-20 10:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-18 16:14 [PATCH NET] net: atlantic: Use readx_poll_timeout() for large timeout Sebastian Andrzej Siewior
2020-08-19 16:19 ` Guenter Roeck
2020-08-20 10:17   ` [EXT] " Igor Russkikh
2020-08-19 23:25 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).