[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 = ¯onix_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