diff mbox series

[2/2] mtd: spi-nor: core: Add dbg msg for spi_nor_erase_multi_sectors()

Message ID 20210205135253.675793-2-tudor.ambarus@microchip.com
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series [1/2] mtd: spi-nor: core: Advance erase after the erase cmd has been completed | expand

Commit Message

Tudor Ambarus Feb. 5, 2021, 1:52 p.m. UTC
Useful when debugging non-uniform erase.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/core.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Pratyush Yadav Feb. 8, 2021, 11:41 a.m. UTC | #1
On 05/02/21 03:52PM, Tudor Ambarus wrote:
> Useful when debugging non-uniform erase.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index bcaa161bc7db..7401c60b53e6 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1622,6 +1622,9 @@ static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len)
>  			if (ret)
>  				goto destroy_erase_cmd_list;
>  
> +			dev_dbg(nor->dev, "erase_cmd->size = 0x%08x, erase_cmd->opcode = 0x%02x, erase_cmd->count = %d\n",
> +				cmd->size, cmd->opcode, cmd->count);
> +

I don't like the position of this debug message. This prints cmd->count 
_after_ the erase is done but _before_ cmd->count is updated. It might 
end up giving some wrong or misleading information. Can you either move 
it before the start of the erase or after all the bookkeeping is done?

>  			addr += cmd->size;
>  			cmd->count--;
>  		}
> -- 
> 2.25.1
Tudor Ambarus Feb. 8, 2021, 11:59 a.m. UTC | #2
On 2/8/21 1:41 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 05/02/21 03:52PM, Tudor Ambarus wrote:
>> Useful when debugging non-uniform erase.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/mtd/spi-nor/core.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index bcaa161bc7db..7401c60b53e6 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -1622,6 +1622,9 @@ static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len)
>>                       if (ret)
>>                               goto destroy_erase_cmd_list;
>>
>> +                     dev_dbg(nor->dev, "erase_cmd->size = 0x%08x, erase_cmd->opcode = 0x%02x, erase_cmd->count = %d\n",
>> +                             cmd->size, cmd->opcode, cmd->count);
>> +
> 
> I don't like the position of this debug message. This prints cmd->count
> _after_ the erase is done but _before_ cmd->count is updated. It might

oh, yes.

> end up giving some wrong or misleading information. Can you either move
> it before the start of the erase or after all the bookkeeping is done?

Before the start of the erase sounds good, but still inside the while,
so that we catch each executed command. And maybe I'll change dev_dbg
to dev_vdbg. I'll made my mind before v2.

Cheers,
ta
> 
>>                       addr += cmd->size;
>>                       cmd->count--;
>>               }
>> --
>> 2.25.1
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments Inc.
>
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index bcaa161bc7db..7401c60b53e6 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1622,6 +1622,9 @@  static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len)
 			if (ret)
 				goto destroy_erase_cmd_list;
 
+			dev_dbg(nor->dev, "erase_cmd->size = 0x%08x, erase_cmd->opcode = 0x%02x, erase_cmd->count = %d\n",
+				cmd->size, cmd->opcode, cmd->count);
+
 			addr += cmd->size;
 			cmd->count--;
 		}