| Message ID | 1644481960-15049-1-git-send-email-wangqing@vivo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: wl128x: use time_after_eq() instead of jiffies judgment | expand |
Hi, All of these patches you've just sent say "Use time_after_eq()" in the subject, but I haven't yet seen a usage of that. Could you make your patch subject reflective of the true changes in each patch please? Batching them in a series as suggested by Joe would be helpful too. Quoting Qing Wang (2022-02-10 08:32:39) > From: Wang Qing <wangqing@vivo.com> > > It is better to use time_xxx() directly instead of jiffies judgment > for understanding. > > Signed-off-by: Wang Qing <wangqing@vivo.com> > --- > drivers/media/radio/wl128x/fmdrv_common.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c > index 6142484d..a599d08 > --- a/drivers/media/radio/wl128x/fmdrv_common.c > +++ b/drivers/media/radio/wl128x/fmdrv_common.c > @@ -23,6 +23,7 @@ > #include <linux/firmware.h> > #include <linux/module.h> > #include <linux/nospec.h> > +#include <linux/jiffies.h> > > #include "fmdrv.h" > #include "fmdrv_v4l2.h" > @@ -342,7 +343,7 @@ static void send_tasklet(struct tasklet_struct *t) > return; > > /* Check, is there any timeout happened to last transmitted packet */ > - if ((jiffies - fmdev->last_tx_jiffies) > FM_DRV_TX_TIMEOUT) { > + if (time_after(jiffies, fmdev->last_tx_jiffies + FM_DRV_TX_TIMEOUT)) { It looks like there are specific macros for working with jiffies too. Should this be time_is_after_jiffies(fmdev->last_tx_jiffies + FM_DRV_TX_TIMEOUT) { Although that is in fact 2 characters longer ;-S -- Kieran > fmerr("TX timeout occurred\n"); > atomic_set(&fmdev->tx_cnt, 1); > } > -- > 2.7.4 >
> >Hi, > >All of these patches you've just sent say "Use time_after_eq()" in the >subject, but I haven't yet seen a usage of that. > >Could you make your patch subject reflective of the true changes in each >patch please? > >Batching them in a series as suggested by Joe would be helpful too. > My fault, I will correct and batch them in a series for V2 >Quoting Qing Wang (2022-02-10 08:32:39) >> From: Wang Qing <wangqing@vivo.com> >> >> It is better to use time_xxx() directly instead of jiffies judgment >> for understanding. >> >> Signed-off-by: Wang Qing <wangqing@vivo.com> >> --- >> drivers/media/radio/wl128x/fmdrv_common.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c >> index 6142484d..a599d08 >> --- a/drivers/media/radio/wl128x/fmdrv_common.c >> +++ b/drivers/media/radio/wl128x/fmdrv_common.c >> @@ -23,6 +23,7 @@ >> #include <linux/firmware.h> >> #include <linux/module.h> >> #include <linux/nospec.h> >> +#include <linux/jiffies.h> >> >> #include "fmdrv.h" >> #include "fmdrv_v4l2.h" >> @@ -342,7 +343,7 @@ static void send_tasklet(struct tasklet_struct *t) >> return; >> >> /* Check, is there any timeout happened to last transmitted packet */ >> - if ((jiffies - fmdev->last_tx_jiffies) > FM_DRV_TX_TIMEOUT) { >> + if (time_after(jiffies, fmdev->last_tx_jiffies + FM_DRV_TX_TIMEOUT)) { > >It looks like there are specific macros for working with jiffies too. > >Should this be > time_is_after_jiffies(fmdev->last_tx_jiffies + FM_DRV_TX_TIMEOUT) { > >Although that is in fact 2 characters longer ;-S Accept it, thanks Qing > > >-- >Kieran > > >> fmerr("TX timeout occurred\n"); >> atomic_set(&fmdev->tx_cnt, 1); >> } >> -- >> 2.7.4 >>
diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c index 6142484d..a599d08 --- a/drivers/media/radio/wl128x/fmdrv_common.c +++ b/drivers/media/radio/wl128x/fmdrv_common.c @@ -23,6 +23,7 @@ #include <linux/firmware.h> #include <linux/module.h> #include <linux/nospec.h> +#include <linux/jiffies.h> #include "fmdrv.h" #include "fmdrv_v4l2.h" @@ -342,7 +343,7 @@ static void send_tasklet(struct tasklet_struct *t) return; /* Check, is there any timeout happened to last transmitted packet */ - if ((jiffies - fmdev->last_tx_jiffies) > FM_DRV_TX_TIMEOUT) { + if (time_after(jiffies, fmdev->last_tx_jiffies + FM_DRV_TX_TIMEOUT)) { fmerr("TX timeout occurred\n"); atomic_set(&fmdev->tx_cnt, 1); }