linux-kernel - Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 8 Aug 2018 11:48:32 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Kurt Kanzenbach <kurt@...utronix.de>
Cc:     Boris Brezillon <boris.brezillon@...tlin.com>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Marek Vasut <marek.vasut@...il.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Gregory CLEMENT <gregory.clement@...tlin.com>,
        Jane Wan <Jane.Wan@...ia.com>,
        Jagdish Gediya <jagdish.gediya@....com>,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl
 versions

Hi Kurt,

Subject prefix should be "mtd: rawnand: fsl_ifc:".

Kurt Kanzenbach <kurt@...utronix.de> wrote on Mon,  6 Aug 2018 11:21:37
+0200:

> Newer versions of the IFC controller use a different method of initializing the
> internal SRAM: Instead of reading from flash, a bit in the NAND configuration
> register has to be set in order to trigger the self-initializing process.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@...utronix.de>
> ---
>  drivers/mtd/nand/raw/fsl_ifc_nand.c | 18 ++++++++++++++++++
>  include/linux/fsl_ifc.h             |  2 ++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> index e4f5792dc589..384d5e12b05c 100644
> --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> @@ -30,6 +30,7 @@
>  #include <linux/mtd/partitions.h>
>  #include <linux/mtd/nand_ecc.h>
>  #include <linux/fsl_ifc.h>
> +#include <linux/iopoll.h>
>  
>  #define ERR_BYTE		0xFF /* Value returned for read
>  					bytes when read failed	*/
> @@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
>  	uint32_t csor = 0, csor_8k = 0, csor_ext = 0;
>  	uint32_t cs = priv->bank;
>  
> +	if (ctrl->version > FSL_IFC_VERSION_1_1_0) {

This is redundant and fsl_ifc_sram_init() is called only if
"ctrl->version > FSL_FC_VERSION_1_1_0".

So this means this function has never worked?

If this is the case, there should be at least a Fixes: tag.

Maybe it would be cleaner to always call fsl_ifc_sram_init() from the
probe(), and just exit with a "return 0" here if the version is old?
(I'll let you choose the way you prefer).

> +		u32 ncfgr, status;
> +		int ret;
> +
> +		/* Trigger auto initialization */
> +		ncfgr = ifc_in32(&ifc_runtime->ifc_nand.ncfgr);
> +		ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN, &ifc_runtime->ifc_nand.ncfgr);
> +
> +		/* Wait until done */
> +		ret = readx_poll_timeout(ifc_in32, &ifc_runtime->ifc_nand.ncfgr,
> +					 status, !(status & IFC_NAND_NCFGR_SRAM_INIT_EN),
> +					 10, 1000);

Nit: I always prefer when delays/timeouts are defined (and may be
reused).

> +		if (ret)
> +			dev_err(priv->dev, "Failed to initialize SRAM!\n");

Space

> +		return ret;
> +	}
> +
>  	/* Save CSOR and CSOR_ext */
>  	csor = ifc_in32(&ifc_global->csor_cs[cs].csor);
>  	csor_ext = ifc_in32(&ifc_global->csor_cs[cs].csor_ext);
> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
> index 3fdfede2f0f3..5f343b796ad9 100644
> --- a/include/linux/fsl_ifc.h
> +++ b/include/linux/fsl_ifc.h
> @@ -274,6 +274,8 @@
>   */
>  /* Auto Boot Mode */
>  #define IFC_NAND_NCFGR_BOOT		0x80000000
> +/* SRAM Initialization */
> +#define IFC_NAND_NCFGR_SRAM_INIT_EN	0x20000000
>  /* Addressing Mode-ROW0+n/COL0 */
>  #define IFC_NAND_NCFGR_ADDR_MODE_RC0	0x00000000
>  /* Addressing Mode-ROW0+n/COL0+n */


Thanks,
Miquèl