Re: [PATCH] mtd: spi-nor: Allow post_sfdp hook to return errors

[Tudor Ambarus]

On 05.04.2023 09:07, Tudor Ambarus wrote:
> Multi die flashes like s25hl02gt need to determine the page_size at
> run-time by querying a configuration register for each die. Since the
> number of dice is determined in an optional SFDP table, SCCR MC, the
> page size configuration must be done in the post_sfdp hook. Allow
> post_sfdp to return errors, as reading the configuration register might
> return errors.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>
> ---
>  drivers/mtd/spi-nor/core.h      |  2 +-
>  drivers/mtd/spi-nor/micron-st.c |  4 +++-
>  drivers/mtd/spi-nor/sfdp.c      | 10 ++++++----
>  drivers/mtd/spi-nor/spansion.c  | 12 +++++++++---
>  4 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 8cfa82ed06c7..a9e5e091547d 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -426,7 +426,7 @@ struct spi_nor_fixups {
>  	int (*post_bfpt)(struct spi_nor *nor,
>  			 const struct sfdp_parameter_header *bfpt_header,
>  			 const struct sfdp_bfpt *bfpt);
> -	void (*post_sfdp)(struct spi_nor *nor);
> +	int (*post_sfdp)(struct spi_nor *nor);
>  	void (*late_init)(struct spi_nor *nor);
>  };
>  
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index a6f080112a51..4b919756a205 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -131,7 +131,7 @@ static void mt35xu512aba_default_init(struct spi_nor *nor)
>  	nor->params->octal_dtr_enable = micron_st_nor_octal_dtr_enable;
>  }
>  
> -static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
> +static int mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
>  {
>  	/* Set the Fast Read settings. */
>  	nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
> @@ -149,6 +149,8 @@ static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
>  	 * disable it.
>  	 */
>  	nor->params->quad_enable = NULL;
> +
> +	return 0;
>  }
>  
>  static const struct spi_nor_fixups mt35xu512aba_fixups = {
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 69e47c9778a2..e501686a6dc3 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -1260,14 +1260,16 @@ static int spi_nor_parse_sccr(struct spi_nor *nor,
>   * Used to tweak various flash parameters when information provided by the SFDP
>   * tables are wrong.
>   */
> -static void spi_nor_post_sfdp_fixups(struct spi_nor *nor)
> +static int spi_nor_post_sfdp_fixups(struct spi_nor *nor)
>  {
>  	if (nor->manufacturer && nor->manufacturer->fixups &&
>  	    nor->manufacturer->fixups->post_sfdp)
> -		nor->manufacturer->fixups->post_sfdp(nor);
> +		return nor->manufacturer->fixups->post_sfdp(nor);

oops, this excludes the next call, sending v2
>  
>  	if (nor->info->fixups && nor->info->fixups->post_sfdp)
> -		nor->info->fixups->post_sfdp(nor);
> +		return nor->info->fixups->post_sfdp(nor);
> +
> +	return 0;
>  }
>  
>  /**
> @@ -1477,7 +1479,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
>  		}
>  	}
>  
> -	spi_nor_post_sfdp_fixups(nor);
> +	err = spi_nor_post_sfdp_fixups(nor);
>  exit:
>  	kfree(param_headers);
>  	return err;
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index c937f0ac61de..519fdad79a19 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -370,7 +370,7 @@ s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
>  	return cypress_nor_get_page_size(nor);
>  }
>  
> -static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor)
> +static int s25fs256t_post_sfdp_fixup(struct spi_nor *nor)
>  {
>  	struct spi_nor_flash_parameter *params = nor->params;
>  
> @@ -379,6 +379,8 @@ static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor)
>  	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
>  				SPINOR_OP_PP_1_1_4_4B,
>  				SNOR_PROTO_1_1_4);
> +
> +	return 0;
>  }
>  
>  static void s25fs256t_late_init(struct spi_nor *nor)
> @@ -409,7 +411,7 @@ s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
>  	return cypress_nor_get_page_size(nor);
>  }
>  
> -static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor)
> +static int s25hx_t_post_sfdp_fixup(struct spi_nor *nor)
>  {
>  	struct spi_nor_erase_type *erase_type =
>  					nor->params->erase_map.erase_type;
> @@ -431,6 +433,8 @@ static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor)
>  			break;
>  		}
>  	}
> +
> +	return 0;
>  }
>  
>  static void s25hx_t_late_init(struct spi_nor *nor)
> @@ -463,7 +467,7 @@ static int cypress_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
>  			cypress_nor_octal_dtr_dis(nor);
>  }
>  
> -static void s28hx_t_post_sfdp_fixup(struct spi_nor *nor)
> +static int s28hx_t_post_sfdp_fixup(struct spi_nor *nor)
>  {
>  	/*
>  	 * On older versions of the flash the xSPI Profile 1.0 table has the
> @@ -489,6 +493,8 @@ static void s28hx_t_post_sfdp_fixup(struct spi_nor *nor)
>  	 * actual value for that is 4.
>  	 */
>  	nor->params->rdsr_addr_nbytes = 4;
> +
> +	return 0;
>  }
>  
>  static int s28hx_t_post_bfpt_fixup(struct spi_nor *nor,