[PATCH v15 3/8] mtd: spi-nor: sfdp: Clarify that nor->read_{opcode, dummy} are uninitialized

[Pratyush Yadav]

On 10/05/22 07:10AM, tkuw584924 at gmail.com wrote:
> From: Tudor Ambarus <tudor.ambarus at microchip.com>
> 
> nor->read_{opcode, dummy} are uninitialized (value zero) at SFDP parsing
> time. Clarify that in the code.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus at microchip.com>
> Tested-By: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> ---
>  drivers/mtd/spi-nor/sfdp.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 61ae8c8c5237..058ce218d2af 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -178,12 +178,10 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)
>  static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr,
>  			     size_t len, void *buf)
>  {
> -	u8 addr_nbytes, read_opcode, read_dummy;
> +	u8 addr_nbytes;
>  	int ret;
>  
> -	read_opcode = nor->read_opcode;
>  	addr_nbytes = nor->addr_nbytes;
> -	read_dummy = nor->read_dummy;
>  
>  	nor->read_opcode = SPINOR_OP_RDSFDP;
>  	nor->addr_nbytes = 3;
> @@ -191,9 +189,10 @@ static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr,
>  
>  	ret = spi_nor_read_raw(nor, addr, len, buf);
>  
> -	nor->read_opcode = read_opcode;
>  	nor->addr_nbytes = addr_nbytes;
> -	nor->read_dummy = read_dummy;
> +	/* Restore setup to its uninitialized state. */
> +	nor->read_opcode = 0;
> +	nor->read_dummy = 0;

Hmm, I don't like this too much. Why do you need to explicitly set the 
default value here? What's wrong with this function just not caring 
about what is initialized by this point and what is not? It is really 
not its business caring about default values of these parameters. Modify 
them, run the command, and restore them, regardless of default or 
initialized values.

>  
>  	return ret;
>  }
> @@ -690,7 +689,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>  	u32 addr;
>  	int err;
>  	u8 i;
> -	u8 addr_nbytes, read_opcode, read_dummy;
> +	u8 addr_nbytes;
>  	u8 read_data_mask, map_id;
>  
>  	/* Use a kmalloc'ed bounce buffer to guarantee it is DMA-able. */
> @@ -699,8 +698,6 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>  		return ERR_PTR(-ENOMEM);
>  
>  	addr_nbytes = nor->addr_nbytes;
> -	read_dummy = nor->read_dummy;
> -	read_opcode = nor->read_opcode;
>  
>  	map_id = 0;
>  	/* Determine if there are any optional Detection Command Descriptors */
> @@ -757,8 +754,9 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>  out:
>  	kfree(buf);
>  	nor->addr_nbytes = addr_nbytes;
> -	nor->read_dummy = read_dummy;
> -	nor->read_opcode = read_opcode;
> +	/* Restore setup to its uninitialized state. */
> +	nor->read_dummy = 0;
> +	nor->read_opcode = 0;
>  	return ret;
>  }
>  
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.