Re: [PATCH net-next 2/2] ibmvnic: Update firmware error reporting with cause string

[Thomas Falcon]

On 08/06/2018 10:48 PM, Nathan Fontenot wrote:

On 08/06/2018 09:39 PM, Thomas Falcon wrote:

Print a string instead of the error code. Since there is a
possibility that the driver can recover, classify it as a
warning instead of an error.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---

  drivers/net/ethernet/ibm/ibmvnic.c | 34 ++++++++++++++++++++++++++++++----

  1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c

index 109e4a58efad..dafdd4ade705 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c

@@ -3825,15 +3825,41 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)

      ibmvnic_send_crq(adapter, &crq);
  }

+static const char *ibmvnic_fw_err_cause(u16 cause)
+{
+    switch (cause) {
+    case ADAPTER_PROBLEM:
+        return "adapter problem";
+    case BUS_PROBLEM:
+        return "bus problem";
+    case FW_PROBLEM:
+        return "firmware problem";
+    case DD_PROBLEM:
+        return "device driver problem";
+    case EEH_RECOVERY:
+        return "EEH recovery";
+    case FW_UPDATED:
+        return "firmware updated";
+    case LOW_MEMORY:
+        return "low Memory";
+    default:
+        return "unknown";
+    }
+}
+
  static void handle_error_indication(union ibmvnic_crq *crq,
                      struct ibmvnic_adapter *adapter)
  {
      struct device *dev = &adapter->vdev->dev;
+    u16 cause;
+
+    cause = be16_to_cpu(crq->error_indication.error_cause);

-    dev_err(dev, "Firmware reports %serror, cause %d\n",
-        crq->error_indication.flags
-            & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
-        be16_to_cpu(crq->error_indication.error_cause));
+    dev_warn_ratelimited(dev,

+                 "Firmware reports %serror, cause: %s. Starting recovery...\n",

                                               ^^^^^^^

You're going to want a space between after the %s here.

-Nathan

It does look odd at first glance, but there is a space after "FATAL" below.

Thanks,
Tom

+ crq->error_indication.flags
+                & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
+                 ibmvnic_fw_err_cause(cause));

      if (crq->error_indication.flags & IBMVNIC_FATAL_ERROR)
          ibmvnic_reset(adapter, VNIC_RESET_FATAL);