[PATCH v3] mtd: spi-nor: sfdp: Get the 1-1-8 and 1-8-8 protocol from SFDP

[Tudor Ambarus]

On 19.12.2023 12:21, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao at mxic.com.tw>
> 
> BFPT 17th DWORD contains the information about 1-1-8
> and 1-8-8, parse BFPT 17 DWORD instruction to determine
> whether flash support 1-1-8 and 1-8-8, and set its dummy
> cycles accordingly.
> 
> Protocols 1-1-8 and 1-8-8 support are not determined by a
> single bit, so that use 8 bits instruction to determine
> whether the protocol support or not.
> 
> Macronix spi-nor flash didn't support 1-8-8 read feature,
> this patch validated 1-1-8 feature only on Xilinx board
> zynq-picozed.
> 
> Signed-off-by: JaimeLiao <jaimeliao at mxic.com.tw>
> Reviewed-by: Michael Walle <mwalle at kernel.org>
> ---
> Changes in v3:
>  - Modify title of patch
>  - Parse 1-8-8 read as well
>  - Modify the typo descriptions in commit log
> ---
>  drivers/mtd/spi-nor/sfdp.c | 29 +++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/sfdp.h |  7 +++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index b3b11dfed789..3f1eba83a7d8 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -446,6 +446,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	u32 dword;
>  	u16 half;
>  	u8 erase_mask;
> +	u8 wait_states, mode_clocks, opcode;
>  
>  	/* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
>  	if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> @@ -631,6 +632,32 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
>  		return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt);
>  
> +	/* Parse 1-1-8 read instruction */
> +	opcode = FIELD_GET(BFPT_DWORD17_RD_1_1_8_CMD, bfpt.dwords[SFDP_DWORD(17)]);
> +	if (opcode) {
> +		mode_clocks = FIELD_GET(BFPT_DWORD17_RD_1_1_8_MODE_CLOCKS,
> +					bfpt.dwords[SFDP_DWORD(17)]);
> +		wait_states = FIELD_GET(BFPT_DWORD17_RD_1_1_8_WAIT_STATES,
> +					bfpt.dwords[SFDP_DWORD(17)]);
> +		nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> +		spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_1_1_8],
> +					  mode_clocks, wait_states, opcode,
> +					  SNOR_PROTO_1_1_8);
> +	}
> +
> +	/* Parse 1-8-8 read instruction */
> +	opcode = FIELD_GET(BFPT_DWORD17_RD_1_8_8_CMD, bfpt.dwords[SFDP_DWORD(17)]);
> +	if (opcode) {
> +		mode_clocks = FIELD_GET(BFPT_DWORD17_RD_1_8_8_MODE_CLOCKS,
> +					bfpt.dwords[SFDP_DWORD(17)]);
> +		wait_states = FIELD_GET(BFPT_DWORD17_RD_1_8_8_WAIT_STATES,
> +					bfpt.dwords[SFDP_DWORD(17)]);
> +		nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
> +		spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_1_8_8],
> +					  mode_clocks, wait_states, opcode,
> +					  SNOR_PROTO_1_8_8);
> +	}

you can use params directly instead of nor->params. And it could have
been written in a generic way, considering the other read settings
parsed from SFDP. I'll get rid of the extra dereference and queue it,
you can address how settings are parsed later on if you care. Or I'll do
it myself if I won't be able to bear it.

> +
>  	/* 8D-8D-8D command extension. */
>  	switch (bfpt.dwords[SFDP_DWORD(18)] & BFPT_DWORD18_CMD_EXT_MASK) {
>  	case BFPT_DWORD18_CMD_EXT_REP:
> @@ -968,6 +995,8 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
>  		{ SNOR_HWCAPS_READ_1_1_1_DTR,	BIT(13) },
>  		{ SNOR_HWCAPS_READ_1_2_2_DTR,	BIT(14) },
>  		{ SNOR_HWCAPS_READ_1_4_4_DTR,	BIT(15) },
> +		{ SNOR_HWCAPS_READ_1_1_8,	BIT(20) },
> +		{ SNOR_HWCAPS_READ_1_8_8,	BIT(21) },
>  	};
>  	static const struct sfdp_4bait programs[] = {
>  		{ SNOR_HWCAPS_PP,		BIT(6) },
> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> index 6eb99e1cdd61..da0fe5aa9bb0 100644
> --- a/drivers/mtd/spi-nor/sfdp.h
> +++ b/drivers/mtd/spi-nor/sfdp.h
> @@ -118,6 +118,13 @@ struct sfdp_bfpt {
>  	(BFPT_DWORD16_EN4B_EN4B | BFPT_DWORD16_EX4B_EX4B)
>  #define BFPT_DWORD16_SWRST_EN_RST		BIT(12)
>  
> +#define BFPT_DWORD17_RD_1_1_8_CMD		GENMASK(31, 24)
> +#define BFPT_DWORD17_RD_1_1_8_MODE_CLOCKS	GENMASK(23, 21)
> +#define BFPT_DWORD17_RD_1_1_8_WAIT_STATES	GENMASK(20, 16)
> +#define BFPT_DWORD17_RD_1_8_8_CMD		GENMASK(15, 8)
> +#define BFPT_DWORD17_RD_1_8_8_MODE_CLOCKS	GENMASK(7, 5)
> +#define BFPT_DWORD17_RD_1_8_8_WAIT_STATES	GENMASK(4, 0)
> +
>  #define BFPT_DWORD18_CMD_EXT_MASK		GENMASK(30, 29)
>  #define BFPT_DWORD18_CMD_EXT_REP		(0x0UL << 29) /* Repeat */
>  #define BFPT_DWORD18_CMD_EXT_INV		(0x1UL << 29) /* Invert */