[PATCH v2] b43: Replace mdelay with usleep_range in b43_radio_2057_init

Discussion:

[PATCH v2] b43: Replace mdelay with usleep_range in b43_radio_2057_init_post

Jia-Ju Bai

2018-01-09 01:40:06 UTC

b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.

Signed-off-by: Jia-Ju Bai <***@gmail.com>
---
v2:
* Replace mdelay with usleep_range, instead of msleep in v1.
Thank Larry for good advice.
---
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev *dev)

b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
- mdelay(2);
+ usleep_range(2000, 3000);
b43_radio_mask(dev, R2057_RFPLL_MISC_CAL_RESETN, ~0x78);
b43_radio_mask(dev, R2057_XTAL_CONFIG2, ~0x80);

--
1.7.9.5

Greg KH

2018-01-09 08:35:41 UTC

Permalink

Post by Jia-Ju Bai
b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.
---
* Replace mdelay with usleep_range, instead of msleep in v1.
Thank Larry for good advice.
---
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev *dev)
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
- mdelay(2);
+ usleep_range(2000, 3000);

Where did 3000 come from? Are you sure about that?

thanks,

greg k-h

Jia-Ju Bai

2018-01-09 08:39:31 UTC

Permalink

Post by Greg KH

Where did 3000 come from? Are you sure about that?
I had negative comments on one of those due to the possibility of
msleep(2) extending as long as 20 msec. Until the author, or someone
else, can test that this is OK, then the mdelay(2) can only be
replaced with usleep_range(2000, 3000).

Thanks,
Jia-Ju Bai

Arend van Spriel

2018-01-09 09:07:22 UTC

Permalink

Post by Greg KH

Post by Jia-Ju Bai
b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.
---
* Replace mdelay with usleep_range, instead of msleep in v1.
Thank Larry for good advice.
---
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c
b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev *dev)
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
- mdelay(2);
+ usleep_range(2000, 3000);

Where did 3000 come from? Are you sure about that?

Hi Jia-Ju Bai,

The duration here is for settling the registers so hardware can pick it
up. Right after this they are written again. Now this is during
initialization of the radio so not time critical, but probably anything
in the range of 2000..3000 would also have been fine.

Regards,
Arend

Jia-Ju Bai

2018-01-09 09:47:58 UTC

Permalink

Post by Arend van Spriel

Post by Greg KH

Post by Jia-Ju Bai
b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.
---
* Replace mdelay with usleep_range, instead of msleep in v1.
Thank Larry for good advice.
---
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c
b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev *dev)
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
- mdelay(2);
+ usleep_range(2000, 3000);

Where did 3000 come from? Are you sure about that?

Hi Jia-Ju Bai,
The duration here is for settling the registers so hardware can pick
it up. Right after this they are written again. Now this is during
initialization of the radio so not time critical, but probably
anything in the range of 2000..3000 would also have been fine.

Hi Arend,

Thanks for your detailed explanation :)
So I think usleep_range(2000, 3000) is okay.

Thanks,
Jia-Ju Bai

Arend van Spriel

2018-01-09 11:11:58 UTC

Permalink

Post by Jia-Ju Bai

Post by Arend van Spriel

Post by Greg KH

Post by Jia-Ju Bai
b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.
---
* Replace mdelay with usleep_range, instead of msleep in v1.
Thank Larry for good advice.
---
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c
b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev *dev)
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
- mdelay(2);
+ usleep_range(2000, 3000);

Where did 3000 come from? Are you sure about that?

Hi Jia-Ju Bai,
The duration here is for settling the registers so hardware can pick
it up. Right after this they are written again. Now this is during
initialization of the radio so not time critical, but probably
anything in the range of 2000..3000 would also have been fine.

Hi Arend,
Thanks for your detailed explanation :)
So I think usleep_range(2000, 3000) is okay.

Sure.

Regards,
Arend

Larry Finger

2018-01-09 16:49:58 UTC

Permalink

I agree that a sleep of 2-3 ms should be OK here.

Acked-by: Larry Finger <***@lwfinger.net>

Larry

Post by Jia-Ju Bai
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev *dev)
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
- mdelay(2);
+ usleep_range(2000, 3000);
b43_radio_mask(dev, R2057_RFPLL_MISC_CAL_RESETN, ~0x78);
b43_radio_mask(dev, R2057_XTAL_CONFIG2, ~0x80);

Kalle Valo

2018-01-11 19:54:29 UTC

Permalink

Post by Jia-Ju Bai
b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.

Patch applied to wireless-drivers-next.git, thanks.

4330b53e9662 b43: Replace mdelay with usleep_range in b43_radio_2057_init_post

--
https://patchwork.kernel.org/patch/10150845/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches