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

From: Tudor.Ambarus
Date: Fri Dec 21 2018 - 09:44:46 EST



On 12/19/2018 12:12 PM, 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

I verified that the above opcodes are compliant with the MT35X Public datasheet.

> * opcodes for 4-byte address mode command

opcodes compliant with jesd216c.

>
> - Entry of macros in _convert_3to4_xxx function
>
> - Add flag specifying flash support octal read commands.

It would be nicer to explain the need of this flag, similar to what you did in
patch's 4/7 commit message.

>
> Signed-off-by: Vignesh R <vigneshr@xxxxxx>
> Signed-off-by: Yogesh Narayan Gaur <yogeshnarayan.gaur@xxxxxxx>

Looks good:

Reviewed-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>

> ---
> Changes for v6:
> - Correct S-o-b tag with full author name as 'Yogesh Narayan Gaur'.
> 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 | 16 ++++++++++++----
> 2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 6e13bbd..872d707 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -68,7 +68,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,
> @@ -85,7 +85,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,
> @@ -278,6 +278,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 */
>
> /* Part specific fixup hooks. */
> const struct spi_nor_fixups *fixups;
> @@ -398,6 +399,8 @@ static 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 },
> @@ -414,6 +417,8 @@ static 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,
> @@ -3591,6 +3596,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],
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index fa2d89e..2353af8 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -46,9 +46,13 @@
> #define SPINOR_OP_READ_1_2_2 0xbb /* Read data bytes (Dual I/O SPI) */
> #define SPINOR_OP_READ_1_1_4 0x6b /* Read data bytes (Quad Output SPI) */
> #define SPINOR_OP_READ_1_4_4 0xeb /* Read data bytes (Quad I/O SPI) */
> +#define SPINOR_OP_READ_1_1_8 0x8b /* Read data bytes (Octal Output SPI) */
> +#define SPINOR_OP_READ_1_8_8 0xcb /* Read data bytes (Octal I/O SPI) */
> #define SPINOR_OP_PP 0x02 /* Page program (up to 256 bytes) */
> #define SPINOR_OP_PP_1_1_4 0x32 /* Quad page program */
> #define SPINOR_OP_PP_1_4_4 0x38 /* Quad page program */
> +#define SPINOR_OP_PP_1_1_8 0x82 /* Octal page program */
> +#define SPINOR_OP_PP_1_8_8 0xc2 /* Octal page program */
> #define SPINOR_OP_BE_4K 0x20 /* Erase 4KiB block */
> #define SPINOR_OP_BE_4K_PMC 0xd7 /* Erase 4KiB block on PMC chips */
> #define SPINOR_OP_BE_32K 0x52 /* Erase 32KiB block */
> @@ -69,9 +73,13 @@
> #define SPINOR_OP_READ_1_2_2_4B 0xbc /* Read data bytes (Dual I/O SPI) */
> #define SPINOR_OP_READ_1_1_4_4B 0x6c /* Read data bytes (Quad Output SPI) */
> #define SPINOR_OP_READ_1_4_4_4B 0xec /* Read data bytes (Quad I/O SPI) */
> +#define SPINOR_OP_READ_1_1_8_4B 0x7c /* Read data bytes (Octal Output SPI) */
> +#define SPINOR_OP_READ_1_8_8_4B 0xcc /* Read data bytes (Octal I/O SPI) */
> #define SPINOR_OP_PP_4B 0x12 /* Page program (up to 256 bytes) */
> #define SPINOR_OP_PP_1_1_4_4B 0x34 /* Quad page program */
> #define SPINOR_OP_PP_1_4_4_4B 0x3e /* Quad page program */
> +#define SPINOR_OP_PP_1_1_8_4B 0x84 /* Octal page program */
> +#define SPINOR_OP_PP_1_8_8_4B 0x8e /* Octal page program */
> #define SPINOR_OP_BE_4K_4B 0x21 /* Erase 4KiB block */
> #define SPINOR_OP_BE_32K_4B 0x5c /* Erase 32KiB block */
> #define SPINOR_OP_SE_4B 0xdc /* Sector erase (usually 64KiB) */
> @@ -458,7 +466,7 @@ struct spi_nor_hwcaps {
> /*
> *(Fast) Read capabilities.
> * MUST be ordered by priority: the higher bit position, the higher priority.
> - * As a matter of performances, it is relevant to use Octo SPI protocols first,
> + * As a matter of performances, it is relevant to use Octal SPI protocols first,
> * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
> * (Slow) Read.
> */
> @@ -479,7 +487,7 @@ struct spi_nor_hwcaps {
> #define SNOR_HWCAPS_READ_4_4_4 BIT(9)
> #define SNOR_HWCAPS_READ_1_4_4_DTR BIT(10)
>
> -#define SNOR_HWCPAS_READ_OCTO GENMASK(14, 11)
> +#define SNOR_HWCPAS_READ_OCTAL GENMASK(14, 11)
> #define SNOR_HWCAPS_READ_1_1_8 BIT(11)
> #define SNOR_HWCAPS_READ_1_8_8 BIT(12)
> #define SNOR_HWCAPS_READ_8_8_8 BIT(13)
> @@ -488,7 +496,7 @@ struct spi_nor_hwcaps {
> /*
> * Page Program capabilities.
> * MUST be ordered by priority: the higher bit position, the higher priority.
> - * Like (Fast) Read capabilities, Octo/Quad SPI protocols are preferred to the
> + * Like (Fast) Read capabilities, Octal/Quad SPI protocols are preferred to the
> * legacy SPI 1-1-1 protocol.
> * Note that Dual Page Programs are not supported because there is no existing
> * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
> @@ -502,7 +510,7 @@ struct spi_nor_hwcaps {
> #define SNOR_HWCAPS_PP_1_4_4 BIT(18)
> #define SNOR_HWCAPS_PP_4_4_4 BIT(19)
>
> -#define SNOR_HWCAPS_PP_OCTO GENMASK(22, 20)
> +#define SNOR_HWCAPS_PP_OCTAL GENMASK(22, 20)
> #define SNOR_HWCAPS_PP_1_1_8 BIT(20)
> #define SNOR_HWCAPS_PP_1_8_8 BIT(21)
> #define SNOR_HWCAPS_PP_8_8_8 BIT(22)
>