[PATCH v2 4/8] mtd: spi-nor: spansion: switch set_octal_dtr method to use vreg_offset

[Tudor Ambarus]

On 20.07.2023 10:09, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> 
> All the Infineon flashes that currently support octal DTR mode
> define the optional SCCR SFDP table, thus all retrieve vreg_offset.
> Switch all the available octal DTR Infineon flashes to use the
> volatile register offset to set the configuration registers. The goal
> is to have a single pair of methods for both single/multi-chip package
> devices.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> ---
>  drivers/mtd/spi-nor/spansion.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index b99ed088b377..9150840e748a 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -6,6 +6,7 @@
>  
>  #include <linux/bitfield.h>
>  #include <linux/device.h>
> +#include <linux/errno.h>
>  #include <linux/mtd/spi-nor.h>
>  
>  #include "core.h"
> @@ -202,14 +203,18 @@ static int cypress_nor_set_octal_dtr_bits(struct spi_nor *nor, u64 addr)
>  
>  static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
>  {
> +	const struct spi_nor_flash_parameter *params = nor->params;
>  	u8 *buf = nor->bouncebuf;
> +	u64 addr;
>  	int ret;
>  
> -	ret = cypress_nor_set_memlat(nor, SPINOR_REG_CYPRESS_CFR2V);
> +	addr = params->vreg_offset[0] + SPINOR_REG_CYPRESS_CFR2;
> +	ret = cypress_nor_set_memlat(nor, addr);
>  	if (ret)
>  		return ret;
>  
> -	ret = cypress_nor_set_octal_dtr_bits(nor, SPINOR_REG_CYPRESS_CFR5V);
> +	addr = params->vreg_offset[0] + SPINOR_REG_CYPRESS_CFR5;
> +	ret = cypress_nor_set_octal_dtr_bits(nor, addr);
>  	if (ret)
>  		return ret;
>  
> @@ -247,9 +252,11 @@ static int cypress_nor_set_single_spi_bits(struct spi_nor *nor, u64 addr)
>  static int cypress_nor_octal_dtr_dis(struct spi_nor *nor)
>  {
>  	u8 *buf = nor->bouncebuf;
> +	u64 addr;
>  	int ret;
>  
> -	ret = cypress_nor_set_single_spi_bits(nor, SPINOR_REG_CYPRESS_CFR5V);
> +	addr = nor->params->vreg_offset[0] + SPINOR_REG_CYPRESS_CFR5;
> +	ret = cypress_nor_set_single_spi_bits(nor, addr);
>  	if (ret)
>  		return ret;
>  
> @@ -671,6 +678,11 @@ static struct spi_nor_fixups s25hx_t_fixups = {
>   */
>  static int cypress_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
>  {
const struct spi_nor_flash_parameter *params = nor->params;

> +	if (!nor->params->vreg_offset) {

wouldn't hurt to check for params->n_dice as well, even though one
implies the other. In the next patch you have a loop with n-dice being
the max number of elements. If n-dice becomes zero sometime, the loop is
skipped. Thus:

	if (params->n_dice && !nor->params->vreg_offset)

> +		dev_err(nor->dev, "Can not set octal DTR. The volatile register offset could not be retrieved from SFDP\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	return enable ? cypress_nor_octal_dtr_en(nor) :
>  			cypress_nor_octal_dtr_dis(nor);
>  }