Re: [PATCH v3 12/17] mtd: spinand: Add mtd_suspend() to disable Octal DTR mode at suspend

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Apurva Nandan <a-nandan@ti.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Mark Brown <broonie@kernel.org>,
	Patrice Chotard <patrice.chotard@foss.st.com>,
	Christophe Kerello <christophe.kerello@foss.st.com>,
	Daniel Palmer <daniel@0x0f.com>,
	Alexander Lobakin <alobakin@pm.me>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<linux-spi@vger.kernel.org>, <p.yadav@ti.com>
Subject: Re: [PATCH v3 12/17] mtd: spinand: Add mtd_suspend() to disable Octal DTR mode at suspend
Date: Mon, 3 Jan 2022 11:17:25 +0100	[thread overview]
Message-ID: <20220103111725.3e3ce291@collabora.com> (raw)
In-Reply-To: <20220101074250.14443-13-a-nandan@ti.com>

On Sat, 1 Jan 2022 13:12:45 +0530
Apurva Nandan <a-nandan@ti.com> wrote:

> The flash might or might not have gone a power-down during sleep.
> Hence, its SPI IO mode is unpredictable for the core at the time of
> resume. To ensure proper reinitialization during resume, disable the
> Octal DTR SPI IO mode and bring the flash to 1S-1S-1S mode when
> performing suspend using mtd_suspend().
> 
> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
> ---
>  drivers/mtd/nand/spi/core.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 2fd08085db6f..9d1c72634e5a 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -1307,6 +1307,16 @@ static void spinand_mtd_resume(struct mtd_info *mtd)
>  	spinand_ecc_enable(spinand, false);
>  }
>  
> +static int spinand_mtd_suspend(struct mtd_info *mtd)
> +{
> +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> +
> +	if (spinand->ctrl_ops->protocol == SPINAND_8D_8D_8D)
> +		return spinand_init_octal_dtr_disable(spinand);

Ok, so you're calling it here. I'd re-order commmits so that DTR
support is added after the suspend hook is defined, even if this hook
does nothing at first. This way you don't have an unused warning in the
previous commit, and most importantly, you don't enter 8DTR before
you're sure things will be set back to 1S when entering suspend.

> +
> +	return 0;
> +}
> +
>  static int spinand_init(struct spinand_device *spinand)
>  {
>  	struct device *dev = &spinand->spimem->spi->dev;
> @@ -1381,6 +1391,7 @@ static int spinand_init(struct spinand_device *spinand)
>  	mtd->_erase = spinand_mtd_erase;
>  	mtd->_max_bad_blocks = nanddev_mtd_max_bad_blocks;
>  	mtd->_resume = spinand_mtd_resume;
> +	mtd->_suspend = spinand_mtd_suspend;
>  
>  	if (nand->ecc.engine) {
>  		ret = mtd_ooblayout_count_freebytes(mtd);