[PATCH v2 26/35] mtd: spi-nor: core: Introduce spi_nor_init_default_params()

[Pratyush Yadav]

On 27/07/21 07:52AM, Tudor Ambarus wrote:
> Called for all flashes, regardless if they define SFDP tables or not.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus at microchip.com>
> ---
>  drivers/mtd/spi-nor/core.c | 92 +++++++++++++++++++++-----------------
>  1 file changed, 52 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index b3a01d7d6f0b..9193317f897d 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2543,6 +2543,56 @@ static int spi_nor_setup(struct spi_nor *nor,
>  	return spi_nor_set_addr_width(nor);
>  }
>  
> +/**
> + * spi_nor_init_default_params() - Default initialization of flash parameters
> + * and settings. Done for all flashes, regardless is they define SFDP tables
> + * or not.
> + * @nor:	pointer to a 'struct spi_nor'.
> + */
> +static void spi_nor_init_default_params(struct spi_nor *nor)
> +{
> +	struct spi_nor_flash_parameter *params = nor->params;
> +	const struct flash_info *info = nor->info;
> +	struct device_node *np = spi_nor_get_flash_node(nor);
> +
> +	params->quad_enable = spi_nor_sr2_bit1_quad_enable;
> +	params->set_4byte_addr_mode = spansion_set_4byte_addr_mode;
> +	params->setup = spi_nor_default_setup;
> +	params->otp.org = &info->otp_org;
> +
> +	/* Default to 16-bit Write Status (01h) Command */
> +	nor->flags |= SNOR_F_HAS_16BIT_SR;
> +
> +	/* Set SPI NOR sizes. */
> +	params->writesize = 1;
> +	params->size = (u64)info->sector_size * info->n_sectors;
> +	params->page_size = info->page_size;

I think these two lines should go in spi_nor_info_init_params() since 
you are using the nor info to initialize these parameters. Otherwise, 
what even is the difference between these two functions?

> +
> +	if (!(info->flags & SPI_NOR_NO_FR)) {
> +		/* Default to Fast Read for DT and non-DT platform devices. */
> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> +
> +		/* Mask out Fast Read if not requested at DT instantiation. */
> +		if (np && !of_property_read_bool(np, "m25p,fast-read"))
> +			params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> +	}
> +
> +	/* (Fast) Read settings. */
> +	params->hwcaps.mask |= SNOR_HWCAPS_READ;
> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
> +				  0, 0, SPINOR_OP_READ,
> +				  SNOR_PROTO_1_1_1);
> +
> +	if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
> +		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
> +					  0, 8, SPINOR_OP_READ_FAST,
> +					  SNOR_PROTO_1_1_1);
> +	/* Page Program settings. */
> +	params->hwcaps.mask |= SNOR_HWCAPS_PP;
> +	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
> +				SPINOR_OP_PP, SNOR_PROTO_1_1_1);
> +}
> +
>  /**
>   * spi_nor_manufacturer_init_params() - Initialize the flash's parameters and
>   * settings based on MFR register and ->default_init() hook.
> @@ -2609,43 +2659,8 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
>  	struct spi_nor_flash_parameter *params = nor->params;
>  	struct spi_nor_erase_map *map = &params->erase_map;
>  	const struct flash_info *info = nor->info;
> -	struct device_node *np = spi_nor_get_flash_node(nor);
>  	u8 i, erase_mask;
>  
> -	/* Initialize default flash parameters and settings. */
> -	params->quad_enable = spi_nor_sr2_bit1_quad_enable;
> -	params->set_4byte_addr_mode = spansion_set_4byte_addr_mode;
> -	params->setup = spi_nor_default_setup;
> -	params->otp.org = &info->otp_org;
> -
> -	/* Default to 16-bit Write Status (01h) Command */
> -	nor->flags |= SNOR_F_HAS_16BIT_SR;
> -
> -	/* Set SPI NOR sizes. */
> -	params->writesize = 1;
> -	params->size = (u64)info->sector_size * info->n_sectors;
> -	params->page_size = info->page_size;
> -
> -	if (!(info->flags & SPI_NOR_NO_FR)) {
> -		/* Default to Fast Read for DT and non-DT platform devices. */
> -		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> -
> -		/* Mask out Fast Read if not requested at DT instantiation. */
> -		if (np && !of_property_read_bool(np, "m25p,fast-read"))
> -			params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> -	}
> -
> -	/* (Fast) Read settings. */
> -	params->hwcaps.mask |= SNOR_HWCAPS_READ;
> -	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
> -				  0, 0, SPINOR_OP_READ,
> -				  SNOR_PROTO_1_1_1);
> -
> -	if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
> -		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
> -					  0, 8, SPINOR_OP_READ_FAST,
> -					  SNOR_PROTO_1_1_1);
> -
>  	if (info->flags & SPI_NOR_DUAL_READ) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
>  		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_2],
> @@ -2674,11 +2689,6 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
>  					  SNOR_PROTO_8_8_8_DTR);
>  	}
>  
> -	/* Page Program settings. */
> -	params->hwcaps.mask |= SNOR_HWCAPS_PP;
> -	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
> -				SPINOR_OP_PP, SNOR_PROTO_1_1_1);
> -
>  	if (info->flags & SPI_NOR_OCTAL_DTR_PP) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
>  		/*
> @@ -2823,6 +2833,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
>  	if (!nor->params)
>  		return -ENOMEM;
>  
> +	spi_nor_init_default_params(nor);
> +
>  	spi_nor_info_init_params(nor);
>  
>  	spi_nor_manufacturer_init_params(nor);

I am neutral towards this patch. I don't think it improves much, but at 
the same time it doesn't make anything worse either.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.