| Message ID | 20170613133348.48044-2-ramesh.shanmugasundaram@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Ramesh, On Tue, Jun 13, 2017 at 3:33 PM, Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com> wrote: > Renamed variable "timeout" to "__timeout" to avoid namespace collision. > Tidy up macro arguments with paranthesis. > > Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com> Thanks for your patches! > --- a/include/linux/iopoll.h > +++ b/include/linux/iopoll.h > @@ -42,18 +42,19 @@ > */ > #define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us) \ > ({ \ > - ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \ > + ktime_t __timeout = ktime_add_us(ktime_get(), timeout_us); \ I think timeout_us should be within parentheses, too. > might_sleep_if(sleep_us); \ > for (;;) { \ > (val) = op(addr); \ > if (cond) \ > break; \ > - if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \ > + if ((timeout_us) && \ > + ktime_compare(ktime_get(), __timeout) > 0) { \ > (val) = op(addr); \ > break; \ > } \ > if (sleep_us) \ > - usleep_range((sleep_us >> 2) + 1, sleep_us); \ > + usleep_range(((sleep_us) >> 2) + 1, sleep_us); \ Same for sleep_us. Also in readx_poll_timeout_atomic(), and in your second patch. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for the review. Replying to the thread to update what we discussed in IRC sometime back. > On Tue, Jun 13, 2017 at 3:33 PM, Ramesh Shanmugasundaram > <ramesh.shanmugasundaram@bp.renesas.com> wrote: > > Renamed variable "timeout" to "__timeout" to avoid namespace collision. > > Tidy up macro arguments with paranthesis. > > > > Signed-off-by: Ramesh Shanmugasundaram > > <ramesh.shanmugasundaram@bp.renesas.com> > > Thanks for your patches! > > > --- a/include/linux/iopoll.h > > +++ b/include/linux/iopoll.h > > @@ -42,18 +42,19 @@ > > */ > > #define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us) > > \ ({ \ > > - ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \ > > + ktime_t __timeout = ktime_add_us(ktime_get(), timeout_us); \ > > I think timeout_us should be within parentheses, too. It is not required as it is passed as an function (ktime_add_us) argument. > > > might_sleep_if(sleep_us); \ > > for (;;) { \ > > (val) = op(addr); \ > > if (cond) \ > > break; \ > > - if (timeout_us && ktime_compare(ktime_get(), timeout) > > 0) { \ > > + if ((timeout_us) && \ > > + ktime_compare(ktime_get(), __timeout) > 0) { \ > > (val) = op(addr); \ > > break; \ > > } \ > > if (sleep_us) \ > > - usleep_range((sleep_us >> 2) + 1, sleep_us); \ > > + usleep_range(((sleep_us) >> 2) + 1, sleep_us); > > + \ > > Same for sleep_us. > > Also in readx_poll_timeout_atomic(), and in your second patch. Same as the above comment. Thanks, Ramesh
diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h index d29e1e21bf3f..e000172bee54 100644 --- a/include/linux/iopoll.h +++ b/include/linux/iopoll.h @@ -42,18 +42,19 @@ */ #define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us) \ ({ \ - ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \ + ktime_t __timeout = ktime_add_us(ktime_get(), timeout_us); \ might_sleep_if(sleep_us); \ for (;;) { \ (val) = op(addr); \ if (cond) \ break; \ - if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \ + if ((timeout_us) && \ + ktime_compare(ktime_get(), __timeout) > 0) { \ (val) = op(addr); \ break; \ } \ if (sleep_us) \ - usleep_range((sleep_us >> 2) + 1, sleep_us); \ + usleep_range(((sleep_us) >> 2) + 1, sleep_us); \ } \ (cond) ? 0 : -ETIMEDOUT; \ }) @@ -77,12 +78,13 @@ */ #define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \ ({ \ - ktime_t timeout = ktime_add_us(ktime_get(), timeout_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) { \ + if ((timeout_us) && \ + ktime_compare(ktime_get(), __timeout) > 0) { \ (val) = op(addr); \ break; \ } \
Renamed variable "timeout" to "__timeout" to avoid namespace collision. Tidy up macro arguments with paranthesis. Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com> --- include/linux/iopoll.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)