Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <1644481960-15049-1-git-send-email-wangqing@vivo.com>
References: <1644481960-15049-1-git-send-email-wangqing@vivo.com>
Subject: Re: [PATCH] media: wl128x: use time_after_eq() instead of jiffies judgment
From:   Kieran Bingham <kieran.bingham@ideasonboard.com>
Cc:     Wang Qing <wangqing@vivo.com>
To:     Mauro Carvalho Chehab <mchehab@kernel.org>,
        Qing Wang <wangqing@vivo.com>, linux-kernel@vger.kernel.org,
        linux-media@vger.kernel.org
Date:   Thu, 10 Feb 2022 09:38:38 +0000
Message-ID: <164448591856.3354066.6123333934504651771@Monstersaurus>
User-Agent: alot/0.10
Precedence: bulk

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>
>=20
> It is better to use time_xxx() directly instead of jiffies judgment
> for understanding.
>=20
> Signed-off-by: Wang Qing <wangqing@vivo.com>
> ---
>  drivers/media/radio/wl128x/fmdrv_common.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>=20
> diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/ra=
dio/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>
> =20
>  #include "fmdrv.h"
>  #include "fmdrv_v4l2.h"
> @@ -342,7 +343,7 @@ static void send_tasklet(struct tasklet_struct *t)
>                 return;
> =20
>         /* Check, is there any timeout happened to last transmitted packe=
t */
> -       if ((jiffies - fmdev->last_tx_jiffies) > FM_DRV_TX_TIMEOUT) {
> +       if (time_after(jiffies, fmdev->last_tx_jiffies + FM_DRV_TX_TIMEOU=
T)) {

It looks like there are specific macros for working with jiffies too.

Should this be=20
              time_is_after_jiffies(fmdev->last_tx_jiffies + FM_DRV_TX_TIME=
OUT) {

Although that is in fact 2 characters longer ;-S


--
Kieran


>                 fmerr("TX timeout occurred\n");
>                 atomic_set(&fmdev->tx_cnt, 1);
>         }
> --=20
> 2.7.4
>