[PATCH 1/2] mtd: spi-nor: check 4-byte address support when parsing 4bait

[Pratyush Yadav]

Hi Yicong,

On 27/01/21 05:40PM, Yicong Yang wrote:
> The spi-nor core will convert the address mode to 4-btye,
> without checking whether 4-byte address is supported or not.
> For example, the 16M s25fs128s1 can work under both 3-byte
> and 4-byte address and provides a 4bait table. The spi-nor
> will drive the flash under 4-byte address mode after parsing
> the 4bait and will cause it unusable on platforms doesn't
> support 4-byte.

Another problem caused by 4BAIT parser prematurely selecting the address 
width. See [0].
 
Let's fix this 4BAIT problem once and for all. Instead of setting 
nor->addr_width to 4 in spi_nor_parse_4bait(), just set SNOR_F_HAS_4BAIT 
(which is already being done). Then in spi_nor_default_setup(), use this 
information when negotiating the read/write/program commands with the 
controller to determine the correct address width to use.

This refactor is easier said than done. We don't associate address width 
information with a command. Just protocol, opcode, and dummy cycles 
(only for read commands). A new mechanism needs to be added where we 
associate a set of supported addresses with a command and then the 
command negotiation can use all this information to arrive at the 
optimal set of commands.

With this in mind, it would be great if you can come up with a patch to 
add such a mechanism. But I would also be OK with this fix with the 
condition that it is clearly marked as a temporary fix, and mentions 
what should ideally be done.

> Add checking of 4-byte address support when parsing the 4bait
> table, stop converting the address mode if it's not supported.
> 
> Signed-off-by: Yicong Yang <yangyicong at hisilicon.com>
> ---
>  drivers/mtd/spi-nor/sfdp.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 6ee7719..fdafc9b 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -940,6 +940,27 @@ static int spi_nor_parse_smpt(struct spi_nor *nor,
>  	return ret;
>  }
>  
> +static int spi_nor_spimem_check_4byte_addr(struct spi_nor *nor,
> +					   const struct spi_nor_read_command *read)
> +{
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(read->opcode, 1),
> +					  SPI_MEM_OP_ADDR(4, 0, 1),
> +					  SPI_MEM_OP_DUMMY(0, 1),
> +					  SPI_MEM_OP_DATA_IN(0, NULL, 1));

Set buswidths to 0 here...

> +
> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(read->proto);
> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(read->proto);
> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(read->proto);
> +	op.dummy.buswidth = op.addr.buswidth;

... and use spi_nor_spimem_setup_op() to do this. It will also take care 
of setting up DTR ops.

> +	op.dummy.nbytes = (read->num_mode_clocks + read->num_wait_states) *
> +			  op.dummy.buswidth / 8;
> +
> +	if (!spi_mem_supports_op(nor->spimem, &op))
> +		return -EOPNOTSUPP;
> +
> +	return 0;
> +}
> +
>  /**
>   * spi_nor_parse_4bait() - parse the 4-Byte Address Instruction Table
>   * @nor:		pointer to a 'struct spi_nor'.
> @@ -1061,6 +1082,33 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
>  		goto out;
>  
>  	/*
> +	 * Check whether the 4-byte address is supported before converting
> +	 * the instruction set to 4-byte.
> +	 */
> +	if (nor->spimem) {
> +		bool support = false;
> +
> +		for (i = 0; i < SNOR_CMD_READ_MAX; i++) {
> +			struct spi_nor_read_command read_cmd;
> +
> +			memcpy(&read_cmd, &params->reads[i], sizeof(read_cmd));
> +
> +			read_cmd.opcode = spi_nor_convert_3to4_read(read_cmd.opcode);
> +			if (!spi_nor_spimem_check_4byte_addr(nor, &read_cmd)) {
> +				support = true;
> +				break;
> +			}
> +		}
> +
> +		/*
> +		 * No supported 4-byte instruction is found, stop parsing the
> +		 * 4bait table.
> +		 */
> +		if (!support)
> +			goto out;
> +	}
> +
> +	/*
>  	 * Discard all operations from the 4-byte instruction set which are
>  	 * not supported by this memory.
>  	 */

[0] https://lore.kernel.org/linux-mtd/20201212115817.5122-1-vigneshr@ti.com/

-- 
Regards,
Pratyush Yadav
Texas Instruments India