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

Michael Walle
2022-05-12T21:33:30Z

Am 2022-05-10 00:10, schrieb tkuw584924@gmail.com:
> From: Tudor Ambarus <tudor.ambarus@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@microchip.com>
> Tested-By: Takahiro Kuwano <Takahiro.Kuwano@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;
>
> 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. */

Who is modifying this? FWIW, I don't like this because it's now
different than all the other occurrences of this pattern.
Ultimately, I'd like to get rid of all these

saved_params = params;
params = new_params;
spi_nor_call()
params = saved_params;

And instead use something like

__spi_nor_call(non_default_params);

-michael