[PATCH v3 3/4] spi-nor: Adapt soft reset to XTX25F32B in Rock Pi 4 rev 1.4

Kever Yang kever.yang at rock-chips.com
Thu Sep 1 14:17:17 CEST 2022


Hi Jagan,

     This patchset goes into my todo list, but I'm not very good at SPI 
module, how do you think about this patchset?

     I will happy to merge this if there is no regression for other SPI 
NOR device.


Thanks,

  - Kever

On 2022/7/20 23:42, Xavier Drudis Ferran wrote:
> XTX25F32B does not use octal mode and accepts soft reset, despite its
> SFDP tables. Soft reset at U-Boot exit seems to be required to write
> to /dev/mtd0 from flashrom in linux. Soft reset at U-Boot start seems
> to help booting from SPI (at least with the dts properties I'm using).
>
> The first soft reset is done before the flash id is read and the
> fixups can be called, and v1 of this patch would now fail in Rock Pi 4
> because of unsupported operation if tried with
> SNOR_PROTO_8_8_8_DTR. This was since commit 5752d6ae8daa ("spi:
> spi-mem: add spi_mem_dtr_supports_op()") by Pratyush Yadav.
>
> So try a few modes until SNOR_PROTO_1_1_1 works and then remember the
> reset_proto.  This tries to be useful for other boards, but I still
> don't know any other that needs it and what would work there.
>
> Changed since v2:
>
>     - none (rebase)
>
> Changed since v1:
>
>     - Generalization. First soft reset with SNOR_PROTO_8_8_8_DTR stopped
>       working since the improvement by Pratyush Yadav.
>       Instead of trying 8_8_8_DTR always at first reset, and force
>       1_1_1 once XTX25F32B is detected (so, for last reset) do
>       a generic loop for any chip trying protocols until one is
>       supported and starting by the 8_8_8_DTR protocol (the one previously
>       used). This leverages the fixed supports_op() to take into account
>       controller and device.
>
> fixups can be called, and it would now fail in Rock Pi 4 because of
> unsupported operation if tried with SNOR_PROTO_8_8_8_DTR. So try a few
> modes until SNOR_PROTO_1_1_1 works and then remember the reset_proto.
> This tries to be useful for other boards, but I still don't know any
> other that needs it and what would work there.
>
> Signed-off-by: Xavier Drudis Ferran <xdrudis at tinet.cat>
>
> Cc: Jagan Teki <jagan at amarulasolutions.com>
> Cc: Vignesh R <vigneshr at ti.com>
>
> ---
>   drivers/mtd/spi/spi-nor-core.c | 98 ++++++++++++++++++++++++++++++----
>   include/linux/mtd/spi-nor.h    |  5 ++
>   2 files changed, 92 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 8a226a7af5..17f0cb51fb 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -3575,6 +3575,18 @@ static struct spi_nor_fixups mt35xu512aba_fixups = {
>   };
>   #endif /* CONFIG_SPI_FLASH_MT35XU */
>   
> +#ifdef CONFIG_SPI_FLASH_XTX
> +static void xtx25f32b_post_sfdp_fixup(struct spi_nor *nor,
> +				      struct spi_nor_flash_parameter *params)
> +{
> +	nor->flags |= SNOR_F_SOFT_RESET;
> +}
> +
> +static struct spi_nor_fixups xtx25f32b_fixups = {
> +	.post_sfdp = xtx25f32b_post_sfdp_fixup,
> +};
> +#endif
> +
>   #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
>   /**
>    * spi_nor_macronix_octal_dtr_enable() - Enable octal DTR on Macronix flashes.
> @@ -3733,6 +3745,53 @@ static int spi_nor_init(struct spi_nor *nor)
>   }
>   
>   #ifdef CONFIG_SPI_FLASH_SOFT_RESET
> +/**
> + * spi_nor_soft_reset_setup_op() - Initial setup of op for soft
> + * reset. Initializes nor->reset_proto iff needed.
> + *
> + * @nor:	the spi_nor structure nor->reset_proto will be used or if
> + *		null, it'll be assigned the used protocol, from a few
> + *		protocols that get tried for device and controller
> + *		support.
> + *
> + * @opcode	operation code for soft reset (or soft reset enable,
> + *		we currently assume they work the same in all systems)
> + *
> + * @op:	pointer to operation struct to set up for reset or reset
> + *		enable.
> + */
> +static void spi_nor_soft_reset_setup_op(struct spi_nor *nor, u16 opcode, struct spi_mem_op *op)
> +{
> +	enum spi_nor_protocol reset_proto;
> +	/**
> +	 *  admitedly arbitrary list, to be improved if there's
> +	 *  new knowledge of what works in different systems
> +	 */
> +	enum spi_nor_protocol reset_proto_candidates[] = {
> +		SNOR_PROTO_8_8_8_DTR, SNOR_PROTO_1_1_1_DTR, SNOR_PROTO_8_8_8,
> +		SNOR_PROTO_4_4_4,  SNOR_PROTO_2_2_2, SNOR_PROTO_1_1_1
> +	};
> +
> +	if (nor->reset_proto) {
> +		*op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 0),
> +						    SPI_MEM_OP_NO_DUMMY,
> +						    SPI_MEM_OP_NO_ADDR,
> +						    SPI_MEM_OP_NO_DATA);
> +		spi_nor_setup_op(nor, op, nor->reset_proto);
> +	} else {
> +		for (int i = 0; i < ARRAY_SIZE(reset_proto_candidates) && !nor->reset_proto ; i++) {
> +			reset_proto = reset_proto_candidates[i];
> +			*op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 0),
> +							    SPI_MEM_OP_NO_DUMMY,
> +							    SPI_MEM_OP_NO_ADDR,
> +							    SPI_MEM_OP_NO_DATA);
> +			spi_nor_setup_op(nor, op, reset_proto);
> +			if (spi_mem_supports_op(nor->spi, op))
> +				nor->reset_proto = reset_proto;
> +		}
> +	}
> +}
> +
>   /**
>    * spi_nor_soft_reset() - perform the JEDEC Software Reset sequence
>    * @nor:	the spi_nor structure
> @@ -3756,11 +3815,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
>   #endif /* SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT */
>   	}
>   
> -	op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0),
> -			SPI_MEM_OP_NO_DUMMY,
> -			SPI_MEM_OP_NO_ADDR,
> -			SPI_MEM_OP_NO_DATA);
> -	spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
> +	spi_nor_soft_reset_setup_op(nor, SPINOR_OP_SRSTEN, &op);
>   	ret = spi_mem_exec_op(nor->spi, &op);
>   	if (ret) {
>   		dev_warn(nor->dev, "Software reset enable failed: %d\n", ret);
> @@ -3771,7 +3826,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
>   			SPI_MEM_OP_NO_DUMMY,
>   			SPI_MEM_OP_NO_ADDR,
>   			SPI_MEM_OP_NO_DATA);
> -	spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
> +	spi_nor_setup_op(nor, &op, nor->reset_proto);
>   	ret = spi_mem_exec_op(nor->spi, &op);
>   	if (ret) {
>   		dev_warn(nor->dev, "Software reset failed: %d\n", ret);
> @@ -3789,18 +3844,34 @@ out:
>   	nor->cmd_ext_type = ext;
>   	return ret;
>   }
> -#endif /* CONFIG_SPI_FLASH_SOFT_RESET */
> +
> +static bool flash_supports_proto(const struct flash_info *flash,  enum spi_nor_protocol proto)
> +{
> +	switch (spi_nor_get_protocol_data_nbits(proto)) {
> +	case 8:
> +		return flash->flags & (spi_nor_protocol_is_dtr(proto)
> +				       ? SPI_NOR_OCTAL_DTR_READ
> +				       : SPI_NOR_OCTAL_READ);
> +	case 4:	return flash->flags & SPI_NOR_QUAD_READ;
> +	case 2: return flash->flags & SPI_NOR_DUAL_READ;
> +	default:
> +		return 1;
> +	}
> +}
>   
>   int spi_nor_remove(struct spi_nor *nor)
>   {
> -#ifdef CONFIG_SPI_FLASH_SOFT_RESET
> -	if (nor->info->flags & SPI_NOR_OCTAL_DTR_READ &&
> +	if (flash_supports_proto(nor->info, nor->reset_proto) &&
>   	    nor->flags & SNOR_F_SOFT_RESET)
>   		return spi_nor_soft_reset(nor);
> -#endif
> -
>   	return 0;
>   }
> +#else /* !CONFIG_SPI_FLASH_SOFT_RESET */
> +int spi_nor_remove(struct spi_nor *nor)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_SPI_FLASH_SOFT_RESET */
>   
>   void spi_nor_set_fixups(struct spi_nor *nor)
>   {
> @@ -3835,6 +3906,11 @@ void spi_nor_set_fixups(struct spi_nor *nor)
>   #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
>   	nor->fixups = &macronix_octal_fixups;
>   #endif /* SPI_FLASH_MACRONIX */
> +
> +#ifdef CONFIG_SPI_FLASH_XTX
> +	if (!strcmp(nor->info->name, "xt25f32b"))
> +		nor->fixups = &xtx25f32b_fixups;
> +#endif
>   }
>   
>   int spi_nor_scan(struct spi_nor *nor)
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 2595bad9df..4fc81e7b8d 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -501,6 +501,8 @@ struct spi_flash {
>    * @read_proto:		the SPI protocol for read operations
>    * @write_proto:	the SPI protocol for write operations
>    * @reg_proto		the SPI protocol for read_reg/write_reg/erase operations
> + * @reset_proto:	The SPI protocol for soft reset to return to state expected
> + *			by OS before running the OS (at remove time) or at sf probe
>    * @cmd_buf:		used by the write_reg
>    * @cmd_ext_type:	the command opcode extension for DTR mode.
>    * @fixups:		flash-specific fixup hooks.
> @@ -546,6 +548,9 @@ struct spi_nor {
>   	enum spi_nor_protocol	read_proto;
>   	enum spi_nor_protocol	write_proto;
>   	enum spi_nor_protocol	reg_proto;
> +#ifdef CONFIG_SPI_FLASH_SOFT_RESET
> +	enum spi_nor_protocol	reset_proto;
> +#endif
>   	bool			sst_write_second;
>   	u32			flags;
>   	u8			cmd_buf[SPI_NOR_MAX_CMD_SIZE];


More information about the U-Boot mailing list