Re: [PATCH] drivers:soc:fsl:qbman:qman.c: unsigned long jiffies value.

From: Scott Wood
Date: Sat Apr 29 2017 - 19:33:24 EST


On Sat, 2017-04-29 at 22:43 +0200, Karim Eshapa wrote:
> unsigned long jiffies value sorry for that.

You mean unsigned long msecs?

>
> Signed-off-by: Karim Eshapa <karim.eshapa@xxxxxxxxx>
> ---
> Âdrivers/soc/fsl/qbman/qman.c | 2 +-
> Â1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index e0df4d1..6e1a44a 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -1084,7 +1084,7 @@ static int drain_mr_fqrni(struct qm_portal *p)
> Â Â* entries well before the ring has been fully consumed, so
> Â Â* we're being *really* paranoid here.
> Â Â*/
> - unsigned int udel_time = jiffies_to_usecs(10000);
> + unsigned long udel_time = jiffies_to_usecs(10000);
> Â
> Â usleep_range(udel_time/2, udel_time);
> Â msg = qm_mr_current(p);

If unsigned int isn't big enough, then unsigned long won't be either on 32-
bit. With such a long delay why not use msleep()?

As for the previous patch[1], you're halving the minimum timeout which may not
be correct.

For the NXP people: Is there *really* no better way to handle this than
waiting for so long? Nothing that can be checked to exit the loop early (at
least, you could exit early if there is more work to do so only the final
iteration takes the full timeout)? And why is the desired timeout specified
in jiffies, the duration of which can change based on kernel config and
doesn't reflect anything about the hardware?

-Scott

[1] When fixing a patch you've already posted that hasn't yet been applied,
send a replacement (v2) patch rather than a separate fix.