diff mbox series

[v4,12/12] mtd: rawnand: brcmnand: update log level messages

Message ID 20240203002834.171462-13-william.zhang@broadcom.com
State New
Headers show
Series dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs | expand

Commit Message

William Zhang Feb. 3, 2024, 12:28 a.m. UTC
From: David Regan <dregan@broadcom.com>

Update log level messages so that more critical messages
can be seen.

Signed-off-by: David Regan <dregan@broadcom.com>
Signed-off-by: William Zhang <william.zhang@broadcom.com>
Reviewed-by: William Zhang <william.zhang@broadcom.com>

---

Changes in v4:
- Revert the log level change for correctable ecc error

Changes in v3: None
Changes in v2:
- Added to patch series

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Miquel Raynal Feb. 5, 2024, 1:37 p.m. UTC | #1
Hi William,

william.zhang@broadcom.com wrote on Fri,  2 Feb 2024 16:28:33 -0800:

> From: David Regan <dregan@broadcom.com>
> 
> Update log level messages so that more critical messages
> can be seen.

This commit log does not convince me. Warning messages are visible,
they are in dmesg. If you want them on your console, lower your default
console level by 1 and they will appear. I'm fine increasing the log
level on error messages, but the justification cannot be specific to
your own setup.

> 
> Signed-off-by: David Regan <dregan@broadcom.com>
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> Reviewed-by: William Zhang <william.zhang@broadcom.com>
> 
> ---
> 
> Changes in v4:
> - Revert the log level change for correctable ecc error
> 
> Changes in v3: None
> Changes in v2:
> - Added to patch series
> 
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 7ce2b267676f..e50582b45182 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -1143,7 +1143,7 @@ static int bcmnand_ctrl_poll_status(struct brcmnand_host *host,
>  	if ((val & mask) == expected_val)
>  		return 0;
>  
> -	dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
> +	dev_err(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
>  		 expected_val, val & mask);
>  
>  	return -ETIMEDOUT;
> @@ -2196,7 +2196,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
>  				return err;
>  		}
>  
> -		dev_dbg(ctrl->dev, "uncorrectable error at 0x%llx\n",
> +		dev_err(ctrl->dev, "uncorrectable error at 0x%llx\n",
>  			(unsigned long long)err_addr);
>  		mtd->ecc_stats.failed++;
>  		/* NAND layer expects zero on ECC errors */


Thanks,
Miquèl
William Zhang Feb. 5, 2024, 6:20 p.m. UTC | #2
On 2/5/24 05:37, Miquel Raynal wrote:
> Hi William,
> 
> william.zhang@broadcom.com wrote on Fri,  2 Feb 2024 16:28:33 -0800:
> 
>> From: David Regan <dregan@broadcom.com>
>>
>> Update log level messages so that more critical messages
>> can be seen.
> 
> This commit log does not convince me. Warning messages are visible,
> they are in dmesg. If you want them on your console, lower your default
> console level by 1 and they will appear. I'm fine increasing the log
> level on error messages, but the justification cannot be specific to
> your own setup.
> 
It is mainly for troubleshooting our customer devices. Understand we can 
get the log through dmesg but when we deal with system hang issue, 
console log is very important. And also sometimes customer has rebooted 
the device due to error condition and we lost the dmesg.

How about we update the commit message to:
Update log level messages so that more critical messages
can be logged to console and help the troubleshooting with customer 
devices.

>>
>> Signed-off-by: David Regan <dregan@broadcom.com>
>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>> Reviewed-by: William Zhang <william.zhang@broadcom.com>
>>
>> ---
>>
>> Changes in v4:
>> - Revert the log level change for correctable ecc error
>>
>> Changes in v3: None
>> Changes in v2:
>> - Added to patch series
>>
>>   drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> index 7ce2b267676f..e50582b45182 100644
>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> @@ -1143,7 +1143,7 @@ static int bcmnand_ctrl_poll_status(struct brcmnand_host *host,
>>   	if ((val & mask) == expected_val)
>>   		return 0;
>>   
>> -	dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
>> +	dev_err(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
>>   		 expected_val, val & mask);
>>   
>>   	return -ETIMEDOUT;
>> @@ -2196,7 +2196,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
>>   				return err;
>>   		}
>>   
>> -		dev_dbg(ctrl->dev, "uncorrectable error at 0x%llx\n",
>> +		dev_err(ctrl->dev, "uncorrectable error at 0x%llx\n",
>>   			(unsigned long long)err_addr);
>>   		mtd->ecc_stats.failed++;
>>   		/* NAND layer expects zero on ECC errors */
> 
> 
> Thanks,
> Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 7ce2b267676f..e50582b45182 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1143,7 +1143,7 @@  static int bcmnand_ctrl_poll_status(struct brcmnand_host *host,
 	if ((val & mask) == expected_val)
 		return 0;
 
-	dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
+	dev_err(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
 		 expected_val, val & mask);
 
 	return -ETIMEDOUT;
@@ -2196,7 +2196,7 @@  static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
 				return err;
 		}
 
-		dev_dbg(ctrl->dev, "uncorrectable error at 0x%llx\n",
+		dev_err(ctrl->dev, "uncorrectable error at 0x%llx\n",
 			(unsigned long long)err_addr);
 		mtd->ecc_stats.failed++;
 		/* NAND layer expects zero on ECC errors */