[PATCH v2 09/35] mtd: spi-nor: atmel: Use flash late_init() for locking

[Pratyush Yadav]

On 27/07/21 07:51AM, Tudor Ambarus wrote:
> Locking is not described in JESD216 SFDP standard, place the locking
> init in late_init().

You are chaning the order of setting the locking ops here. Earlier, they 
were set before we parsed SFDP. Now they are set after we parse SFDP. 
Though I don't see it making much of a difference.

> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus at microchip.com>
> ---
>  drivers/mtd/spi-nor/atmel.c | 30 +++++++++++-------------------
>  1 file changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c
> index 1fea5cab492c..b937ef734e55 100644
> --- a/drivers/mtd/spi-nor/atmel.c
> +++ b/drivers/mtd/spi-nor/atmel.c
> @@ -48,15 +48,11 @@ static const struct spi_nor_locking_ops atmel_at25fs_locking_ops = {
>  	.is_locked = atmel_at25fs_is_locked,
>  };
>  
> -static void atmel_at25fs_default_init(struct spi_nor *nor)
> +static void atmel_at25fs_late_init(struct spi_nor *nor)
>  {
>  	nor->params->locking_ops = &atmel_at25fs_locking_ops;
>  }
>  
> -static const struct spi_nor_fixups atmel_at25fs_fixups = {
> -	.default_init = atmel_at25fs_default_init,
> -};
> -
>  /**
>   * atmel_set_global_protection - Do a Global Protect or Unprotect command
>   * @nor:	pointer to 'struct spi_nor'
> @@ -146,34 +142,30 @@ static const struct spi_nor_locking_ops atmel_global_protection_ops = {
>  	.is_locked = atmel_is_global_protected,
>  };
>  
> -static void atmel_global_protection_default_init(struct spi_nor *nor)
> +static void atmel_global_protection_late_init(struct spi_nor *nor)
>  {
>  	nor->params->locking_ops = &atmel_global_protection_ops;
>  }
>  
> -static const struct spi_nor_fixups atmel_global_protection_fixups = {
> -	.default_init = atmel_global_protection_default_init,
> -};
> -
>  static const struct flash_info atmel_parts[] = {
>  	/* Atmel -- some are (confusingly) marketed as "DataFlash" */
>  	{ "at25fs010",  INFO(0x1f6601, 0, 32 * 1024,   4, SECT_4K | SPI_NOR_HAS_LOCK)
> -		.fixups = &atmel_at25fs_fixups },
> +		.late_init = atmel_at25fs_late_init },
>  	{ "at25fs040",  INFO(0x1f6604, 0, 64 * 1024,   8, SECT_4K | SPI_NOR_HAS_LOCK)
> -		.fixups = &atmel_at25fs_fixups },
> +		.late_init = atmel_at25fs_late_init },
>  
>  	{ "at25df041a", INFO(0x1f4401, 0, 64 * 1024,   8,
>  			     SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> -			.fixups = &atmel_global_protection_fixups },
> +		.late_init = atmel_global_protection_late_init },

Won't you be better off setting this in the manufacturer late_init()? It 
seems common for most atmel flashes.

Of course, this would cause a problem for atmel flashes that don't have 
this at all, since we would set locking for those as well. But I think 
we can avoid that by checking for SNOR_F_HAS_LOCK in 
spi_nor_register_locking_ops().

>  	{ "at25df321",  INFO(0x1f4700, 0, 64 * 1024,  64,
>  			     SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> -			.fixups = &atmel_global_protection_fixups },
> +		.late_init = atmel_global_protection_late_init },
>  	{ "at25df321a", INFO(0x1f4701, 0, 64 * 1024,  64,
>  			     SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> -			.fixups = &atmel_global_protection_fixups },
> +		.late_init = atmel_global_protection_late_init },
>  	{ "at25df641",  INFO(0x1f4800, 0, 64 * 1024, 128,
>  			     SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> -			.fixups = &atmel_global_protection_fixups },
> +		.late_init = atmel_global_protection_late_init },
>  
>  	{ "at25sl321",	INFO(0x1f4216, 0, 64 * 1024, 64,
>  			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> @@ -181,13 +173,13 @@ static const struct flash_info atmel_parts[] = {
>  	{ "at26f004",   INFO(0x1f0400, 0, 64 * 1024,  8, SECT_4K) },
>  	{ "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16,
>  			     SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> -			.fixups = &atmel_global_protection_fixups },
> +		.late_init = atmel_global_protection_late_init },
>  	{ "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32,
>  			     SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> -			.fixups = &atmel_global_protection_fixups },
> +		.late_init = atmel_global_protection_late_init },
>  	{ "at26df321",  INFO(0x1f4700, 0, 64 * 1024, 64,
>  			     SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> -			.fixups = &atmel_global_protection_fixups },
> +		.late_init = atmel_global_protection_late_init },
>  
>  	{ "at45db081d", INFO(0x1f2500, 0, 64 * 1024, 16, SECT_4K) },
>  };
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.