All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Martin Devera <devik@eaxlabs.cz>
Cc: Christophe Kerello <christophe.kerello@st.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	linux-kernel@vger.kernel.org,
	Boris Brezillon <boris.brezillon@bootlin.com>,
	linux-mtd@lists.infradead.org, jan.pohanka@merz.cz,
	Brian Norris <computersforpeace@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH] mtd: rawnand: Fix unexpected timeouts in waitrdy
Date: Thu, 9 Jan 2020 16:37:52 +0100	[thread overview]
Message-ID: <20200109163752.621c6248@xps13> (raw)
In-Reply-To: <20191210150319.3125-1-devik@eaxlabs.cz>

Hi Martin,

Martin Devera <devik@eaxlabs.cz> wrote on Tue, 10 Dec 2019 16:03:18
+0100:

> The used way to compute jiffies timeout brokes when
> jiffie difference is 1. Simply add 1 - it has no other
> side effects.
> Fixes STM32MP1 FMC2 NAND controller which sometimes failed
> exactly in this way.
> 
> Signed-off-by: Martin Devera <devik@eaxlabs.cz>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index d527e448ce19..beab3a775cc7 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -721,7 +721,11 @@ int nand_soft_waitrdy(struct nand_chip *chip, unsigned long timeout_ms)
>  	if (ret)
>  		return ret;
>  
> -	timeout_ms = jiffies + msecs_to_jiffies(timeout_ms);
> +	/* +1 below is necessary because if we are now in the last fraction
> +	 * of jiffy and msecs_to_jiffies is 1 then we will wait only that
> +	 * small jiffy fraction - possibly leading to false timeout
> +	 */
> +	timeout_ms = jiffies + msecs_to_jiffies(timeout_ms) + 1;
>  	do {
>  		ret = nand_read_data_op(chip, &status, sizeof(status), true);
>  		if (ret)

I don't really what you are fixing here, I suspect the root cause to be
a wrongly calculated timeout_ms in the calling driver.

It is the responsibility of the caller to use this function with a
relevant timeout_ms parameter. Maybe Christophe can help you here?


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Martin Devera <devik@eaxlabs.cz>
Cc: linux-kernel@vger.kernel.org, jan.pohanka@merz.cz,
	Christophe Kerello <christophe.kerello@st.com>,
	Boris Brezillon <boris.brezillon@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	linux-mtd@lists.infradead.org
Subject: Re: [PATCH] mtd: rawnand: Fix unexpected timeouts in waitrdy
Date: Thu, 9 Jan 2020 16:37:52 +0100	[thread overview]
Message-ID: <20200109163752.621c6248@xps13> (raw)
In-Reply-To: <20191210150319.3125-1-devik@eaxlabs.cz>

Hi Martin,

Martin Devera <devik@eaxlabs.cz> wrote on Tue, 10 Dec 2019 16:03:18
+0100:

> The used way to compute jiffies timeout brokes when
> jiffie difference is 1. Simply add 1 - it has no other
> side effects.
> Fixes STM32MP1 FMC2 NAND controller which sometimes failed
> exactly in this way.
> 
> Signed-off-by: Martin Devera <devik@eaxlabs.cz>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index d527e448ce19..beab3a775cc7 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -721,7 +721,11 @@ int nand_soft_waitrdy(struct nand_chip *chip, unsigned long timeout_ms)
>  	if (ret)
>  		return ret;
>  
> -	timeout_ms = jiffies + msecs_to_jiffies(timeout_ms);
> +	/* +1 below is necessary because if we are now in the last fraction
> +	 * of jiffy and msecs_to_jiffies is 1 then we will wait only that
> +	 * small jiffy fraction - possibly leading to false timeout
> +	 */
> +	timeout_ms = jiffies + msecs_to_jiffies(timeout_ms) + 1;
>  	do {
>  		ret = nand_read_data_op(chip, &status, sizeof(status), true);
>  		if (ret)

I don't really what you are fixing here, I suspect the root cause to be
a wrongly calculated timeout_ms in the calling driver.

It is the responsibility of the caller to use this function with a
relevant timeout_ms parameter. Maybe Christophe can help you here?


Thanks,
Miquèl

  reply	other threads:[~2020-01-09 15:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-10 15:03 [PATCH] mtd: rawnand: Fix unexpected timeouts in waitrdy Martin Devera
2019-12-10 15:03 ` Martin Devera
2020-01-09 15:37 ` Miquel Raynal [this message]
2020-01-09 15:37   ` Miquel Raynal
2020-01-09 16:17   ` Martin DEVERA
2020-01-09 16:17     ` Martin DEVERA
2020-01-09 17:22     ` Miquel Raynal
2020-01-09 17:22       ` Miquel Raynal
     [not found]       ` <0501ddbd-9e0f-62ec-ab57-d092502680d5@eaxlabs.cz>
2020-01-09 18:02         ` Miquel Raynal
2020-01-09 18:04           ` Miquel Raynal
2020-01-16 13:54             ` [PATCH] mtd: rawnand: Ensure nand_soft_waitrdy wait period is enough Martin Devera
2020-03-10 18:34               ` Miquel Raynal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200109163752.621c6248@xps13 \
    --to=miquel.raynal@bootlin.com \
    --cc=boris.brezillon@bootlin.com \
    --cc=christophe.kerello@st.com \
    --cc=computersforpeace@gmail.com \
    --cc=devik@eaxlabs.cz \
    --cc=dwmw2@infradead.org \
    --cc=jan.pohanka@merz.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=richard@nod.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.