[PATCH v3 3/6] mtd: spi-nor: spansion: Add support for Read/Write Any Register

[Pratyush Yadav]

On 12/03/21 06:44PM, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> 
> Some of Spansion/Cypress chips support Read/Write Any Register commands.
> These commands are mainly used to write volatile registers and access to
> the registers in second and subsequent die for multi-die package parts.
> 
> The Read Any Register instruction (65h) is followed by register address
> and dummy cycles, then the selected register byte is returned.
> 
> The Write Any Register instruction (71h) is followed by register address
> and register byte to write.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> ---
> Changes in v3:
>   - Cleanup implementation
>   
>  drivers/mtd/spi-nor/spansion.c | 102 +++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index b0c5521c1e27..1bce95cb7896 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -19,6 +19,108 @@
>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
>  #define SPINOR_OP_CYPRESS_RD_FAST		0xee
>  
> +/**
> + * spansion_read_any_reg() - Read Any Register.
> + * @nor:	pointer to a 'struct spi_nor'
> + * @reg_addr:	register address
> + * @reg_dummy:	number of dummy cycles for register read
> + * @reg_val:	pointer to a buffer where the register value is copied into
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
> +				 u8 reg_dummy, u8 *reg_val)
> +{
> +	ssize_t ret;
> +
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_ANY_REG, 0),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, reg_addr, 0),
> +				   SPI_MEM_OP_DUMMY((reg_dummy/8), 0),

This is only correct when dummy buswidth is 1. It will lead to some very 
nasty and hard-to-catch bugs when someone uses it for a flash that has a 
different dummy buswidth. See how dummy nbytes are calculated in 
spi_nor_spimem_read_data().

> +				   SPI_MEM_OP_DATA_IN(1, reg_val, 0));
> +
> +		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		enum spi_nor_protocol proto = nor->read_proto;
> +		u8 opcode = nor->read_opcode;
> +		u8 dummy = nor->read_dummy;
> +
> +		nor->read_opcode = SPINOR_OP_RD_ANY_REG;
> +		nor->read_dummy = reg_dummy;
> +		nor->read_proto = nor->reg_proto;
> +
> +		ret = nor->controller_ops->read(nor, reg_addr, 1, reg_val);
> +
> +		nor->read_opcode = opcode;
> +		nor->read_dummy = dummy;
> +		nor->read_proto = proto;
> +
> +		if (ret == 1)
> +			return ret;
> +		if (ret != 1)
> +			return -EIO;
> +
> +		ret = 0;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * spansion_write_any_reg() - Write Any Register.
> + * @nor:	pointer to a 'struct spi_nor'
> + * @reg_addr:	register address
> + * @reg_val:	register value to be written
> + *
> + * Volatile register write will be effective immediately after the operation so
> + * this function does not poll the status.

The same opcode can be used to write to non-volatile registers as well, 
you just need to change the address passed. It is not recommended to 
write to non-volatile registers in SPI NOR though. Maybe add "(should be 
a volatile register)" after "register address" above?

As for not polling the status, can a volatile register write cause a 
program error? If so, you should check the status to know if any error 
bits were set.

> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr, u8 reg_val)
> +{
> +	ssize_t ret;
> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 0),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, reg_addr, 0),
> +				   SPI_MEM_OP_NO_DUMMY,
> +				   SPI_MEM_OP_DATA_OUT(1, &reg_val, 0));
> +
> +		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		enum spi_nor_protocol proto = nor->write_proto;
> +		u8 opcode = nor->program_opcode;
> +
> +		nor->program_opcode = SPINOR_OP_WR_ANY_REG;
> +		nor->write_proto = nor->reg_proto;
> +
> +		ret = nor->controller_ops->write(nor, reg_addr, 1, &reg_val);
> +
> +		nor->program_opcode = opcode;
> +		nor->write_proto = proto;
> +
> +		if (ret < 0)
> +			return ret;
> +		if (ret != 1)
> +			return -EIO;
> +
> +		ret = 0;
> +	}
> +
> +	return ret;
> +}
> +
>  /**
>   * spi_nor_cypress_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
>   * @nor:		pointer to a 'struct spi_nor'
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.