linux-kernel - Re: [PATCH] mtd: gpmi-nand: do not fail setting ONFI timing mode if available

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]

Message-ID: <20170713221520.268bc554@bbrezillon>
Date:   Thu, 13 Jul 2017 22:15:20 +0200
From:   Boris Brezillon <boris.brezillon@...e-electrons.com>
To:     Miquel Raynal <miquel.raynal@...e-electrons.com>
Cc:     han.xu@....com, richard@....at, linux-mtd@...ts.infradead.org,
        dwmw2@...radead.org, computersforpeace@...il.com,
        marek.vasut@...il.com, cyrille.pitchen@...ev4u.fr,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: gpmi-nand: do not fail setting ONFI timing mode if
 available

Hi Miquel,

Le Thu, 13 Jul 2017 21:20:30 +0200,
Miquel Raynal <miquel.raynal@...e-electrons.com> a écrit :

> GPMI NFC driver fails to apply timing mode if the ->onfi_get_features()
> does not return the mode that was previously applied.
> 
> We can assume that a nand chip supports a timing as long as it is
> read from the ONFI parameter page. Reading back a different mode than
> the one previously applied does not mean the mode is unsupported but
> that the nand chip does not implement the ONFI feature because it
> probably does not need to.
> 
> The output of ->onfi_get_feature() is irrelevant so delete it.

Having the NAND part that is not supporting the get/set(timing_mode)
feature explicitly mentioned in the commit message would help reviewers
understand why this patch is needed.

Also mention that, even though the SET/GET_FEATURES(timing_mode) is
marked as required in the ONFI spec, this Macronix chip does not
support it which could be considered as a bug.

Regards,

Boris

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@...e-electrons.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index 141bd70a49c2..4d137145439c 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -939,13 +939,6 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
>  	if (ret)
>  		goto err_out;
>  
> -	/* [2] send GET FEATURE command to double-check the timing mode */
> -	memset(feature, 0, ONFI_SUBFEATURE_PARAM_LEN);
> -	ret = nand->onfi_get_features(mtd, nand,
> -				ONFI_FEATURE_ADDR_TIMING_MODE, feature);
> -	if (ret || feature[0] != mode)
> -		goto err_out;
> -
>  	nand->select_chip(mtd, -1);
>  
>  	/* [3] set the main IO clock, 100MHz for mode 5, 80MHz for mode 4. */