linux-kernel - Re: [PATCH v5 3/7] mtd: spi-nor: add opcodes for octal Read/Write commands

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: <ac569071-3c69-ebc8-2c18-ea1399210549@microchip.com>
Date:   Mon, 10 Dec 2018 10:44:44 +0000
From:   <Tudor.Ambarus@...rochip.com>
To:     <yogeshnarayan.gaur@....com>, <linux-mtd@...ts.infradead.org>,
        <boris.brezillon@...tlin.com>, <broonie@...nel.org>,
        <marek.vasut@...il.com>, <vigneshr@...com>,
        <linux-spi@...r.kernel.org>, <devicetree@...r.kernel.org>
CC:     <robh@...nel.org>, <mark.rutland@....com>, <shawnguo@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <computersforpeace@...il.com>, <frieder.schrempf@...eet.de>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 3/7] mtd: spi-nor: add opcodes for octal Read/Write
 commands

Hi, Yogesh,

On 12/03/2018 10:39 AM, Yogesh Narayan Gaur wrote:
> - Add opcodes for octal I/O commands
>   * Read  : 1-1-8 and 1-8-8 protocol
>   * Write : 1-1-8 and 1-8-8 protocol
>   * opcodes for 4-byte address mode command
> 
> - Entry of macros in _convert_3to4_xxx function
> 
> - Add flag specifying flash support octal read commands.
> 
> Signed-off-by: Vignesh R <vigneshr@...com>
> Signed-off-by: Yogesh Gaur <yogeshnarayan.gaur@....com>
> ---
> Changes for v5:
> - Modified string 'octo' with 'octal'.
> Changes for v4:
> - None
> Changes for v3:
> - Modified string 'octal' with 'octo'.
> Changes for v2:
> - Incorporated review comments of Boris and Vignesh
> 
>  drivers/mtd/spi-nor/spi-nor.c | 16 ++++++++++++++--
>  include/linux/mtd/spi-nor.h   |  8 ++++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 398d273..7a2176d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -90,6 +90,7 @@ struct flash_info {
>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>  #define USE_CLSR		BIT(14)	/* use CLSR command */
> +#define SPI_NOR_OCTAL_READ	BIT(15)	/* Flash supports Octal Read */
>  
>  	int	(*quad_enable)(struct spi_nor *nor);
>  };
> @@ -209,6 +210,8 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
>  		{ SPINOR_OP_READ_1_2_2,	SPINOR_OP_READ_1_2_2_4B },
>  		{ SPINOR_OP_READ_1_1_4,	SPINOR_OP_READ_1_1_4_4B },
>  		{ SPINOR_OP_READ_1_4_4,	SPINOR_OP_READ_1_4_4_4B },
> +		{ SPINOR_OP_READ_1_1_8,	SPINOR_OP_READ_1_1_8_4B },
> +		{ SPINOR_OP_READ_1_8_8,	SPINOR_OP_READ_1_8_8_4B },
>  
>  		{ SPINOR_OP_READ_1_1_1_DTR,	SPINOR_OP_READ_1_1_1_DTR_4B },
>  		{ SPINOR_OP_READ_1_2_2_DTR,	SPINOR_OP_READ_1_2_2_DTR_4B },
> @@ -225,6 +228,8 @@ static inline u8 spi_nor_convert_3to4_program(u8 opcode)
>  		{ SPINOR_OP_PP,		SPINOR_OP_PP_4B },
>  		{ SPINOR_OP_PP_1_1_4,	SPINOR_OP_PP_1_1_4_4B },
>  		{ SPINOR_OP_PP_1_4_4,	SPINOR_OP_PP_1_4_4_4B },
> +		{ SPINOR_OP_PP_1_1_8,	SPINOR_OP_PP_1_1_8_4B },
> +		{ SPINOR_OP_PP_1_8_8,	SPINOR_OP_PP_1_8_8_4B },
>  	};
>  
>  	return spi_nor_convert_opcode(opcode, spi_nor_3to4_program,
> @@ -2093,7 +2098,7 @@ enum spi_nor_read_command_index {
>  	SNOR_CMD_READ_4_4_4,
>  	SNOR_CMD_READ_1_4_4_DTR,
>  
> -	/* Octo SPI */
> +	/* Octal SPI */
>  	SNOR_CMD_READ_1_1_8,
>  	SNOR_CMD_READ_1_8_8,
>  	SNOR_CMD_READ_8_8_8,
> @@ -2110,7 +2115,7 @@ enum spi_nor_pp_command_index {
>  	SNOR_CMD_PP_1_4_4,
>  	SNOR_CMD_PP_4_4_4,
>  
> -	/* Octo SPI */
> +	/* Octal SPI */
>  	SNOR_CMD_PP_1_1_8,
>  	SNOR_CMD_PP_1_8_8,
>  	SNOR_CMD_PP_8_8_8,
> @@ -3195,6 +3200,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  					  SNOR_PROTO_1_1_4);
>  	}
>  
> +	if (info->flags & SPI_NOR_OCTAL_READ) {
> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> +		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
> +					  0, 8, SPINOR_OP_READ_1_1_8,
> +					  SNOR_PROTO_1_1_8);
> +	}
> +>  	/* Page Program settings. */
>  	params->hwcaps.mask |= SNOR_HWCAPS_PP;
>  	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],

At the end of spi_nor_init_params we check the conditions for parsing the sfdp.
Shouldn't SPI_NOR_OCTAL_READ trigger the sfdp parsing when SPI_NOR_SKIP_SFDP is
not set?

> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 8b1acf6..019f534 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -50,9 +50,13 @@

Can you please s/octo/octal on these as well:
$  grep -ni octo include/linux/mtd/spi-nor.h
471: * As a matter of performances, it is relevant to use Octo SPI protocols first,
492:#define SNOR_HWCPAS_READ_OCTO		GENMASK(14, 11)
501: * Like (Fast) Read capabilities, Octo/Quad SPI protocols are preferred to the
515:#define SNOR_HWCAPS_PP_OCTO	GENMASK(22, 20)

thanks,
ta