* [PATCH 2/2] rtw88: Use udelay instead of usleep in atomic context
[not found] <20200423063811.2636-1-kai.heng.feng@canonical.com>
@ 2020-04-23 6:38 ` Kai-Heng Feng
2020-04-23 6:49 ` Kalle Valo
2020-04-23 7:30 ` [PATCH v2 " Kai-Heng Feng
2020-04-24 18:49 ` [PATCH v2 1/2] iopoll: Introduce read_poll_timeout_atomic macro Kai-Heng Feng
1 sibling, 2 replies; 9+ messages in thread
From: Kai-Heng Feng @ 2020-04-23 6:38 UTC (permalink / raw)
To: yhchuang, kvalo
Cc: Kai-Heng Feng, David S. Miller,
open list:REALTEK WIRELESS DRIVER (rtw88),
open list:NETWORKING DRIVERS, open list
It's incorrect to use usleep in atomic context.
Switch to a macro which uses udelay instead of usleep to prevent the issue.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/net/wireless/realtek/rtw88/fw.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 245da96dfddc..8f998b4a7234 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -228,9 +228,9 @@ static void rtw_fw_send_h2c_command(struct rtw_dev *rtwdev,
goto out;
}
- ret = read_poll_timeout(rtw_read8, box_state,
- !((box_state >> box) & 0x1), 100, 3000, false,
- rtwdev, REG_HMETFR);
+ ret = read_poll_timeout_atomic(rtw_read8, box_state,
+ !((box_state >> box) & 0x1), 100, 3000,
+ false, rtwdev, REG_HMETFR);
if (ret) {
rtw_err(rtwdev, "failed to send h2c command\n");
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] rtw88: Use udelay instead of usleep in atomic context
2020-04-23 6:38 ` [PATCH 2/2] rtw88: Use udelay instead of usleep in atomic context Kai-Heng Feng
@ 2020-04-23 6:49 ` Kalle Valo
2020-04-23 6:53 ` Kai-Heng Feng
2020-04-23 7:30 ` [PATCH v2 " Kai-Heng Feng
1 sibling, 1 reply; 9+ messages in thread
From: Kalle Valo @ 2020-04-23 6:49 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: yhchuang, David S. Miller,
open list:REALTEK WIRELESS DRIVER (rtw88),
open list:NETWORKING DRIVERS, open list
Kai-Heng Feng <kai.heng.feng@canonical.com> writes:
> It's incorrect to use usleep in atomic context.
>
> Switch to a macro which uses udelay instead of usleep to prevent the issue.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
This fixes a regression, right? So there should be a Fixes line.
Also I can't take this until patch 1 is in my tree. And I don't know who
takes iopoll.h patches.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] rtw88: Use udelay instead of usleep in atomic context
2020-04-23 6:49 ` Kalle Valo
@ 2020-04-23 6:53 ` Kai-Heng Feng
2020-04-23 7:01 ` Kalle Valo
0 siblings, 1 reply; 9+ messages in thread
From: Kai-Heng Feng @ 2020-04-23 6:53 UTC (permalink / raw)
To: Kalle Valo
Cc: Tony Chuang, David S. Miller,
open list:REALTEK WIRELESS DRIVER (rtw88),
open list:NETWORKING DRIVERS, open list
> On Apr 23, 2020, at 14:49, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Kai-Heng Feng <kai.heng.feng@canonical.com> writes:
>
>> It's incorrect to use usleep in atomic context.
>>
>> Switch to a macro which uses udelay instead of usleep to prevent the issue.
>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>
> This fixes a regression, right? So there should be a Fixes line.
Yes, but the regression commit isn't in Linus' tree, so the sha1 may change.
Kai-Heng
>
> Also I can't take this until patch 1 is in my tree. And I don't know who
> takes iopoll.h patches.
>
> --
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] rtw88: Use udelay instead of usleep in atomic context
2020-04-23 6:53 ` Kai-Heng Feng
@ 2020-04-23 7:01 ` Kalle Valo
0 siblings, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2020-04-23 7:01 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: Tony Chuang, David S. Miller,
open list:REALTEK WIRELESS DRIVER (rtw88),
open list:NETWORKING DRIVERS, open list
Kai-Heng Feng <kai.heng.feng@canonical.com> writes:
>> On Apr 23, 2020, at 14:49, Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Kai-Heng Feng <kai.heng.feng@canonical.com> writes:
>>
>>> It's incorrect to use usleep in atomic context.
>>>
>>> Switch to a macro which uses udelay instead of usleep to prevent the issue.
>>>
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>
>> This fixes a regression, right? So there should be a Fixes line.
>
> Yes, but the regression commit isn't in Linus' tree, so the sha1 may change.
No, the commit id won't change after I have commited the patch. I don't
rebase my trees.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] rtw88: Use udelay instead of usleep in atomic context
2020-04-23 6:38 ` [PATCH 2/2] rtw88: Use udelay instead of usleep in atomic context Kai-Heng Feng
2020-04-23 6:49 ` Kalle Valo
@ 2020-04-23 7:30 ` Kai-Heng Feng
2020-04-23 16:07 ` Kalle Valo
2020-05-06 8:31 ` Kalle Valo
1 sibling, 2 replies; 9+ messages in thread
From: Kai-Heng Feng @ 2020-04-23 7:30 UTC (permalink / raw)
To: yhchuang, kvalo
Cc: Kai-Heng Feng, David S. Miller,
open list:REALTEK WIRELESS DRIVER (rtw88),
open list:NETWORKING DRIVERS, open list
It's incorrect to use usleep in atomic context.
Switch to a macro which uses udelay instead of usleep to prevent the issue.
Fixes: 6343a6d4b213 ("rtw88: Add delay on polling h2c command status bit")
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Add Fixes tag.
drivers/net/wireless/realtek/rtw88/fw.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 245da96dfddc..8f998b4a7234 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -228,9 +228,9 @@ static void rtw_fw_send_h2c_command(struct rtw_dev *rtwdev,
goto out;
}
- ret = read_poll_timeout(rtw_read8, box_state,
- !((box_state >> box) & 0x1), 100, 3000, false,
- rtwdev, REG_HMETFR);
+ ret = read_poll_timeout_atomic(rtw_read8, box_state,
+ !((box_state >> box) & 0x1), 100, 3000,
+ false, rtwdev, REG_HMETFR);
if (ret) {
rtw_err(rtwdev, "failed to send h2c command\n");
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] rtw88: Use udelay instead of usleep in atomic context
2020-04-23 7:30 ` [PATCH v2 " Kai-Heng Feng
@ 2020-04-23 16:07 ` Kalle Valo
2020-05-06 8:31 ` Kalle Valo
1 sibling, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2020-04-23 16:07 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: yhchuang, Kai-Heng Feng, David S. Miller,
open list:REALTEK WIRELESS DRIVER (rtw88),
open list:NETWORKING DRIVERS, open list
Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> It's incorrect to use usleep in atomic context.
>
> Switch to a macro which uses udelay instead of usleep to prevent the issue.
>
> Fixes: 6343a6d4b213 ("rtw88: Add delay on polling h2c command status bit")
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
For patch 1 please also CC linux-wireless, otherwise patchwork cannot see it.
--
https://patchwork.kernel.org/patch/11505147/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] iopoll: Introduce read_poll_timeout_atomic macro
[not found] <20200423063811.2636-1-kai.heng.feng@canonical.com>
2020-04-23 6:38 ` [PATCH 2/2] rtw88: Use udelay instead of usleep in atomic context Kai-Heng Feng
@ 2020-04-24 18:49 ` Kai-Heng Feng
2020-05-06 8:30 ` Kalle Valo
1 sibling, 1 reply; 9+ messages in thread
From: Kai-Heng Feng @ 2020-04-24 18:49 UTC (permalink / raw)
To: yhchuang, kvalo
Cc: linux-wireless, netdev, Kai-Heng Feng, David S. Miller,
Dejin Zheng, Allison Randal, Alexios Zavras,
Mauro Carvalho Chehab, Thomas Gleixner, open list
Like read_poll_timeout, an atomic variant for multiple parameter read
function can be useful.
Will be used by a later patch.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Cc linux-wireless.
include/linux/iopoll.h | 62 +++++++++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 19 deletions(-)
diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
index cb20c733b15a..bc89ac625f26 100644
--- a/include/linux/iopoll.h
+++ b/include/linux/iopoll.h
@@ -57,6 +57,48 @@
(cond) ? 0 : -ETIMEDOUT; \
})
+/**
+ * read_poll_timeout_atomic - Periodically poll an address until a condition is
+ * met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops). Should
+ * be less than ~10us since udelay is used (see
+ * Documentation/timers/timers-howto.rst).
+ * @timeout_us: Timeout in us, 0 means never timeout
+ * @delay_before_read: if it is true, delay @delay_us before read.
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @args is stored in @val.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define read_poll_timeout_atomic(op, val, cond, delay_us, timeout_us, \
+ delay_before_read, args...) \
+({ \
+ u64 __timeout_us = (timeout_us); \
+ unsigned long __delay_us = (delay_us); \
+ ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \
+ if (delay_before_read && __delay_us) \
+ udelay(__delay_us); \
+ for (;;) { \
+ (val) = op(args); \
+ if (cond) \
+ break; \
+ if (__timeout_us && \
+ ktime_compare(ktime_get(), __timeout) > 0) { \
+ (val) = op(args); \
+ break; \
+ } \
+ if (__delay_us) \
+ udelay(__delay_us); \
+ } \
+ (cond) ? 0 : -ETIMEDOUT; \
+})
+
/**
* readx_poll_timeout - Periodically poll an address until a condition is met or a timeout occurs
* @op: accessor function (takes @addr as its only argument)
@@ -96,25 +138,7 @@
* macros defined below rather than this macro directly.
*/
#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
-({ \
- u64 __timeout_us = (timeout_us); \
- unsigned long __delay_us = (delay_us); \
- ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \
- for (;;) { \
- (val) = op(addr); \
- if (cond) \
- break; \
- if (__timeout_us && \
- ktime_compare(ktime_get(), __timeout) > 0) { \
- (val) = op(addr); \
- break; \
- } \
- if (__delay_us) \
- udelay(__delay_us); \
- } \
- (cond) ? 0 : -ETIMEDOUT; \
-})
-
+ read_poll_timeout_atomic(op, val, cond, delay_us, timeout_us, false, addr)
#define readb_poll_timeout(addr, val, cond, delay_us, timeout_us) \
readx_poll_timeout(readb, addr, val, cond, delay_us, timeout_us)
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] iopoll: Introduce read_poll_timeout_atomic macro
2020-04-24 18:49 ` [PATCH v2 1/2] iopoll: Introduce read_poll_timeout_atomic macro Kai-Heng Feng
@ 2020-05-06 8:30 ` Kalle Valo
0 siblings, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2020-05-06 8:30 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: yhchuang, linux-wireless, netdev, Kai-Heng Feng, David S. Miller,
Dejin Zheng, Allison Randal, Alexios Zavras,
Mauro Carvalho Chehab, Thomas Gleixner, open list
Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> Like read_poll_timeout, an atomic variant for multiple parameter read
> function can be useful.
>
> Will be used by a later patch.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
57a29df34146 iopoll: Introduce read_poll_timeout_atomic macro
--
https://patchwork.kernel.org/patch/11508809/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] rtw88: Use udelay instead of usleep in atomic context
2020-04-23 7:30 ` [PATCH v2 " Kai-Heng Feng
2020-04-23 16:07 ` Kalle Valo
@ 2020-05-06 8:31 ` Kalle Valo
1 sibling, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2020-05-06 8:31 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: yhchuang, Kai-Heng Feng, David S. Miller,
open list:REALTEK WIRELESS DRIVER (rtw88),
open list:NETWORKING DRIVERS, open list
Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> It's incorrect to use usleep in atomic context.
>
> Switch to a macro which uses udelay instead of usleep to prevent the issue.
>
> Fixes: 6343a6d4b213 ("rtw88: Add delay on polling h2c command status bit")
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
fd5d781964b0 rtw88: Use udelay instead of usleep in atomic context
--
https://patchwork.kernel.org/patch/11505147/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-06 8:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200423063811.2636-1-kai.heng.feng@canonical.com>
2020-04-23 6:38 ` [PATCH 2/2] rtw88: Use udelay instead of usleep in atomic context Kai-Heng Feng
2020-04-23 6:49 ` Kalle Valo
2020-04-23 6:53 ` Kai-Heng Feng
2020-04-23 7:01 ` Kalle Valo
2020-04-23 7:30 ` [PATCH v2 " Kai-Heng Feng
2020-04-23 16:07 ` Kalle Valo
2020-05-06 8:31 ` Kalle Valo
2020-04-24 18:49 ` [PATCH v2 1/2] iopoll: Introduce read_poll_timeout_atomic macro Kai-Heng Feng
2020-05-06 8:30 ` Kalle Valo
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).