Patchwork [net-next,1/6] qlcnic: fix unsupported CDRP command error message.

login
register
mail settings
Submitter Jitendra Kalsaria
Date Feb. 17, 2013, 4:53 a.m.
Message ID <1361076831-31746-2-git-send-email-jitendra.kalsaria@qlogic.com>
Download mbox | patch
Permalink /patch/221052/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Jitendra Kalsaria - Feb. 17, 2013, 4:53 a.m.
From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

Add debug messages for FW CDRP command failure.

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c |   32 +++++++++++++++++++++-
 1 files changed, 30 insertions(+), 2 deletions(-)
Joe Perches - Feb. 17, 2013, 5:38 a.m.
On Sat, 2013-02-16 at 23:53 -0500, Jitendra Kalsaria wrote:
> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> Add debug messages for FW CDRP command failure.

These aren't debug messages but are more detailed.

> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
[]
> @@ -147,8 +147,36 @@ int qlcnic_82xx_issue_cmd(struct qlcnic_adapter *adapter,
>  		cmd->rsp.arg[0] = QLCNIC_RCODE_TIMEOUT;
>  	} else if (rsp == QLCNIC_CDRP_RSP_FAIL) {
>  		cmd->rsp.arg[0] = QLCRD32(adapter, QLCNIC_CDRP_ARG(1));
> -		dev_err(&pdev->dev, "failed card response code:0x%x\n",
> -			cmd->rsp.arg[0]);
> +		switch (cmd->rsp.arg[0]) {
> +		case QLCNIC_RCODE_INVALID_ARGS:
> +			dev_err(&pdev->dev, "CDRP invalid args 0x%x\n",
> +				cmd->rsp.arg[0]);
> +			break;

Not sure you care about object size much, but
it's much smaller object code to do:

	const char *fmt;
	switch (cmd->rsp.arg[0]);
	case FOO:
		fmt = "...";
		break;
	etc...
	}

	dev_err(&pdev->dev, fmt, cmd->rsp.arg[0]);

Also, the #defines are decimal, I don't know why you
print them out as hex or if you need to print them
at all really.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jitendra Kalsaria - Feb. 17, 2013, 9:40 a.m.
On 2/16/13 9:38 PM, "Joe Perches" <joe@perches.com> wrote:

>On Sat, 2013-02-16 at 23:53 -0500, Jitendra Kalsaria wrote:
>> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>> 
>> Add debug messages for FW CDRP command failure.
>
>These aren't debug messages but are more detailed.
>
>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
>>b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
>[]
>> @@ -147,8 +147,36 @@ int qlcnic_82xx_issue_cmd(struct qlcnic_adapter
>>*adapter,
>>  		cmd->rsp.arg[0] = QLCNIC_RCODE_TIMEOUT;
>>  	} else if (rsp == QLCNIC_CDRP_RSP_FAIL) {
>>  		cmd->rsp.arg[0] = QLCRD32(adapter, QLCNIC_CDRP_ARG(1));
>> -		dev_err(&pdev->dev, "failed card response code:0x%x\n",
>> -			cmd->rsp.arg[0]);
>> +		switch (cmd->rsp.arg[0]) {
>> +		case QLCNIC_RCODE_INVALID_ARGS:
>> +			dev_err(&pdev->dev, "CDRP invalid args 0x%x\n",
>> +				cmd->rsp.arg[0]);
>> +			break;
>
>Not sure you care about object size much, but
>it's much smaller object code to do:
>
>	const char *fmt;
>	switch (cmd->rsp.arg[0]);
>	case FOO:
>		fmt = "...";
>		break;
>	etc...
>	}
>
>	dev_err(&pdev->dev, fmt, cmd->rsp.arg[0]);
>
>Also, the #defines are decimal, I don't know why you
>print them out as hex or if you need to print them
>at all really.

Joe,

Yes, your suggestion will definitely reduce object code and we don't
really need
to print them all, will print in decimal. I will re-spin series and submit
again.

Thanks!
    Jiten


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
index 4a3bd64..51461ad 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
@@ -147,8 +147,36 @@  int qlcnic_82xx_issue_cmd(struct qlcnic_adapter *adapter,
 		cmd->rsp.arg[0] = QLCNIC_RCODE_TIMEOUT;
 	} else if (rsp == QLCNIC_CDRP_RSP_FAIL) {
 		cmd->rsp.arg[0] = QLCRD32(adapter, QLCNIC_CDRP_ARG(1));
-		dev_err(&pdev->dev, "failed card response code:0x%x\n",
-			cmd->rsp.arg[0]);
+		switch (cmd->rsp.arg[0]) {
+		case QLCNIC_RCODE_INVALID_ARGS:
+			dev_err(&pdev->dev, "CDRP invalid args 0x%x\n",
+				cmd->rsp.arg[0]);
+			break;
+		case QLCNIC_RCODE_NOT_SUPPORTED:
+		case QLCNIC_RCODE_NOT_IMPL:
+			dev_err(&pdev->dev,
+				"CDRP command not supported: 0x%x.\n",
+				cmd->rsp.arg[0]);
+			break;
+		case QLCNIC_RCODE_NOT_PERMITTED:
+			dev_err(&pdev->dev,
+				"CDRP requested action not permitted: 0x%x.\n",
+				cmd->rsp.arg[0]);
+			break;
+		case QLCNIC_RCODE_INVALID:
+			dev_err(&pdev->dev,
+				"CDRP invalid or unknown cmd received: 0x%x.\n",
+				cmd->rsp.arg[0]);
+			break;
+		case QLCNIC_RCODE_TIMEOUT:
+			dev_err(&pdev->dev, "CDRP command timeout: 0x%x.\n",
+				cmd->rsp.arg[0]);
+			break;
+		default:
+			dev_err(&pdev->dev, "CDRP command failed: 0x%x.\n",
+				cmd->rsp.arg[0]);
+			break;
+		}
 	} else if (rsp == QLCNIC_CDRP_RSP_OK)
 		cmd->rsp.arg[0] = QLCNIC_RCODE_SUCCESS;