Re: [net-next 02/11] devlink: add 'reset_dev_on_drv_probe' param

[Jiri Pirko <jiri@resnulli.us>]

Fri, Sep 06, 2019 at 06:00:52PM CEST, simon.horman@netronome.com wrote:
>From: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
>
>Add the 'reset_dev_on_drv_probe' devlink parameter, controlling the
>device reset policy on driver probe.
>
>This parameter is useful in conjunction with the existing
>'fw_load_policy' parameter.
>
>Signed-off-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
>Signed-off-by: Simon Horman <simon.horman@netronome.com>
>---
> Documentation/networking/devlink-params.txt | 14 ++++++++++++++
> include/net/devlink.h                       |  4 ++++
> include/uapi/linux/devlink.h                |  7 +++++++
> net/core/devlink.c                          |  5 +++++
> 4 files changed, 30 insertions(+)
>
>diff --git a/Documentation/networking/devlink-params.txt b/Documentation/networking/devlink-params.txt
>index fadb5436188d..f9e30d686243 100644
>--- a/Documentation/networking/devlink-params.txt
>+++ b/Documentation/networking/devlink-params.txt
>@@ -51,3 +51,17 @@ fw_load_policy		[DEVICE, GENERIC]
> 			* DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_DISK (2)
> 			  Load firmware currently available on host's disk.
> 			Type: u8
>+
>+reset_dev_on_drv_probe	[DEVICE, GENERIC]
>+			Controls the device's reset policy on driver probe.
>+			Valid values:
>+			* DEVLINK_PARAM_RESET_DEV_VALUE_UNKNOWN (0)
>+			  Unknown or invalid value.

Why do you need this? Do you have usecase for this value?


>+			* DEVLINK_PARAM_RESET_DEV_VALUE_ALWAYS (1)
>+			  Always reset device on driver probe.
>+			* DEVLINK_PARAM_RESET_DEV_VALUE_NEVER (2)
>+			  Never reset device on driver probe.
>+			* DEVLINK_PARAM_RESET_DEV_VALUE_DISK (3)
>+			  Reset only if device firmware can be found in the
>+			  filesystem.
>+			Type: u8
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 460bc629d1a4..d880de5b8d3a 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -398,6 +398,7 @@ enum devlink_param_generic_id {
> 	DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
> 	DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
> 	DEVLINK_PARAM_GENERIC_ID_FW_LOAD_POLICY,
>+	DEVLINK_PARAM_GENERIC_ID_RESET_DEV,
> 
> 	/* add new param generic ids above here*/
> 	__DEVLINK_PARAM_GENERIC_ID_MAX,
>@@ -428,6 +429,9 @@ enum devlink_param_generic_id {
> #define DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_NAME "fw_load_policy"
> #define DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_TYPE DEVLINK_PARAM_TYPE_U8
> 
>+#define DEVLINK_PARAM_GENERIC_RESET_DEV_NAME "reset_dev_on_drv_probe"

The name of the define and name of the string should be the same. Please
adjust.


>+#define DEVLINK_PARAM_GENERIC_RESET_DEV_TYPE DEVLINK_PARAM_TYPE_U8
>+
> #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate)	\
> {									\
> 	.id = DEVLINK_PARAM_GENERIC_ID_##_id,				\
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index c25cc29a6647..3172d1b3329f 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -205,6 +205,13 @@ enum devlink_param_fw_load_policy_value {
> 	DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_DISK,
> };
> 
>+enum devlink_param_reset_dev_value {
>+	DEVLINK_PARAM_RESET_DEV_VALUE_UNKNOWN,
>+	DEVLINK_PARAM_RESET_DEV_VALUE_ALWAYS,
>+	DEVLINK_PARAM_RESET_DEV_VALUE_NEVER,
>+	DEVLINK_PARAM_RESET_DEV_VALUE_DISK,
>+};
>+
> enum {
> 	DEVLINK_ATTR_STATS_RX_PACKETS,		/* u64 */
> 	DEVLINK_ATTR_STATS_RX_BYTES,		/* u64 */
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 6e52d639dac6..e8bc96f104a7 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -2852,6 +2852,11 @@ static const struct devlink_param devlink_param_generic[] = {
> 		.name = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_NAME,
> 		.type = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_TYPE,
> 	},
>+	{
>+		.id = DEVLINK_PARAM_GENERIC_ID_RESET_DEV,
>+		.name = DEVLINK_PARAM_GENERIC_RESET_DEV_NAME,
>+		.type = DEVLINK_PARAM_GENERIC_RESET_DEV_TYPE,
>+	},
> };
> 
> static int devlink_param_generic_verify(const struct devlink_param *param)
>-- 
>2.11.0
>