[PATCH net-next 0/6] mlxfw: Improve error reporting

[PATCH net-next 0/6] mlxfw: Improve error reporting

[Saeed Mahameed]

Hi Dave,

This patchset improves mlxfw error reporting of mlxfw to netlink and
kernel log.

1) patch #1, Convert extack msg to a formattable buffer

2) patch #2, Make mlxfw/mlxsw fw flash devlink status notify generic,
   and enable it for mlx5.

3) rest of the patches are improving mlxfw flash error messages by
reporting detailed mlxfw FSM error messages to netlink and more detailed
kernel log.

merge conflict with net:
@ drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c

++<<<<<<< (net-next) (keep)
+
+       if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK)
+               return mlxfw_fsm_state_err(mlxfw_dev, extack, fsm_state_err);
+
++=======  
+ 
+       if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
+               fsm_state_err = min_t(enum mlxfw_fsm_state_err,
+                                     fsm_state_err, MLXFW_FSM_STATE_ERR_MAX);
+               pr_err("Firmware flash failed: %s\n",
+                      mlxfw_fsm_state_err_str[fsm_state_err]);
+               NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
+               return -EINVAL;
++>>>>>>> (net) (delete) 

To resolve just use the 1st hunk, from net-next.

Thanks,
Saeed.

---

Saeed Mahameed (6):
  netlink: Convert extack msg to a formattable buffer
  net/mlxfw: Generic mlx FW flash status notify
  net/mlxfw: Improve FSM err message reporting and return codes
  net/mlxfw: Convert pr_* to dev_* in mlxfw_fsm.c
  net/mlxfw: More error messages coverage
  net/mlxfw: Macro for error reporting

 drivers/net/ethernet/mellanox/mlx5/core/fw.c  |   1 +
 drivers/net/ethernet/mellanox/mlxfw/Kconfig   |   1 +
 drivers/net/ethernet/mellanox/mlxfw/mlxfw.h   |  36 ++--
 .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c   | 162 +++++++++++++-----
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  17 +-
 include/linux/netlink.h                       |  27 ++-
 lib/nlattr.c                                  |   2 +-
 7 files changed, 162 insertions(+), 84 deletions(-)

-- 
2.21.0

[PATCH net-next 1/6] netlink: Convert extack msg to a formattable buffer

[Saeed Mahameed]

Before this patch the extack msg had to be a const char and didn't leave
much room for developers to report formattable and flexible messages.

All current usages are oneliner messages, hence, a buffer of size 128B
should be sufficient to replace the const char pointer.

This will allow future usages to provide formattable messages and more
flexible error reporting, without any impact on current users.

Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/linux/netlink.h | 27 ++++++++++++---------------
 lib/nlattr.c            |  2 +-
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 205fa7b1f07a..de9004da0288 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -62,6 +62,7 @@ netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg)
 
 /* this can be increased when necessary - don't expose to userland */
 #define NETLINK_MAX_COOKIE_LEN	20
+#define NL_EXTACK_MAX_MSG_SZ	128
 
 /**
  * struct netlink_ext_ack - netlink extended ACK report struct
@@ -72,40 +73,36 @@ netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg)
  * @cookie_len: actual cookie data length
  */
 struct netlink_ext_ack {
-	const char *_msg;
+	char _msg[NL_EXTACK_MAX_MSG_SZ];
 	const struct nlattr *bad_attr;
 	u8 cookie[NETLINK_MAX_COOKIE_LEN];
 	u8 cookie_len;
 };
 
-/* Always use this macro, this allows later putting the
- * message into a separate section or such for things
- * like translation or listing all possible messages.
- * Currently string formatting is not supported (due
- * to the lack of an output buffer.)
- */
-#define NL_SET_ERR_MSG(extack, msg) do {		\
-	static const char __msg[] = msg;		\
+#define NL_MSG_FMT(extack, fmt, ...) \
+	WARN_ON(snprintf(extack->_msg, NL_EXTACK_MAX_MSG_SZ, fmt, ## __VA_ARGS__) \
+		>= NL_EXTACK_MAX_MSG_SZ)
+
+#define NL_SET_ERR_MSG(extack, fmt, ...) do {		\
 	struct netlink_ext_ack *__extack = (extack);	\
 							\
 	if (__extack)					\
-		__extack->_msg = __msg;			\
+		NL_MSG_FMT(__extack, fmt, ## __VA_ARGS__); \
 } while (0)
 
-#define NL_SET_ERR_MSG_MOD(extack, msg)			\
-	NL_SET_ERR_MSG((extack), KBUILD_MODNAME ": " msg)
+#define NL_SET_ERR_MSG_MOD(extack, fmt, ...)			\
+	NL_SET_ERR_MSG((extack), KBUILD_MODNAME ": " fmt, ## __VA_ARGS__)
 
 #define NL_SET_BAD_ATTR(extack, attr) do {		\
 	if ((extack))					\
 		(extack)->bad_attr = (attr);		\
 } while (0)
 
-#define NL_SET_ERR_MSG_ATTR(extack, attr, msg) do {	\
-	static const char __msg[] = msg;		\
+#define NL_SET_ERR_MSG_ATTR(extack, attr, fmt, ...) do {	\
 	struct netlink_ext_ack *__extack = (extack);	\
 							\
 	if (__extack) {					\
-		__extack->_msg = __msg;			\
+		NL_MSG_FMT(__extack, fmt, ## __VA_ARGS__); \
 		__extack->bad_attr = (attr);		\
 	}						\
 } while (0)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index cace9b307781..2ce1d6b68ce8 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -208,7 +208,7 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
 	case NLA_REJECT:
 		if (extack && pt->validation_data) {
 			NL_SET_BAD_ATTR(extack, nla);
-			extack->_msg = pt->validation_data;
+			NL_MSG_FMT(extack, pt->validation_data);
 			return -EINVAL;
 		}
 		err = -EINVAL;
-- 
2.21.0

[PATCH net-next 2/6] net/mlxfw: Generic mlx FW flash status notify

[Saeed Mahameed]

FW flash status notify is currently implemented via a callback to the
caller mlx module, and all it is doing is to call
devlink_flash_update_status_notify with the specific module devlink
instance.

Instead of repeating the whole process for all mlx modules and
re-implement the status_notify callback again and again. Just provide the
devlink instance as part of mlxfw_dev when calling mlxfw_firmware_flash
and let mlxfw do the devlink status updates directly.

This will be very useful for adding status notify support to mlx5, as
already done in this patch, with a simple one line of just providing the
devlink instance to mlxfw_firmware_flash.

mlxfw now depends on NET_DEVLINK as all other mlx modules.

Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/fw.c    |  1 +
 drivers/net/ethernet/mellanox/mlxfw/Kconfig     |  1 +
 drivers/net/ethernet/mellanox/mlxfw/mlxfw.h     |  6 ++----
 drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c |  9 +++++----
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c  | 17 +----------------
 5 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw.c b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
index a19790dee7b2..9c8956c51169 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
@@ -624,6 +624,7 @@ int mlx5_firmware_flash(struct mlx5_core_dev *dev,
 			.ops = &mlx5_mlxfw_dev_ops,
 			.psid = dev->board_id,
 			.psid_size = strlen(dev->board_id),
+			.devlink = priv_to_devlink(dev),
 		},
 		.mlx5_core_dev = dev
 	};
diff --git a/drivers/net/ethernet/mellanox/mlxfw/Kconfig b/drivers/net/ethernet/mellanox/mlxfw/Kconfig
index 0367f835a846..5b604501f33e 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxfw/Kconfig
@@ -12,3 +12,4 @@ config MLXFW
 	  To compile this driver as a module, choose M here: the
 	  module will be called mlxfw.
 	select XZ_DEC
+	select NET_DEVLINK
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
index c50e74ab02c4..cd88fd257501 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
@@ -6,6 +6,7 @@
 
 #include <linux/firmware.h>
 #include <linux/netlink.h>
+#include <net/devlink.h>
 
 enum mlxfw_fsm_state {
 	MLXFW_FSM_STATE_IDLE,
@@ -58,16 +59,13 @@ struct mlxfw_dev_ops {
 	void (*fsm_cancel)(struct mlxfw_dev *mlxfw_dev, u32 fwhandle);
 
 	void (*fsm_release)(struct mlxfw_dev *mlxfw_dev, u32 fwhandle);
-
-	void (*status_notify)(struct mlxfw_dev *mlxfw_dev,
-			      const char *msg, const char *comp_name,
-			      u32 done_bytes, u32 total_bytes);
 };
 
 struct mlxfw_dev {
 	const struct mlxfw_dev_ops *ops;
 	const char *psid;
 	u16 psid_size;
+	struct devlink *devlink;
 };
 
 #if IS_REACHABLE(CONFIG_MLXFW)
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index 67990406cba2..afcdc579578c 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -43,10 +43,8 @@ static void mlxfw_status_notify(struct mlxfw_dev *mlxfw_dev,
 				const char *msg, const char *comp_name,
 				u32 done_bytes, u32 total_bytes)
 {
-	if (!mlxfw_dev->ops->status_notify)
-		return;
-	mlxfw_dev->ops->status_notify(mlxfw_dev, msg, comp_name,
-				      done_bytes, total_bytes);
+	devlink_flash_update_status_notify(mlxfw_dev->devlink, msg, comp_name,
+					   done_bytes, total_bytes);
 }
 
 static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
@@ -223,6 +221,7 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 		return PTR_ERR(mfa2_file);
 
 	pr_info("Initialize firmware flash process\n");
+	devlink_flash_update_begin_notify(mlxfw_dev->devlink);
 	mlxfw_status_notify(mlxfw_dev, "Initializing firmware flash process",
 			    NULL, 0, 0);
 	err = mlxfw_dev->ops->fsm_lock(mlxfw_dev, &fwhandle);
@@ -261,6 +260,7 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	pr_info("Firmware flash done.\n");
 	mlxfw_status_notify(mlxfw_dev, "Firmware flash done", NULL, 0, 0);
 	mlxfw_mfa2_file_fini(mfa2_file);
+	devlink_flash_update_end_notify(mlxfw_dev->devlink);
 	return 0;
 
 err_state_wait_activate_to_locked:
@@ -270,6 +270,7 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	mlxfw_dev->ops->fsm_release(mlxfw_dev, fwhandle);
 err_fsm_lock:
 	mlxfw_mfa2_file_fini(mfa2_file);
+	devlink_flash_update_end_notify(mlxfw_dev->devlink);
 	return err;
 }
 EXPORT_SYMBOL(mlxfw_firmware_flash);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 556dca328bb5..4df6f01fe5ca 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -345,19 +345,6 @@ static void mlxsw_sp_fsm_release(struct mlxfw_dev *mlxfw_dev, u32 fwhandle)
 	mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(mcc), mcc_pl);
 }
 
-static void mlxsw_sp_status_notify(struct mlxfw_dev *mlxfw_dev,
-				   const char *msg, const char *comp_name,
-				   u32 done_bytes, u32 total_bytes)
-{
-	struct mlxsw_sp_mlxfw_dev *mlxsw_sp_mlxfw_dev =
-		container_of(mlxfw_dev, struct mlxsw_sp_mlxfw_dev, mlxfw_dev);
-	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_mlxfw_dev->mlxsw_sp;
-
-	devlink_flash_update_status_notify(priv_to_devlink(mlxsw_sp->core),
-					   msg, comp_name,
-					   done_bytes, total_bytes);
-}
-
 static const struct mlxfw_dev_ops mlxsw_sp_mlxfw_dev_ops = {
 	.component_query	= mlxsw_sp_component_query,
 	.fsm_lock		= mlxsw_sp_fsm_lock,
@@ -368,7 +355,6 @@ static const struct mlxfw_dev_ops mlxsw_sp_mlxfw_dev_ops = {
 	.fsm_query_state	= mlxsw_sp_fsm_query_state,
 	.fsm_cancel		= mlxsw_sp_fsm_cancel,
 	.fsm_release		= mlxsw_sp_fsm_release,
-	.status_notify		= mlxsw_sp_status_notify,
 };
 
 static int mlxsw_sp_firmware_flash(struct mlxsw_sp *mlxsw_sp,
@@ -380,16 +366,15 @@ static int mlxsw_sp_firmware_flash(struct mlxsw_sp *mlxsw_sp,
 			.ops = &mlxsw_sp_mlxfw_dev_ops,
 			.psid = mlxsw_sp->bus_info->psid,
 			.psid_size = strlen(mlxsw_sp->bus_info->psid),
+			.devlink = priv_to_devlink(mlxsw_sp->core),
 		},
 		.mlxsw_sp = mlxsw_sp
 	};
 	int err;
 
 	mlxsw_core_fw_flash_start(mlxsw_sp->core);
-	devlink_flash_update_begin_notify(priv_to_devlink(mlxsw_sp->core));
 	err = mlxfw_firmware_flash(&mlxsw_sp_mlxfw_dev.mlxfw_dev,
 				   firmware, extack);
-	devlink_flash_update_end_notify(priv_to_devlink(mlxsw_sp->core));
 	mlxsw_core_fw_flash_end(mlxsw_sp->core);
 
 	return err;
-- 
2.21.0

[PATCH net-next 3/6] net/mlxfw: Improve FSM err message reporting and return codes

[Saeed Mahameed]

Report unique and standard error codes corresponding to the specific
FW flash error and report more detailed error messages to netlink.

Before:
$ devlink dev flash pci/0000:05:00.0 file ...
Error: mlxfw: Firmware flash failed.
devlink answers: Invalid argument

After:
$ devlink dev flash pci/0000:05:00.0 file ...
Error: mlxfw: Firmware flash failed: pending reset.
devlink answers: Operation already in progress

Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c   | 55 +++++++++++++++++--
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index afcdc579578c..ba1e5b276c54 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -39,6 +39,52 @@ static const char * const mlxfw_fsm_state_err_str[] = {
 		"unknown error"
 };
 
+static int mlxfw_fsm_state_err(struct netlink_ext_ack *extack,
+			       enum mlxfw_fsm_state_err fsm_state_err)
+{
+#define MLXFW_ERR_PRFX "Firmware flash failed: "
+#define MLXFW_FSM_STATE_ERR_NL(extack, msg) \
+	NL_SET_ERR_MSG_MOD((extack), MLXFW_ERR_PRFX msg)
+
+	fsm_state_err = min_t(enum mlxfw_fsm_state_err, fsm_state_err,
+			      MLXFW_FSM_STATE_ERR_MAX);
+	pr_err(MLXFW_ERR_PRFX "%s\n", mlxfw_fsm_state_err_str[fsm_state_err]);
+	switch (fsm_state_err) {
+	case MLXFW_FSM_STATE_ERR_ERROR:
+		MLXFW_FSM_STATE_ERR_NL(extack, "general error");
+		return -EREMOTEIO;
+	case MLXFW_FSM_STATE_ERR_REJECTED_DIGEST_ERR:
+		MLXFW_FSM_STATE_ERR_NL(extack, "component hash mismatch");
+		return -EBADMSG;
+	case MLXFW_FSM_STATE_ERR_REJECTED_NOT_APPLICABLE:
+		MLXFW_FSM_STATE_ERR_NL(extack, "component not applicable");
+		return -ENOENT;
+	case MLXFW_FSM_STATE_ERR_REJECTED_UNKNOWN_KEY:
+		MLXFW_FSM_STATE_ERR_NL(extack, "unknown key");
+		return -ENOKEY;
+	case MLXFW_FSM_STATE_ERR_REJECTED_AUTH_FAILED:
+		MLXFW_FSM_STATE_ERR_NL(extack, "authentication failed");
+		return -EACCES;
+	case MLXFW_FSM_STATE_ERR_REJECTED_UNSIGNED:
+		MLXFW_FSM_STATE_ERR_NL(extack, "component was not signed");
+		return -EKEYREVOKED;
+	case MLXFW_FSM_STATE_ERR_REJECTED_KEY_NOT_APPLICABLE:
+		MLXFW_FSM_STATE_ERR_NL(extack, "key not applicable");
+		return -EKEYREJECTED;
+	case MLXFW_FSM_STATE_ERR_REJECTED_BAD_FORMAT:
+		MLXFW_FSM_STATE_ERR_NL(extack, "bad format");
+		return -ENOEXEC;
+	case MLXFW_FSM_STATE_ERR_BLOCKED_PENDING_RESET:
+		MLXFW_FSM_STATE_ERR_NL(extack, "pending reset");
+		return -EALREADY;
+	case MLXFW_FSM_STATE_ERR_OK: /* should never happen */
+	case MLXFW_FSM_STATE_ERR_MAX:
+		MLXFW_FSM_STATE_ERR_NL(extack, "unknown error");
+		return -EINVAL;
+	}
+	return -EINVAL;
+};
+
 static void mlxfw_status_notify(struct mlxfw_dev *mlxfw_dev,
 				const char *msg, const char *comp_name,
 				u32 done_bytes, u32 total_bytes)
@@ -63,12 +109,9 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 	if (err)
 		return err;
 
-	if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
-		pr_err("Firmware flash failed: %s\n",
-		       mlxfw_fsm_state_err_str[fsm_state_err]);
-		NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
-		return -EINVAL;
-	}
+	if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK)
+		return mlxfw_fsm_state_err(extack, fsm_state_err);
+
 	if (curr_fsm_state != fsm_state) {
 		if (--times == 0) {
 			pr_err("Timeout reached on FSM state change");
-- 
2.21.0

[PATCH net-next 4/6] net/mlxfw: Convert pr_* to dev_* in mlxfw_fsm.c

[Saeed Mahameed]

Introduce mlxfw_{info, err, dbg} macros and make them call corresponding
dev_* macros, then convert all instances of pr_* to mlxfw_*.

This will allow printing the device name mlxfw is operating on.

Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxfw/mlxfw.h   | 32 ++++++++++-----
 .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c   | 39 ++++++++++---------
 2 files changed, 44 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
index cd88fd257501..a0a63e0c5aca 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
@@ -6,8 +6,31 @@
 
 #include <linux/firmware.h>
 #include <linux/netlink.h>
+#include <linux/device.h>
 #include <net/devlink.h>
 
+struct mlxfw_dev {
+	const struct mlxfw_dev_ops *ops;
+	const char *psid;
+	u16 psid_size;
+	struct devlink *devlink;
+};
+
+static inline
+struct device *mlxfw_dev_dev(struct mlxfw_dev *mlxfw_dev)
+{
+	return mlxfw_dev->devlink->dev;
+}
+
+#define MLXFW_PRFX "mlxfw: "
+
+#define mlxfw_info(mlxfw_dev, fmt, ...) \
+	dev_info(mlxfw_dev_dev(mlxfw_dev), MLXFW_PRFX fmt, ## __VA_ARGS__)
+#define mlxfw_err(mlxfw_dev, fmt, ...) \
+	dev_err(mlxfw_dev_dev(mlxfw_dev), MLXFW_PRFX fmt, ## __VA_ARGS__)
+#define mlxfw_dbg(mlxfw_dev, fmt, ...) \
+	dev_dbg(mlxfw_dev_dev(mlxfw_dev), MLXFW_PRFX fmt, ## __VA_ARGS__)
+
 enum mlxfw_fsm_state {
 	MLXFW_FSM_STATE_IDLE,
 	MLXFW_FSM_STATE_LOCKED,
@@ -32,8 +55,6 @@ enum mlxfw_fsm_state_err {
 	MLXFW_FSM_STATE_ERR_MAX,
 };
 
-struct mlxfw_dev;
-
 struct mlxfw_dev_ops {
 	int (*component_query)(struct mlxfw_dev *mlxfw_dev, u16 component_index,
 			       u32 *p_max_size, u8 *p_align_bits,
@@ -61,13 +82,6 @@ struct mlxfw_dev_ops {
 	void (*fsm_release)(struct mlxfw_dev *mlxfw_dev, u32 fwhandle);
 };
 
-struct mlxfw_dev {
-	const struct mlxfw_dev_ops *ops;
-	const char *psid;
-	u16 psid_size;
-	struct devlink *devlink;
-};
-
 #if IS_REACHABLE(CONFIG_MLXFW)
 int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 			 const struct firmware *firmware,
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index ba1e5b276c54..204ef6ed7651 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -39,7 +39,8 @@ static const char * const mlxfw_fsm_state_err_str[] = {
 		"unknown error"
 };
 
-static int mlxfw_fsm_state_err(struct netlink_ext_ack *extack,
+static int mlxfw_fsm_state_err(struct mlxfw_dev *mlxfw_dev,
+			       struct netlink_ext_ack *extack,
 			       enum mlxfw_fsm_state_err fsm_state_err)
 {
 #define MLXFW_ERR_PRFX "Firmware flash failed: "
@@ -48,7 +49,8 @@ static int mlxfw_fsm_state_err(struct netlink_ext_ack *extack,
 
 	fsm_state_err = min_t(enum mlxfw_fsm_state_err, fsm_state_err,
 			      MLXFW_FSM_STATE_ERR_MAX);
-	pr_err(MLXFW_ERR_PRFX "%s\n", mlxfw_fsm_state_err_str[fsm_state_err]);
+	mlxfw_err(mlxfw_dev, MLXFW_ERR_PRFX "%s\n",
+		  mlxfw_fsm_state_err_str[fsm_state_err]);
 	switch (fsm_state_err) {
 	case MLXFW_FSM_STATE_ERR_ERROR:
 		MLXFW_FSM_STATE_ERR_NL(extack, "general error");
@@ -110,11 +112,11 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 		return err;
 
 	if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK)
-		return mlxfw_fsm_state_err(extack, fsm_state_err);
+		return mlxfw_fsm_state_err(mlxfw_dev, extack, fsm_state_err);
 
 	if (curr_fsm_state != fsm_state) {
 		if (--times == 0) {
-			pr_err("Timeout reached on FSM state change");
+			mlxfw_err(mlxfw_dev, "Timeout reached on FSM state change\n");
 			NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
 			return -ETIMEDOUT;
 		}
@@ -152,8 +154,8 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 
 	comp_max_size = min_t(u32, comp_max_size, MLXFW_FSM_MAX_COMPONENT_SIZE);
 	if (comp->data_size > comp_max_size) {
-		pr_err("Component %d is of size %d which is bigger than limit %d\n",
-		       comp->index, comp->data_size, comp_max_size);
+		mlxfw_err(mlxfw_dev, "Component %d is of size %d which is bigger than limit %d\n",
+			  comp->index, comp->data_size, comp_max_size);
 		NL_SET_ERR_MSG_MOD(extack, "Component is bigger than limit");
 		return -EINVAL;
 	}
@@ -161,7 +163,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 	comp_max_write_size = MLXFW_ALIGN_DOWN(comp_max_write_size,
 					       comp_align_bits);
 
-	pr_debug("Component update\n");
+	mlxfw_dbg(mlxfw_dev, "Component update\n");
 	mlxfw_status_notify(mlxfw_dev, "Updating component", comp_name, 0, 0);
 	err = mlxfw_dev->ops->fsm_component_update(mlxfw_dev, fwhandle,
 						   comp->index,
@@ -174,7 +176,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 	if (err)
 		goto err_out;
 
-	pr_debug("Component download\n");
+	mlxfw_dbg(mlxfw_dev, "Component download\n");
 	mlxfw_status_notify(mlxfw_dev, "Downloading component",
 			    comp_name, 0, comp->data_size);
 	for (offset = 0;
@@ -193,7 +195,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 				    comp->data_size);
 	}
 
-	pr_debug("Component verify\n");
+	mlxfw_dbg(mlxfw_dev, "Component verify\n");
 	mlxfw_status_notify(mlxfw_dev, "Verifying component", comp_name, 0, 0);
 	err = mlxfw_dev->ops->fsm_component_verify(mlxfw_dev, fwhandle,
 						   comp->index);
@@ -223,7 +225,7 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 					      mlxfw_dev->psid_size,
 					      &component_count);
 	if (err) {
-		pr_err("Could not find device PSID in MFA2 file\n");
+		mlxfw_err(mlxfw_dev, "Could not find device PSID in MFA2 file\n");
 		NL_SET_ERR_MSG_MOD(extack, "Could not find device PSID in MFA2 file");
 		return err;
 	}
@@ -236,7 +238,8 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 		if (IS_ERR(comp))
 			return PTR_ERR(comp);
 
-		pr_info("Flashing component type %d\n", comp->index);
+		mlxfw_info(mlxfw_dev, "Flashing component type %d\n",
+			   comp->index);
 		err = mlxfw_flash_component(mlxfw_dev, fwhandle, comp, extack);
 		mlxfw_mfa2_file_component_put(comp);
 		if (err)
@@ -254,7 +257,7 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	int err;
 
 	if (!mlxfw_mfa2_check(firmware)) {
-		pr_err("Firmware file is not MFA2\n");
+		mlxfw_err(mlxfw_dev, "Firmware file is not MFA2\n");
 		NL_SET_ERR_MSG_MOD(extack, "Firmware file is not MFA2");
 		return -EINVAL;
 	}
@@ -263,13 +266,13 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	if (IS_ERR(mfa2_file))
 		return PTR_ERR(mfa2_file);
 
-	pr_info("Initialize firmware flash process\n");
+	mlxfw_info(mlxfw_dev, "Initialize firmware flash process\n");
 	devlink_flash_update_begin_notify(mlxfw_dev->devlink);
 	mlxfw_status_notify(mlxfw_dev, "Initializing firmware flash process",
 			    NULL, 0, 0);
 	err = mlxfw_dev->ops->fsm_lock(mlxfw_dev, &fwhandle);
 	if (err) {
-		pr_err("Could not lock the firmware FSM\n");
+		mlxfw_err(mlxfw_dev, "Could not lock the firmware FSM\n");
 		NL_SET_ERR_MSG_MOD(extack, "Could not lock the firmware FSM");
 		goto err_fsm_lock;
 	}
@@ -283,11 +286,11 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	if (err)
 		goto err_flash_components;
 
-	pr_debug("Activate image\n");
+	mlxfw_dbg(mlxfw_dev, "Activate image\n");
 	mlxfw_status_notify(mlxfw_dev, "Activating image", NULL, 0, 0);
 	err = mlxfw_dev->ops->fsm_activate(mlxfw_dev, fwhandle);
 	if (err) {
-		pr_err("Could not activate the downloaded image\n");
+		mlxfw_err(mlxfw_dev, "Could not activate the downloaded image\n");
 		NL_SET_ERR_MSG_MOD(extack, "Could not activate the downloaded image");
 		goto err_fsm_activate;
 	}
@@ -297,10 +300,10 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	if (err)
 		goto err_state_wait_activate_to_locked;
 
-	pr_debug("Handle release\n");
+	mlxfw_dbg(mlxfw_dev, "Handle release\n");
 	mlxfw_dev->ops->fsm_release(mlxfw_dev, fwhandle);
 
-	pr_info("Firmware flash done.\n");
+	mlxfw_info(mlxfw_dev, "Firmware flash done\n");
 	mlxfw_status_notify(mlxfw_dev, "Firmware flash done", NULL, 0, 0);
 	mlxfw_mfa2_file_fini(mfa2_file);
 	devlink_flash_update_end_notify(mlxfw_dev->devlink);
-- 
2.21.0

[PATCH net-next 5/6] net/mlxfw: More error messages coverage

[Saeed Mahameed]

Make sure mlxfw_firmware_flash reports a detailed user readable error
message in every possible error path, basically every time
mlxfw_dev->ops->*() is called and an error is returned, or when image
initialization is failed.

Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c   | 47 +++++++++++++++----
 1 file changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index 204ef6ed7651..c29c385d1dd0 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -108,8 +108,11 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 retry:
 	err = mlxfw_dev->ops->fsm_query_state(mlxfw_dev, fwhandle,
 					      &curr_fsm_state, &fsm_state_err);
-	if (err)
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack, "FSM state query failed, err (%d)",
+				   err);
 		return err;
+	}
 
 	if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK)
 		return mlxfw_fsm_state_err(mlxfw_dev, extack, fsm_state_err);
@@ -149,8 +152,12 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 	err = mlxfw_dev->ops->component_query(mlxfw_dev, comp->index,
 					      &comp_max_size, &comp_align_bits,
 					      &comp_max_write_size);
-	if (err)
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "FSM component query failed, comp_name(%s) err (%d)",
+				   comp_name, err);
 		return err;
+	}
 
 	comp_max_size = min_t(u32, comp_max_size, MLXFW_FSM_MAX_COMPONENT_SIZE);
 	if (comp->data_size > comp_max_size) {
@@ -168,8 +175,12 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 	err = mlxfw_dev->ops->fsm_component_update(mlxfw_dev, fwhandle,
 						   comp->index,
 						   comp->data_size);
-	if (err)
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "FSM component update failed, comp_name(%s) err (%d)",
+				   comp_name, err);
 		return err;
+	}
 
 	err = mlxfw_fsm_state_wait(mlxfw_dev, fwhandle,
 				   MLXFW_FSM_STATE_DOWNLOAD, extack);
@@ -188,8 +199,12 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 		err = mlxfw_dev->ops->fsm_block_download(mlxfw_dev, fwhandle,
 							 block_ptr, block_size,
 							 offset);
-		if (err)
+		if (err) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Component download failed, comp_name(%s) err (%d)",
+					   comp_name, err);
 			goto err_out;
+		}
 		mlxfw_status_notify(mlxfw_dev, "Downloading component",
 				    comp_name, offset + block_size,
 				    comp->data_size);
@@ -199,8 +214,12 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 	mlxfw_status_notify(mlxfw_dev, "Verifying component", comp_name, 0, 0);
 	err = mlxfw_dev->ops->fsm_component_verify(mlxfw_dev, fwhandle,
 						   comp->index);
-	if (err)
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "FSM component verify failed, comp_name(%s) err (%d)",
+				   comp_name, err);
 		goto err_out;
+	}
 
 	err = mlxfw_fsm_state_wait(mlxfw_dev, fwhandle,
 				   MLXFW_FSM_STATE_LOCKED, extack);
@@ -235,8 +254,13 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 
 		comp = mlxfw_mfa2_file_component_get(mfa2_file, mlxfw_dev->psid,
 						     mlxfw_dev->psid_size, i);
-		if (IS_ERR(comp))
-			return PTR_ERR(comp);
+		if (IS_ERR(comp)) {
+			err = PTR_ERR(comp);
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Failed to get MFA2 component, component (%d) err (%d)",
+					   i, err);
+			return err;
+		}
 
 		mlxfw_info(mlxfw_dev, "Flashing component type %d\n",
 			   comp->index);
@@ -263,8 +287,13 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	}
 
 	mfa2_file = mlxfw_mfa2_file_init(firmware);
-	if (IS_ERR(mfa2_file))
-		return PTR_ERR(mfa2_file);
+	if (IS_ERR(mfa2_file)) {
+		err = PTR_ERR(mfa2_file);
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Failed to initialize MFA2 firmware file, err (%d)",
+				   err);
+		return err;
+	}
 
 	mlxfw_info(mlxfw_dev, "Initialize firmware flash process\n");
 	devlink_flash_update_begin_notify(mlxfw_dev->devlink);
-- 
2.21.0

[PATCH net-next 6/6] net/mlxfw: Macro for error reporting

[Saeed Mahameed]

Add a macro to simplify error message reporting, instead of always
calling both mlxfw_err and NL_SET_ERR_MSG_MOD with the same message,
use a macro instead.

This is doable now since NL_SET_ERR_MSG_MOD can accept formattable
messages.

Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c   | 72 ++++++++++---------
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index c29c385d1dd0..5715192d2195 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -95,6 +95,12 @@ static void mlxfw_status_notify(struct mlxfw_dev *mlxfw_dev,
 					   done_bytes, total_bytes);
 }
 
+#define MLXFW_ERR_MSG(mlxfw_dev, extack, fmt, ...)	\
+({							\
+	mlxfw_err(mlxfw_dev, fmt "\n", ## __VA_ARGS__);	\
+	NL_SET_ERR_MSG_MOD(extack, fmt, ## __VA_ARGS__);\
+})
+
 static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 				enum mlxfw_fsm_state fsm_state,
 				struct netlink_ext_ack *extack)
@@ -109,8 +115,8 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 	err = mlxfw_dev->ops->fsm_query_state(mlxfw_dev, fwhandle,
 					      &curr_fsm_state, &fsm_state_err);
 	if (err) {
-		NL_SET_ERR_MSG_MOD(extack, "FSM state query failed, err (%d)",
-				   err);
+		MLXFW_ERR_MSG(mlxfw_dev, extack, "FSM state query failed, err (%d)",
+			      err);
 		return err;
 	}
 
@@ -119,8 +125,8 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 
 	if (curr_fsm_state != fsm_state) {
 		if (--times == 0) {
-			mlxfw_err(mlxfw_dev, "Timeout reached on FSM state change\n");
-			NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
+			MLXFW_ERR_MSG(mlxfw_dev, extack,
+				      "Timeout reached on FSM state change");
 			return -ETIMEDOUT;
 		}
 		msleep(MLXFW_FSM_STATE_WAIT_CYCLE_MS);
@@ -153,17 +159,17 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 					      &comp_max_size, &comp_align_bits,
 					      &comp_max_write_size);
 	if (err) {
-		NL_SET_ERR_MSG_MOD(extack,
-				   "FSM component query failed, comp_name(%s) err (%d)",
-				   comp_name, err);
+		MLXFW_ERR_MSG(mlxfw_dev, extack,
+			      "FSM component query failed, comp_name(%s) err (%d)",
+			      comp_name, err);
 		return err;
 	}
 
 	comp_max_size = min_t(u32, comp_max_size, MLXFW_FSM_MAX_COMPONENT_SIZE);
 	if (comp->data_size > comp_max_size) {
-		mlxfw_err(mlxfw_dev, "Component %d is of size %d which is bigger than limit %d\n",
-			  comp->index, comp->data_size, comp_max_size);
-		NL_SET_ERR_MSG_MOD(extack, "Component is bigger than limit");
+		MLXFW_ERR_MSG(mlxfw_dev, extack,
+			      "Component size is bigger than limit, Component %d size %d limit %d",
+			      comp->index, comp->data_size, comp_max_size);
 		return -EINVAL;
 	}
 
@@ -176,9 +182,9 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 						   comp->index,
 						   comp->data_size);
 	if (err) {
-		NL_SET_ERR_MSG_MOD(extack,
-				   "FSM component update failed, comp_name(%s) err (%d)",
-				   comp_name, err);
+		MLXFW_ERR_MSG(mlxfw_dev, extack,
+			      "FSM component update failed, comp_name(%s) err (%d)",
+			      comp_name, err);
 		return err;
 	}
 
@@ -200,9 +206,9 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 							 block_ptr, block_size,
 							 offset);
 		if (err) {
-			NL_SET_ERR_MSG_MOD(extack,
-					   "Component download failed, comp_name(%s) err (%d)",
-					   comp_name, err);
+			MLXFW_ERR_MSG(mlxfw_dev, extack,
+				      "Component download failed, comp_name(%s) err (%d)",
+				      comp_name, err);
 			goto err_out;
 		}
 		mlxfw_status_notify(mlxfw_dev, "Downloading component",
@@ -215,9 +221,9 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
 	err = mlxfw_dev->ops->fsm_component_verify(mlxfw_dev, fwhandle,
 						   comp->index);
 	if (err) {
-		NL_SET_ERR_MSG_MOD(extack,
-				   "FSM component verify failed, comp_name(%s) err (%d)",
-				   comp_name, err);
+		MLXFW_ERR_MSG(mlxfw_dev, extack,
+			      "FSM component verify failed, comp_name(%s) err (%d)",
+			      comp_name, err);
 		goto err_out;
 	}
 
@@ -244,8 +250,9 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 					      mlxfw_dev->psid_size,
 					      &component_count);
 	if (err) {
-		mlxfw_err(mlxfw_dev, "Could not find device PSID in MFA2 file\n");
-		NL_SET_ERR_MSG_MOD(extack, "Could not find device PSID in MFA2 file");
+		MLXFW_ERR_MSG(mlxfw_dev, extack,
+			      "Could not find device PSID in MFA2 file, err (%d)",
+			      err);
 		return err;
 	}
 
@@ -256,9 +263,9 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
 						     mlxfw_dev->psid_size, i);
 		if (IS_ERR(comp)) {
 			err = PTR_ERR(comp);
-			NL_SET_ERR_MSG_MOD(extack,
-					   "Failed to get MFA2 component, component (%d) err (%d)",
-					   i, err);
+			MLXFW_ERR_MSG(mlxfw_dev, extack,
+				      "Failed to get MFA2 component, component (%d) err (%d)",
+				      i, err);
 			return err;
 		}
 
@@ -281,17 +288,16 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	int err;
 
 	if (!mlxfw_mfa2_check(firmware)) {
-		mlxfw_err(mlxfw_dev, "Firmware file is not MFA2\n");
-		NL_SET_ERR_MSG_MOD(extack, "Firmware file is not MFA2");
+		MLXFW_ERR_MSG(mlxfw_dev, extack, "Firmware file is not MFA2");
 		return -EINVAL;
 	}
 
 	mfa2_file = mlxfw_mfa2_file_init(firmware);
 	if (IS_ERR(mfa2_file)) {
 		err = PTR_ERR(mfa2_file);
-		NL_SET_ERR_MSG_MOD(extack,
-				   "Failed to initialize MFA2 firmware file, err (%d)",
-				   err);
+		MLXFW_ERR_MSG(mlxfw_dev, extack,
+			      "Failed to initialize MFA2 firmware file, err (%d)",
+			      err);
 		return err;
 	}
 
@@ -301,8 +307,8 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 			    NULL, 0, 0);
 	err = mlxfw_dev->ops->fsm_lock(mlxfw_dev, &fwhandle);
 	if (err) {
-		mlxfw_err(mlxfw_dev, "Could not lock the firmware FSM\n");
-		NL_SET_ERR_MSG_MOD(extack, "Could not lock the firmware FSM");
+		MLXFW_ERR_MSG(mlxfw_dev, extack,
+			      "Could not lock the firmware FSM, err (%d)", err);
 		goto err_fsm_lock;
 	}
 
@@ -319,8 +325,8 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
 	mlxfw_status_notify(mlxfw_dev, "Activating image", NULL, 0, 0);
 	err = mlxfw_dev->ops->fsm_activate(mlxfw_dev, fwhandle);
 	if (err) {
-		mlxfw_err(mlxfw_dev, "Could not activate the downloaded image\n");
-		NL_SET_ERR_MSG_MOD(extack, "Could not activate the downloaded image");
+		MLXFW_ERR_MSG(mlxfw_dev, extack,
+			      "Could not activate the downloaded image, err (%d)", err);
 		goto err_fsm_activate;
 	}
 
-- 
2.21.0

Re: [PATCH net-next 3/6] net/mlxfw: Improve FSM err message reporting and return codes

[Saeed Mahameed]

On Fri, 2019-11-22 at 21:51 +0000, Saeed Mahameed wrote:
> Report unique and standard error codes corresponding to the specific
> FW flash error and report more detailed error messages to netlink.
> 
> Before:
> $ devlink dev flash pci/0000:05:00.0 file ...
> Error: mlxfw: Firmware flash failed.
> devlink answers: Invalid argument
> 
> After:
> $ devlink dev flash pci/0000:05:00.0 file ...
> Error: mlxfw: Firmware flash failed: pending reset.
> devlink answers: Operation already in progress
> 
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c   | 55
> +++++++++++++++++--
>  1 file changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
> b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
> index afcdc579578c..ba1e5b276c54 100644
> --- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
> +++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
> @@ -39,6 +39,52 @@ static const char * const
> mlxfw_fsm_state_err_str[] = {
>  		"unknown error"
>  };
>  
> +static int mlxfw_fsm_state_err(struct netlink_ext_ack *extack,
> +			       enum mlxfw_fsm_state_err fsm_state_err)
> +{
> +#define MLXFW_ERR_PRFX "Firmware flash failed: "
> +#define MLXFW_FSM_STATE_ERR_NL(extack, msg) \
> +	NL_SET_ERR_MSG_MOD((extack), MLXFW_ERR_PRFX msg)
> +
> +	fsm_state_err = min_t(enum mlxfw_fsm_state_err, fsm_state_err,
> +			      MLXFW_FSM_STATE_ERR_MAX);
> +	pr_err(MLXFW_ERR_PRFX "%s\n",
> mlxfw_fsm_state_err_str[fsm_state_err]);
> +	switch (fsm_state_err) {
> +	case MLXFW_FSM_STATE_ERR_ERROR:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "general error");
> +		return -EREMOTEIO;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_DIGEST_ERR:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "component hash
> mismatch");
> +		return -EBADMSG;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_NOT_APPLICABLE:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "component not
> applicable");
> +		return -ENOENT;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_UNKNOWN_KEY:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "unknown key");
> +		return -ENOKEY;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_AUTH_FAILED:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "authentication
> failed");
> +		return -EACCES;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_UNSIGNED:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "component was not
> signed");
> +		return -EKEYREVOKED;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_KEY_NOT_APPLICABLE:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "key not applicable");
> +		return -EKEYREJECTED;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_BAD_FORMAT:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "bad format");
> +		return -ENOEXEC;
> +	case MLXFW_FSM_STATE_ERR_BLOCKED_PENDING_RESET:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "pending reset");
> +		return -EALREADY;
> +	case MLXFW_FSM_STATE_ERR_OK: /* should never happen */
> +	case MLXFW_FSM_STATE_ERR_MAX:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "unknown error");
> +		return -EINVAL;
> +	}

Actually with the introduction of the formattable extack buffer in
first patch this whole switch case can be reduced to 

+       NL_SET_ERR_MSG_MOD(extack,  MLXFW_ERR_PRFX "%s",
+                          mlxfw_fsm_state_err_str[fsm_state_err]);

I will send V2.