[patch net-next v2 00/12] mlxsw: Implement dev info and dev flash for line cards

[patch net-next v2 00/12] mlxsw: Implement dev info and dev flash for line cards

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

This patchset implements two features:
1) "devlink dev info" is exposed for line card (patches 5-9)
2) "devlink dev flash" is implemented for line card gearbox
   flashing (patch 10)

For every line card, "a nested" auxiliary device is created which
allows to bind the features mentioned above (patch 3).

The relationship between line card and its auxiliary dev devlink
is carried over extra line card netlink attribute (patches 2 and 4).

The first patch removes devlink_mutex from devlink_register/unregister()
which eliminates possible deadlock during devlink reload command.

Examples:

$ devlink lc show pci/0000:01:00.0 lc 1
pci/0000:01:00.0:
  lc 1 state active type 16x100G nested_devlink auxiliary/mlxsw_core.lc.0
    supported_types:
       16x100G

$ devlink dev show auxiliary/mlxsw_core.lc.0
auxiliary/mlxsw_core.lc.0

$ devlink dev info auxiliary/mlxsw_core.lc.0
auxiliary/mlxsw_core.lc.0:
  versions:
      fixed:
        hw.revision 0
        fw.psid MT_0000000749
      running:
        ini.version 4
        fw 19.2010.1312

$ devlink dev flash auxiliary/mlxsw_core.lc.0 file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2

Jiri Pirko (12):
  net: devlink: make sure that devlink_try_get() works with valid
    pointer during xarray iteration
  net: devlink: introduce nested devlink entity for line card
  mlxsw: core_linecards: Introduce per line card auxiliary device
  mlxsw: core_linecard_dev: Set nested devlink relationship for a line
    card
  mlxsw: core_linecards: Expose HW revision and INI version
  mlxsw: reg: Extend MDDQ by device_info
  mlxsw: core_linecards: Probe provisioned line cards for devices and
    expose FW version
  mlxsw: reg: Add Management DownStream Device Tunneling Register
  mlxsw: core_linecards: Expose device PSID over device info
  mlxsw: core_linecards: Implement line card device flashing
  selftests: mlxsw: Check line card info on provisioned line card
  selftests: mlxsw: Check line card info on activated line card

 Documentation/networking/devlink/mlxsw.rst    |  24 ++
 drivers/net/ethernet/mellanox/mlxsw/Kconfig   |   1 +
 drivers/net/ethernet/mellanox/mlxsw/Makefile  |   2 +-
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  44 +-
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  35 ++
 .../mellanox/mlxsw/core_linecard_dev.c        | 183 ++++++++
 .../ethernet/mellanox/mlxsw/core_linecards.c  | 404 ++++++++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/reg.h     | 173 +++++++-
 include/net/devlink.h                         |   2 +
 include/uapi/linux/devlink.h                  |   2 +
 net/core/devlink.c                            | 155 ++++++-
 .../drivers/net/mlxsw/devlink_linecard.sh     |  54 +++
 12 files changed, 1047 insertions(+), 32 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c

-- 
2.35.3

[patch net-next v2 01/12] net: devlink: make sure that devlink_try_get() works with valid pointer during xarray iteration

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Remove dependency on devlink_mutex during devlinks xarray iteration.

The devlinks xarray consistency is ensured by internally by xarray.
There is a reference taken when working with devlink using
devlink_try_get(). But there is no guarantee that devlink pointer
picked during xarray iteration is not freed before devlink_try_get()
is called.

Make sure that devlink_try_get() works with valid pointer.
Achieve it by:
1) Splitting devlink_put() so the completion is sent only
   after grace period. Completion unblocks the devlink_unregister()
   routine, which is followed-up by devlink_free()
2) Iterate the devlink xarray holding RCU read lock.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- new patch (originally part of different patchset)
---
 net/core/devlink.c | 113 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 95 insertions(+), 18 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 98d79feeb3dc..aca1dd7c1f07 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -70,6 +70,7 @@ struct devlink {
 	u8 reload_failed:1;
 	refcount_t refcount;
 	struct completion comp;
+	struct rcu_head rcu;
 	char priv[] __aligned(NETDEV_ALIGN);
 };
 
@@ -221,8 +222,6 @@ static DEFINE_XARRAY_FLAGS(devlinks, XA_FLAGS_ALLOC);
 /* devlink_mutex
  *
  * An overall lock guarding every operation coming from userspace.
- * It also guards devlink devices list and it is taken when
- * driver registers/unregisters it.
  */
 static DEFINE_MUTEX(devlink_mutex);
 
@@ -232,10 +231,21 @@ struct net *devlink_net(const struct devlink *devlink)
 }
 EXPORT_SYMBOL_GPL(devlink_net);
 
+static void __devlink_put_rcu(struct rcu_head *head)
+{
+	struct devlink *devlink = container_of(head, struct devlink, rcu);
+
+	complete(&devlink->comp);
+}
+
 void devlink_put(struct devlink *devlink)
 {
 	if (refcount_dec_and_test(&devlink->refcount))
-		complete(&devlink->comp);
+		/* Make sure unregister operation that may await the completion
+		 * is unblocked only after all users are after the enf of
+		 * RCU grace period.
+		 */
+		call_rcu(&devlink->rcu, __devlink_put_rcu);
 }
 
 struct devlink *__must_check devlink_try_get(struct devlink *devlink)
@@ -295,6 +305,7 @@ static struct devlink *devlink_get_from_attrs(struct net *net,
 
 	lockdep_assert_held(&devlink_mutex);
 
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (strcmp(devlink->dev->bus->name, busname) == 0 &&
 		    strcmp(dev_name(devlink->dev), devname) == 0 &&
@@ -306,6 +317,7 @@ static struct devlink *devlink_get_from_attrs(struct net *net,
 
 	if (!found || !devlink_try_get(devlink))
 		devlink = ERR_PTR(-ENODEV);
+	rcu_read_unlock();
 
 	return devlink;
 }
@@ -1329,9 +1341,11 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -1358,7 +1372,9 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 	if (err != -EMSGSIZE)
@@ -1432,29 +1448,32 @@ static int devlink_nl_cmd_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
-		if (!net_eq(devlink_net(devlink), sock_net(msg->sk))) {
-			devlink_put(devlink);
-			continue;
-		}
+		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
+			goto retry;
 
-		if (idx < start) {
-			idx++;
-			devlink_put(devlink);
-			continue;
-		}
+		if (idx < start)
+			goto inc;
 
 		err = devlink_nl_fill(msg, devlink, DEVLINK_CMD_NEW,
 				      NETLINK_CB(cb->skb).portid,
 				      cb->nlh->nlmsg_seq, NLM_F_MULTI);
-		devlink_put(devlink);
-		if (err)
+		if (err) {
+			devlink_put(devlink);
 			goto out;
+		}
+inc:
 		idx++;
+retry:
+		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -1495,9 +1514,11 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -1523,7 +1544,9 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -2177,9 +2200,11 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -2208,7 +2233,9 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg,
 		mutex_unlock(&devlink->linecards_lock);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -2449,9 +2476,11 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -2477,7 +2506,9 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -2601,9 +2632,11 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
 		    !devlink->ops->sb_pool_get)
@@ -2626,7 +2659,9 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -2822,9 +2857,11 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
 		    !devlink->ops->sb_port_pool_get)
@@ -2847,7 +2884,9 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -3071,9 +3110,11 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
 		    !devlink->ops->sb_tc_pool_bind_get)
@@ -3097,7 +3138,9 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -5158,9 +5201,11 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -5188,7 +5233,9 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -5393,9 +5440,11 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -5428,7 +5477,9 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -5977,9 +6028,11 @@ static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -5990,7 +6043,9 @@ static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
 		devlink_put(devlink);
 		if (err)
 			goto out;
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 	cb->args[0] = idx;
@@ -6511,9 +6566,11 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -6537,7 +6594,9 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
 		idx++;
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 	mutex_unlock(&devlink_mutex);
 
 	if (err != -EMSGSIZE)
@@ -7691,9 +7750,11 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry_rep;
@@ -7719,11 +7780,13 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
 		mutex_unlock(&devlink->reporters_lock);
 retry_rep:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
 
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry_port;
@@ -7754,7 +7817,9 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry_port:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -8291,9 +8356,11 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -8319,7 +8386,9 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -8518,9 +8587,11 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -8547,7 +8618,9 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -8832,9 +8905,11 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			goto retry;
@@ -8861,7 +8936,9 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
 		devl_unlock(devlink);
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -9589,10 +9666,8 @@ void devlink_register(struct devlink *devlink)
 	ASSERT_DEVLINK_NOT_REGISTERED(devlink);
 	/* Make sure that we are in .probe() routine */
 
-	mutex_lock(&devlink_mutex);
 	xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
 	devlink_notify_register(devlink);
-	mutex_unlock(&devlink_mutex);
 }
 EXPORT_SYMBOL_GPL(devlink_register);
 
@@ -9609,10 +9684,8 @@ void devlink_unregister(struct devlink *devlink)
 	devlink_put(devlink);
 	wait_for_completion(&devlink->comp);
 
-	mutex_lock(&devlink_mutex);
 	devlink_notify_unregister(devlink);
 	xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
-	mutex_unlock(&devlink_mutex);
 }
 EXPORT_SYMBOL_GPL(devlink_unregister);
 
@@ -12281,9 +12354,11 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
 	 * all devlink instances from this namespace into init_net.
 	 */
 	mutex_lock(&devlink_mutex);
+	rcu_read_lock();
 	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
 		if (!devlink_try_get(devlink))
 			continue;
+		rcu_read_unlock();
 
 		if (!net_eq(devlink_net(devlink), net))
 			goto retry;
@@ -12297,7 +12372,9 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
 			pr_warn("Failed to reload devlink instance into init_net\n");
 retry:
 		devlink_put(devlink);
+		rcu_read_lock();
 	}
+	rcu_read_unlock();
 	mutex_unlock(&devlink_mutex);
 }
 
-- 
2.35.3

Re: [patch net-next v2 01/12] net: devlink: make sure that devlink_try_get() works with valid pointer during xarray iteration

[Ido Schimmel]

> @@ -6511,9 +6566,11 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
>  	int err = 0;
>  
>  	mutex_lock(&devlink_mutex);
> +	rcu_read_lock();
>  	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
>  		if (!devlink_try_get(devlink))
>  			continue;
> +		rcu_read_unlock();
>  
>  		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
>  			goto retry;
> @@ -6537,7 +6594,9 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
>  		idx++;
>  retry:
>  		devlink_put(devlink);
> +		rcu_read_lock();
>  	}
> +	rcu_read_unlock();
>  	mutex_unlock(&devlink_mutex);
>  
>  	if (err != -EMSGSIZE)

The pw CI is complaining about this:

New errors added
--- /tmp/tmp.Lx7CWX0u9n	2022-07-18 23:56:05.513142294 -0700
+++ /tmp/tmp.SFcuLwts4X	2022-07-18 23:56:49.917188829 -0700
@@ -0,0 +1,2 @@
+../net/core/devlink.c: note: in included file (through ../include/linux/rculist.h, ../include/linux/dcache.h, ../include/linux/fs.h, ../include/linux/highmem.h, ../include/linux/bvec.h, ../include/linux/skbuff.h, ...):
+../include/linux/rcupdate.h:726:9: warning: context imbalance in 'devlink_nl_cmd_info_get_dumpit' - unexpected unlock

Might need something like:

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 0e8274feab11..5a39a02b6ed6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6611,6 +6611,7 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
 			err = 0;
 		else if (err) {
 			devlink_put(devlink);
+			rcu_read_lock();
 			break;
 		}
 inc:

Re: [patch net-next v2 01/12] net: devlink: make sure that devlink_try_get() works with valid pointer during xarray iteration

[Jiri Pirko]

Tue, Jul 19, 2022 at 01:39:07PM CEST, idosch@nvidia.com wrote:
>> @@ -6511,9 +6566,11 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
>>  	int err = 0;
>>  
>>  	mutex_lock(&devlink_mutex);
>> +	rcu_read_lock();
>>  	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
>>  		if (!devlink_try_get(devlink))
>>  			continue;
>> +		rcu_read_unlock();
>>  
>>  		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
>>  			goto retry;
>> @@ -6537,7 +6594,9 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
>>  		idx++;
>>  retry:
>>  		devlink_put(devlink);
>> +		rcu_read_lock();
>>  	}
>> +	rcu_read_unlock();
>>  	mutex_unlock(&devlink_mutex);
>>  
>>  	if (err != -EMSGSIZE)
>
>The pw CI is complaining about this:
>
>New errors added
>--- /tmp/tmp.Lx7CWX0u9n	2022-07-18 23:56:05.513142294 -0700
>+++ /tmp/tmp.SFcuLwts4X	2022-07-18 23:56:49.917188829 -0700
>@@ -0,0 +1,2 @@
>+../net/core/devlink.c: note: in included file (through ../include/linux/rculist.h, ../include/linux/dcache.h, ../include/linux/fs.h, ../include/linux/highmem.h, ../include/linux/bvec.h, ../include/linux/skbuff.h, ...):
>+../include/linux/rcupdate.h:726:9: warning: context imbalance in 'devlink_nl_cmd_info_get_dumpit' - unexpected unlock
>
>Might need something like:

Ah, yes, I missed that. Will add. Thx.


>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 0e8274feab11..5a39a02b6ed6 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -6611,6 +6611,7 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
> 			err = 0;
> 		else if (err) {
> 			devlink_put(devlink);
>+			rcu_read_lock();
> 			break;
> 		}
> inc:
>

Re: [patch net-next v2 01/12] net: devlink: make sure that devlink_try_get() works with valid pointer during xarray iteration

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:36AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Remove dependency on devlink_mutex during devlinks xarray iteration.

Missing motivation... Need to explain that devlink_mutex is held
throughout every user space command and that while issuing a reload and
registering / unregistering auxiliary devlink instances we will
deadlock on this mutex.

> 
> The devlinks xarray consistency is ensured by internally by xarray.

s/by//

> There is a reference taken when working with devlink using
> devlink_try_get(). But there is no guarantee that devlink pointer
> picked during xarray iteration is not freed before devlink_try_get()
> is called.
> 
> Make sure that devlink_try_get() works with valid pointer.
> Achieve it by:
> 1) Splitting devlink_put() so the completion is sent only
>    after grace period. Completion unblocks the devlink_unregister()
>    routine, which is followed-up by devlink_free()
> 2) Iterate the devlink xarray holding RCU read lock.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

Re: [patch net-next v2 01/12] net: devlink: make sure that devlink_try_get() works with valid pointer during xarray iteration

[Jiri Pirko]

Wed, Jul 20, 2022 at 09:45:26AM CEST, idosch@nvidia.com wrote:
>On Tue, Jul 19, 2022 at 08:48:36AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> Remove dependency on devlink_mutex during devlinks xarray iteration.
>
>Missing motivation... Need to explain that devlink_mutex is held
>throughout every user space command and that while issuing a reload and
>registering / unregistering auxiliary devlink instances we will
>deadlock on this mutex.

Fixed.


>
>> 
>> The devlinks xarray consistency is ensured by internally by xarray.
>
>s/by//

Fixed.


>
>> There is a reference taken when working with devlink using
>> devlink_try_get(). But there is no guarantee that devlink pointer
>> picked during xarray iteration is not freed before devlink_try_get()
>> is called.
>> 
>> Make sure that devlink_try_get() works with valid pointer.
>> Achieve it by:
>> 1) Splitting devlink_put() so the completion is sent only
>>    after grace period. Completion unblocks the devlink_unregister()
>>    routine, which is followed-up by devlink_free()
>> 2) Iterate the devlink xarray holding RCU read lock.
>> 
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

[patch net-next v2 02/12] net: devlink: introduce nested devlink entity for line card

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

For the purpose of exposing device info and allow flash update which is
going to be implemented in follow-up patches, introduce a possibility
for a line card to expose relation to nested devlink entity. The nested
devlink entity represents the line card.

Example:

$ devlink lc show pci/0000:01:00.0 lc 1
pci/0000:01:00.0:
  lc 1 state active type 16x100G nested_devlink auxiliary/mlxsw_core.lc.0
    supported_types:
       16x100G
$ devlink dev show auxiliary/mlxsw_core.lc.0
auxiliary/mlxsw_core.lc.0

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
---
v1->v2:
- s/delink/devlink in devlink_linecard_nested_dl_set comment
- fixed alignment
- s/updated/update in patch description
- added Jakub's ack
- added "net: " prefix to patch subject
- rebased
---
 include/net/devlink.h        |  2 ++
 include/uapi/linux/devlink.h |  2 ++
 net/core/devlink.c           | 42 ++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 780744b550b8..5bd3fac12e9e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1580,6 +1580,8 @@ void devlink_linecard_provision_clear(struct devlink_linecard *linecard);
 void devlink_linecard_provision_fail(struct devlink_linecard *linecard);
 void devlink_linecard_activate(struct devlink_linecard *linecard);
 void devlink_linecard_deactivate(struct devlink_linecard *linecard);
+void devlink_linecard_nested_dl_set(struct devlink_linecard *linecard,
+				    struct devlink *nested_devlink);
 int devl_sb_register(struct devlink *devlink, unsigned int sb_index,
 		     u32 size, u16 ingress_pools_count,
 		     u16 egress_pools_count, u16 ingress_tc_count,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index b3d40a5d72ff..541321695f52 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -576,6 +576,8 @@ enum devlink_attr {
 	DEVLINK_ATTR_LINECARD_TYPE,		/* string */
 	DEVLINK_ATTR_LINECARD_SUPPORTED_TYPES,	/* nested */
 
+	DEVLINK_ATTR_NESTED_DEVLINK,		/* nested */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index aca1dd7c1f07..777d01223f58 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -89,6 +89,7 @@ struct devlink_linecard {
 	const char *type;
 	struct devlink_linecard_type *types;
 	unsigned int types_count;
+	struct devlink *nested_devlink;
 };
 
 /**
@@ -815,6 +816,24 @@ static int devlink_nl_put_handle(struct sk_buff *msg, struct devlink *devlink)
 	return 0;
 }
 
+static int devlink_nl_put_nested_handle(struct sk_buff *msg, struct devlink *devlink)
+{
+	struct nlattr *nested_attr;
+
+	nested_attr = nla_nest_start(msg, DEVLINK_ATTR_NESTED_DEVLINK);
+	if (!nested_attr)
+		return -EMSGSIZE;
+	if (devlink_nl_put_handle(msg, devlink))
+		goto nla_put_failure;
+
+	nla_nest_end(msg, nested_attr);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(msg, nested_attr);
+	return -EMSGSIZE;
+}
+
 struct devlink_reload_combination {
 	enum devlink_reload_action action;
 	enum devlink_reload_limit limit;
@@ -2127,6 +2146,10 @@ static int devlink_nl_linecard_fill(struct sk_buff *msg,
 		nla_nest_end(msg, attr);
 	}
 
+	if (linecard->nested_devlink &&
+	    devlink_nl_put_nested_handle(msg, linecard->nested_devlink))
+		goto nla_put_failure;
+
 	genlmsg_end(msg, hdr);
 	return 0;
 
@@ -10389,6 +10412,7 @@ EXPORT_SYMBOL_GPL(devlink_linecard_provision_set);
 void devlink_linecard_provision_clear(struct devlink_linecard *linecard)
 {
 	mutex_lock(&linecard->state_lock);
+	WARN_ON(linecard->nested_devlink);
 	linecard->state = DEVLINK_LINECARD_STATE_UNPROVISIONED;
 	linecard->type = NULL;
 	devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_NEW);
@@ -10407,6 +10431,7 @@ EXPORT_SYMBOL_GPL(devlink_linecard_provision_clear);
 void devlink_linecard_provision_fail(struct devlink_linecard *linecard)
 {
 	mutex_lock(&linecard->state_lock);
+	WARN_ON(linecard->nested_devlink);
 	linecard->state = DEVLINK_LINECARD_STATE_PROVISIONING_FAILED;
 	devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_NEW);
 	mutex_unlock(&linecard->state_lock);
@@ -10454,6 +10479,23 @@ void devlink_linecard_deactivate(struct devlink_linecard *linecard)
 }
 EXPORT_SYMBOL_GPL(devlink_linecard_deactivate);
 
+/**
+ *	devlink_linecard_nested_dl_set - Attach/detach nested devlink
+ *					 instance to linecard.
+ *
+ *	@linecard: devlink linecard
+ *	@nested_devlink: devlink instance to attach or NULL to detach
+ */
+void devlink_linecard_nested_dl_set(struct devlink_linecard *linecard,
+				    struct devlink *nested_devlink)
+{
+	mutex_lock(&linecard->state_lock);
+	linecard->nested_devlink = nested_devlink;
+	devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_NEW);
+	mutex_unlock(&linecard->state_lock);
+}
+EXPORT_SYMBOL_GPL(devlink_linecard_nested_dl_set);
+
 int devl_sb_register(struct devlink *devlink, unsigned int sb_index,
 		     u32 size, u16 ingress_pools_count,
 		     u16 egress_pools_count, u16 ingress_tc_count,
-- 
2.35.3

Re: [patch net-next v2 02/12] net: devlink: introduce nested devlink entity for line card

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:37AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> For the purpose of exposing device info and allow flash update which is
> going to be implemented in follow-up patches, introduce a possibility
> for a line card to expose relation to nested devlink entity. The nested
> devlink entity represents the line card.
> 
> Example:
> 
> $ devlink lc show pci/0000:01:00.0 lc 1
> pci/0000:01:00.0:
>   lc 1 state active type 16x100G nested_devlink auxiliary/mlxsw_core.lc.0
>     supported_types:
>        16x100G
> $ devlink dev show auxiliary/mlxsw_core.lc.0
> auxiliary/mlxsw_core.lc.0
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

[patch net-next v2 03/12] mlxsw: core_linecards: Introduce per line card auxiliary device

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

In order to be eventually able to expose line card gearbox version and
possibility to flash FW, model the line card as a separate device on
auxiliary bus.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- added auxdev removal to mlxsw_linecard_fini()
- adjusted mlxsw_linecard_bdev_del() to cope with bdev == NULL
---
 drivers/net/ethernet/mellanox/mlxsw/Kconfig   |   1 +
 drivers/net/ethernet/mellanox/mlxsw/Makefile  |   2 +-
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  13 +-
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  10 ++
 .../mellanox/mlxsw/core_linecard_dev.c        | 155 ++++++++++++++++++
 .../ethernet/mellanox/mlxsw/core_linecards.c  |  11 ++
 6 files changed, 189 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c

diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index 4683312861ac..a510bf2cff2f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -7,6 +7,7 @@ config MLXSW_CORE
 	tristate "Mellanox Technologies Switch ASICs support"
 	select NET_DEVLINK
 	select MLXFW
+	select AUXILIARY_BUS
 	help
 	  This driver supports Mellanox Technologies Switch ASICs family.
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/Makefile b/drivers/net/ethernet/mellanox/mlxsw/Makefile
index c2d6d64ffe4b..3ca9fce759ea 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Makefile
+++ b/drivers/net/ethernet/mellanox/mlxsw/Makefile
@@ -2,7 +2,7 @@
 obj-$(CONFIG_MLXSW_CORE)	+= mlxsw_core.o
 mlxsw_core-objs			:= core.o core_acl_flex_keys.o \
 				   core_acl_flex_actions.o core_env.o \
-				   core_linecards.o
+				   core_linecards.o core_linecard_dev.o
 mlxsw_core-$(CONFIG_MLXSW_CORE_HWMON) += core_hwmon.o
 mlxsw_core-$(CONFIG_MLXSW_CORE_THERMAL) += core_thermal.o
 obj-$(CONFIG_MLXSW_PCI)		+= mlxsw_pci.o
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 61eb96b93889..831b0d3472c6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -3334,9 +3334,15 @@ static int __init mlxsw_core_module_init(void)
 {
 	int err;
 
+	err = mlxsw_linecard_driver_register();
+	if (err)
+		return err;
+
 	mlxsw_wq = alloc_workqueue(mlxsw_core_driver_name, 0, 0);
-	if (!mlxsw_wq)
-		return -ENOMEM;
+	if (!mlxsw_wq) {
+		err = -ENOMEM;
+		goto err_alloc_workqueue;
+	}
 	mlxsw_owq = alloc_ordered_workqueue("%s_ordered", 0,
 					    mlxsw_core_driver_name);
 	if (!mlxsw_owq) {
@@ -3347,6 +3353,8 @@ static int __init mlxsw_core_module_init(void)
 
 err_alloc_ordered_workqueue:
 	destroy_workqueue(mlxsw_wq);
+err_alloc_workqueue:
+	mlxsw_linecard_driver_unregister();
 	return err;
 }
 
@@ -3354,6 +3362,7 @@ static void __exit mlxsw_core_module_exit(void)
 {
 	destroy_workqueue(mlxsw_owq);
 	destroy_workqueue(mlxsw_wq);
+	mlxsw_linecard_driver_unregister();
 }
 
 module_init(mlxsw_core_module_init);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index a3491ef2aa7e..b22db13fa547 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -12,6 +12,7 @@
 #include <linux/skbuff.h>
 #include <linux/workqueue.h>
 #include <linux/net_namespace.h>
+#include <linux/auxiliary_bus.h>
 #include <net/devlink.h>
 
 #include "trap.h"
@@ -561,6 +562,8 @@ enum mlxsw_linecard_status_event_type {
 	MLXSW_LINECARD_STATUS_EVENT_TYPE_UNPROVISION,
 };
 
+struct mlxsw_linecard_bdev;
+
 struct mlxsw_linecard {
 	u8 slot_index;
 	struct mlxsw_linecards *linecards;
@@ -575,6 +578,7 @@ struct mlxsw_linecard {
 	   active:1;
 	u16 hw_revision;
 	u16 ini_version;
+	struct mlxsw_linecard_bdev *bdev;
 };
 
 struct mlxsw_linecard_types_info;
@@ -614,4 +618,10 @@ void mlxsw_linecards_event_ops_unregister(struct mlxsw_core *mlxsw_core,
 					  struct mlxsw_linecards_event_ops *ops,
 					  void *priv);
 
+int mlxsw_linecard_bdev_add(struct mlxsw_linecard *linecard);
+void mlxsw_linecard_bdev_del(struct mlxsw_linecard *linecard);
+
+int mlxsw_linecard_driver_register(void);
+void mlxsw_linecard_driver_unregister(void);
+
 #endif
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
new file mode 100644
index 000000000000..bb6068b62df0
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2022 NVIDIA Corporation and Mellanox Technologies. All rights reserved */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/types.h>
+#include <linux/err.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/idr.h>
+#include <linux/gfp.h>
+#include <linux/slab.h>
+#include <net/devlink.h>
+#include "core.h"
+
+#define MLXSW_LINECARD_DEV_ID_NAME "lc"
+
+struct mlxsw_linecard_dev {
+	struct mlxsw_linecard *linecard;
+};
+
+struct mlxsw_linecard_bdev {
+	struct auxiliary_device adev;
+	struct mlxsw_linecard *linecard;
+	struct mlxsw_linecard_dev *linecard_dev;
+};
+
+static DEFINE_IDA(mlxsw_linecard_bdev_ida);
+
+static int mlxsw_linecard_bdev_id_alloc(void)
+{
+	return ida_alloc(&mlxsw_linecard_bdev_ida, GFP_KERNEL);
+}
+
+static void mlxsw_linecard_bdev_id_free(int id)
+{
+	ida_free(&mlxsw_linecard_bdev_ida, id);
+}
+
+static void mlxsw_linecard_bdev_release(struct device *device)
+{
+	struct auxiliary_device *adev =
+			container_of(device, struct auxiliary_device, dev);
+	struct mlxsw_linecard_bdev *linecard_bdev =
+			container_of(adev, struct mlxsw_linecard_bdev, adev);
+
+	mlxsw_linecard_bdev_id_free(adev->id);
+	kfree(linecard_bdev);
+}
+
+int mlxsw_linecard_bdev_add(struct mlxsw_linecard *linecard)
+{
+	struct mlxsw_linecard_bdev *linecard_bdev;
+	int err;
+	int id;
+
+	id = mlxsw_linecard_bdev_id_alloc();
+	if (id < 0)
+		return id;
+
+	linecard_bdev = kzalloc(sizeof(*linecard_bdev), GFP_KERNEL);
+	if (!linecard_bdev) {
+		mlxsw_linecard_bdev_id_free(id);
+		return -ENOMEM;
+	}
+	linecard_bdev->adev.id = id;
+	linecard_bdev->adev.name = MLXSW_LINECARD_DEV_ID_NAME;
+	linecard_bdev->adev.dev.release = mlxsw_linecard_bdev_release;
+	linecard_bdev->adev.dev.parent = linecard->linecards->bus_info->dev;
+	linecard_bdev->linecard = linecard;
+
+	err = auxiliary_device_init(&linecard_bdev->adev);
+	if (err) {
+		mlxsw_linecard_bdev_id_free(id);
+		kfree(linecard_bdev);
+		return err;
+	}
+
+	err = auxiliary_device_add(&linecard_bdev->adev);
+	if (err) {
+		auxiliary_device_uninit(&linecard_bdev->adev);
+		return err;
+	}
+
+	linecard->bdev = linecard_bdev;
+	return 0;
+}
+
+void mlxsw_linecard_bdev_del(struct mlxsw_linecard *linecard)
+{
+	struct mlxsw_linecard_bdev *linecard_bdev = linecard->bdev;
+
+	if (!linecard_bdev)
+		return;
+	auxiliary_device_delete(&linecard_bdev->adev);
+	auxiliary_device_uninit(&linecard_bdev->adev);
+	linecard->bdev = NULL;
+}
+
+static const struct devlink_ops mlxsw_linecard_dev_devlink_ops = {
+};
+
+static int mlxsw_linecard_bdev_probe(struct auxiliary_device *adev,
+				     const struct auxiliary_device_id *id)
+{
+	struct mlxsw_linecard_bdev *linecard_bdev =
+			container_of(adev, struct mlxsw_linecard_bdev, adev);
+	struct mlxsw_linecard_dev *linecard_dev;
+	struct devlink *devlink;
+
+	devlink = devlink_alloc(&mlxsw_linecard_dev_devlink_ops,
+				sizeof(*linecard_dev), &adev->dev);
+	if (!devlink)
+		return -ENOMEM;
+	linecard_dev = devlink_priv(devlink);
+	linecard_dev->linecard = linecard_bdev->linecard;
+	linecard_bdev->linecard_dev = linecard_dev;
+
+	devlink_register(devlink);
+	return 0;
+}
+
+static void mlxsw_linecard_bdev_remove(struct auxiliary_device *adev)
+{
+	struct mlxsw_linecard_bdev *linecard_bdev =
+			container_of(adev, struct mlxsw_linecard_bdev, adev);
+	struct devlink *devlink = priv_to_devlink(linecard_bdev->linecard_dev);
+
+	devlink_unregister(devlink);
+	devlink_free(devlink);
+}
+
+static const struct auxiliary_device_id mlxsw_linecard_bdev_id_table[] = {
+	{ .name = KBUILD_MODNAME "." MLXSW_LINECARD_DEV_ID_NAME },
+	{},
+};
+
+MODULE_DEVICE_TABLE(auxiliary, mlxsw_linecard_bdev_id_table);
+
+static struct auxiliary_driver mlxsw_linecard_driver = {
+	.name = MLXSW_LINECARD_DEV_ID_NAME,
+	.probe = mlxsw_linecard_bdev_probe,
+	.remove = mlxsw_linecard_bdev_remove,
+	.id_table = mlxsw_linecard_bdev_id_table,
+};
+
+int mlxsw_linecard_driver_register(void)
+{
+	return auxiliary_driver_register(&mlxsw_linecard_driver);
+}
+
+void mlxsw_linecard_driver_unregister(void)
+{
+	auxiliary_driver_unregister(&mlxsw_linecard_driver);
+}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
index 5c9869dcf674..43696d8badca 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
@@ -232,6 +232,7 @@ mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
 {
 	struct mlxsw_linecards *linecards = linecard->linecards;
 	const char *type;
+	int err;
 
 	type = mlxsw_linecard_types_lookup(linecards, card_type);
 	mlxsw_linecard_status_event_done(linecard,
@@ -252,6 +253,14 @@ mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
 	linecard->provisioned = true;
 	linecard->hw_revision = hw_revision;
 	linecard->ini_version = ini_version;
+
+	err = mlxsw_linecard_bdev_add(linecard);
+	if (err) {
+		linecard->provisioned = false;
+		mlxsw_linecard_provision_fail(linecard);
+		return err;
+	}
+
 	devlink_linecard_provision_set(linecard->devlink_linecard, type);
 	return 0;
 }
@@ -260,6 +269,7 @@ static void mlxsw_linecard_provision_clear(struct mlxsw_linecard *linecard)
 {
 	mlxsw_linecard_status_event_done(linecard,
 					 MLXSW_LINECARD_STATUS_EVENT_TYPE_UNPROVISION);
+	mlxsw_linecard_bdev_del(linecard);
 	linecard->provisioned = false;
 	devlink_linecard_provision_clear(linecard->devlink_linecard);
 }
@@ -885,6 +895,7 @@ static void mlxsw_linecard_fini(struct mlxsw_core *mlxsw_core,
 	mlxsw_core_flush_owq();
 	if (linecard->active)
 		mlxsw_linecard_active_clear(linecard);
+	mlxsw_linecard_bdev_del(linecard);
 	devlink_linecard_destroy(linecard->devlink_linecard);
 	mutex_destroy(&linecard->lock);
 }
-- 
2.35.3

Re: [patch net-next v2 03/12] mlxsw: core_linecards: Introduce per line card auxiliary device

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:38AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> In order to be eventually able to expose line card gearbox version and
> possibility to flash FW, model the line card as a separate device on
> auxiliary bus.

What is the reason for adding the auxiliary device for the line card
when it becomes 'provisioned' and not when it becomes 'active'? It needs
to be explained in the commit message

> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> v1->v2:
> - added auxdev removal to mlxsw_linecard_fini()
> - adjusted mlxsw_linecard_bdev_del() to cope with bdev == NULL
> ---
>  drivers/net/ethernet/mellanox/mlxsw/Kconfig   |   1 +
>  drivers/net/ethernet/mellanox/mlxsw/Makefile  |   2 +-
>  drivers/net/ethernet/mellanox/mlxsw/core.c    |  13 +-
>  drivers/net/ethernet/mellanox/mlxsw/core.h    |  10 ++
>  .../mellanox/mlxsw/core_linecard_dev.c        | 155 ++++++++++++++++++
>  .../ethernet/mellanox/mlxsw/core_linecards.c  |  11 ++
>  6 files changed, 189 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
> index 4683312861ac..a510bf2cff2f 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
> +++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
> @@ -7,6 +7,7 @@ config MLXSW_CORE
>  	tristate "Mellanox Technologies Switch ASICs support"
>  	select NET_DEVLINK
>  	select MLXFW
> +	select AUXILIARY_BUS
>  	help
>  	  This driver supports Mellanox Technologies Switch ASICs family.
>  
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/Makefile b/drivers/net/ethernet/mellanox/mlxsw/Makefile
> index c2d6d64ffe4b..3ca9fce759ea 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/Makefile
> +++ b/drivers/net/ethernet/mellanox/mlxsw/Makefile
> @@ -2,7 +2,7 @@
>  obj-$(CONFIG_MLXSW_CORE)	+= mlxsw_core.o
>  mlxsw_core-objs			:= core.o core_acl_flex_keys.o \
>  				   core_acl_flex_actions.o core_env.o \
> -				   core_linecards.o
> +				   core_linecards.o core_linecard_dev.o
>  mlxsw_core-$(CONFIG_MLXSW_CORE_HWMON) += core_hwmon.o
>  mlxsw_core-$(CONFIG_MLXSW_CORE_THERMAL) += core_thermal.o
>  obj-$(CONFIG_MLXSW_PCI)		+= mlxsw_pci.o
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
> index 61eb96b93889..831b0d3472c6 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
> @@ -3334,9 +3334,15 @@ static int __init mlxsw_core_module_init(void)
>  {
>  	int err;
>  
> +	err = mlxsw_linecard_driver_register();
> +	if (err)
> +		return err;
> +
>  	mlxsw_wq = alloc_workqueue(mlxsw_core_driver_name, 0, 0);
> -	if (!mlxsw_wq)
> -		return -ENOMEM;
> +	if (!mlxsw_wq) {
> +		err = -ENOMEM;
> +		goto err_alloc_workqueue;
> +	}
>  	mlxsw_owq = alloc_ordered_workqueue("%s_ordered", 0,
>  					    mlxsw_core_driver_name);
>  	if (!mlxsw_owq) {
> @@ -3347,6 +3353,8 @@ static int __init mlxsw_core_module_init(void)
>  
>  err_alloc_ordered_workqueue:
>  	destroy_workqueue(mlxsw_wq);
> +err_alloc_workqueue:
> +	mlxsw_linecard_driver_unregister();
>  	return err;
>  }
>  
> @@ -3354,6 +3362,7 @@ static void __exit mlxsw_core_module_exit(void)
>  {
>  	destroy_workqueue(mlxsw_owq);
>  	destroy_workqueue(mlxsw_wq);
> +	mlxsw_linecard_driver_unregister();
>  }
>  
>  module_init(mlxsw_core_module_init);
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
> index a3491ef2aa7e..b22db13fa547 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
> @@ -12,6 +12,7 @@
>  #include <linux/skbuff.h>
>  #include <linux/workqueue.h>
>  #include <linux/net_namespace.h>
> +#include <linux/auxiliary_bus.h>
>  #include <net/devlink.h>
>  
>  #include "trap.h"
> @@ -561,6 +562,8 @@ enum mlxsw_linecard_status_event_type {
>  	MLXSW_LINECARD_STATUS_EVENT_TYPE_UNPROVISION,
>  };
>  
> +struct mlxsw_linecard_bdev;
> +
>  struct mlxsw_linecard {
>  	u8 slot_index;
>  	struct mlxsw_linecards *linecards;
> @@ -575,6 +578,7 @@ struct mlxsw_linecard {
>  	   active:1;
>  	u16 hw_revision;
>  	u16 ini_version;
> +	struct mlxsw_linecard_bdev *bdev;
>  };
>  
>  struct mlxsw_linecard_types_info;
> @@ -614,4 +618,10 @@ void mlxsw_linecards_event_ops_unregister(struct mlxsw_core *mlxsw_core,
>  					  struct mlxsw_linecards_event_ops *ops,
>  					  void *priv);
>  
> +int mlxsw_linecard_bdev_add(struct mlxsw_linecard *linecard);
> +void mlxsw_linecard_bdev_del(struct mlxsw_linecard *linecard);
> +
> +int mlxsw_linecard_driver_register(void);
> +void mlxsw_linecard_driver_unregister(void);
> +
>  #endif
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
> new file mode 100644
> index 000000000000..bb6068b62df0
> --- /dev/null
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> +/* Copyright (c) 2022 NVIDIA Corporation and Mellanox Technologies. All rights reserved */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <linux/err.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/idr.h>
> +#include <linux/gfp.h>
> +#include <linux/slab.h>
> +#include <net/devlink.h>
> +#include "core.h"
> +
> +#define MLXSW_LINECARD_DEV_ID_NAME "lc"
> +
> +struct mlxsw_linecard_dev {
> +	struct mlxsw_linecard *linecard;
> +};
> +
> +struct mlxsw_linecard_bdev {
> +	struct auxiliary_device adev;
> +	struct mlxsw_linecard *linecard;
> +	struct mlxsw_linecard_dev *linecard_dev;
> +};
> +
> +static DEFINE_IDA(mlxsw_linecard_bdev_ida);
> +
> +static int mlxsw_linecard_bdev_id_alloc(void)
> +{
> +	return ida_alloc(&mlxsw_linecard_bdev_ida, GFP_KERNEL);
> +}
> +
> +static void mlxsw_linecard_bdev_id_free(int id)
> +{
> +	ida_free(&mlxsw_linecard_bdev_ida, id);
> +}
> +
> +static void mlxsw_linecard_bdev_release(struct device *device)
> +{
> +	struct auxiliary_device *adev =
> +			container_of(device, struct auxiliary_device, dev);
> +	struct mlxsw_linecard_bdev *linecard_bdev =
> +			container_of(adev, struct mlxsw_linecard_bdev, adev);
> +
> +	mlxsw_linecard_bdev_id_free(adev->id);
> +	kfree(linecard_bdev);
> +}
> +
> +int mlxsw_linecard_bdev_add(struct mlxsw_linecard *linecard)
> +{
> +	struct mlxsw_linecard_bdev *linecard_bdev;
> +	int err;
> +	int id;
> +
> +	id = mlxsw_linecard_bdev_id_alloc();
> +	if (id < 0)
> +		return id;
> +
> +	linecard_bdev = kzalloc(sizeof(*linecard_bdev), GFP_KERNEL);
> +	if (!linecard_bdev) {
> +		mlxsw_linecard_bdev_id_free(id);
> +		return -ENOMEM;
> +	}
> +	linecard_bdev->adev.id = id;
> +	linecard_bdev->adev.name = MLXSW_LINECARD_DEV_ID_NAME;
> +	linecard_bdev->adev.dev.release = mlxsw_linecard_bdev_release;
> +	linecard_bdev->adev.dev.parent = linecard->linecards->bus_info->dev;
> +	linecard_bdev->linecard = linecard;
> +
> +	err = auxiliary_device_init(&linecard_bdev->adev);
> +	if (err) {
> +		mlxsw_linecard_bdev_id_free(id);
> +		kfree(linecard_bdev);
> +		return err;
> +	}
> +
> +	err = auxiliary_device_add(&linecard_bdev->adev);
> +	if (err) {
> +		auxiliary_device_uninit(&linecard_bdev->adev);
> +		return err;
> +	}
> +
> +	linecard->bdev = linecard_bdev;
> +	return 0;
> +}
> +
> +void mlxsw_linecard_bdev_del(struct mlxsw_linecard *linecard)
> +{
> +	struct mlxsw_linecard_bdev *linecard_bdev = linecard->bdev;
> +
> +	if (!linecard_bdev)

This check is needed because of the invocation from
mlxsw_linecard_fini() ? If so, please add a comment. Something like:

/* Unprovisioned line cards do not have an auxiliary device. */

> +		return;
> +	auxiliary_device_delete(&linecard_bdev->adev);
> +	auxiliary_device_uninit(&linecard_bdev->adev);
> +	linecard->bdev = NULL;
> +}
> +
> +static const struct devlink_ops mlxsw_linecard_dev_devlink_ops = {
> +};
> +
> +static int mlxsw_linecard_bdev_probe(struct auxiliary_device *adev,
> +				     const struct auxiliary_device_id *id)
> +{
> +	struct mlxsw_linecard_bdev *linecard_bdev =
> +			container_of(adev, struct mlxsw_linecard_bdev, adev);
> +	struct mlxsw_linecard_dev *linecard_dev;
> +	struct devlink *devlink;
> +
> +	devlink = devlink_alloc(&mlxsw_linecard_dev_devlink_ops,
> +				sizeof(*linecard_dev), &adev->dev);
> +	if (!devlink)
> +		return -ENOMEM;
> +	linecard_dev = devlink_priv(devlink);
> +	linecard_dev->linecard = linecard_bdev->linecard;
> +	linecard_bdev->linecard_dev = linecard_dev;
> +
> +	devlink_register(devlink);
> +	return 0;
> +}
> +
> +static void mlxsw_linecard_bdev_remove(struct auxiliary_device *adev)
> +{
> +	struct mlxsw_linecard_bdev *linecard_bdev =
> +			container_of(adev, struct mlxsw_linecard_bdev, adev);
> +	struct devlink *devlink = priv_to_devlink(linecard_bdev->linecard_dev);
> +
> +	devlink_unregister(devlink);
> +	devlink_free(devlink);
> +}
> +
> +static const struct auxiliary_device_id mlxsw_linecard_bdev_id_table[] = {
> +	{ .name = KBUILD_MODNAME "." MLXSW_LINECARD_DEV_ID_NAME },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(auxiliary, mlxsw_linecard_bdev_id_table);
> +
> +static struct auxiliary_driver mlxsw_linecard_driver = {
> +	.name = MLXSW_LINECARD_DEV_ID_NAME,
> +	.probe = mlxsw_linecard_bdev_probe,
> +	.remove = mlxsw_linecard_bdev_remove,
> +	.id_table = mlxsw_linecard_bdev_id_table,
> +};
> +
> +int mlxsw_linecard_driver_register(void)
> +{
> +	return auxiliary_driver_register(&mlxsw_linecard_driver);
> +}
> +
> +void mlxsw_linecard_driver_unregister(void)
> +{
> +	auxiliary_driver_unregister(&mlxsw_linecard_driver);
> +}
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
> index 5c9869dcf674..43696d8badca 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
> @@ -232,6 +232,7 @@ mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
>  {
>  	struct mlxsw_linecards *linecards = linecard->linecards;
>  	const char *type;
> +	int err;
>  
>  	type = mlxsw_linecard_types_lookup(linecards, card_type);
>  	mlxsw_linecard_status_event_done(linecard,
> @@ -252,6 +253,14 @@ mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
>  	linecard->provisioned = true;
>  	linecard->hw_revision = hw_revision;
>  	linecard->ini_version = ini_version;
> +
> +	err = mlxsw_linecard_bdev_add(linecard);
> +	if (err) {
> +		linecard->provisioned = false;
> +		mlxsw_linecard_provision_fail(linecard);
> +		return err;
> +	}
> +
>  	devlink_linecard_provision_set(linecard->devlink_linecard, type);
>  	return 0;
>  }
> @@ -260,6 +269,7 @@ static void mlxsw_linecard_provision_clear(struct mlxsw_linecard *linecard)
>  {
>  	mlxsw_linecard_status_event_done(linecard,
>  					 MLXSW_LINECARD_STATUS_EVENT_TYPE_UNPROVISION);
> +	mlxsw_linecard_bdev_del(linecard);
>  	linecard->provisioned = false;
>  	devlink_linecard_provision_clear(linecard->devlink_linecard);
>  }
> @@ -885,6 +895,7 @@ static void mlxsw_linecard_fini(struct mlxsw_core *mlxsw_core,
>  	mlxsw_core_flush_owq();
>  	if (linecard->active)
>  		mlxsw_linecard_active_clear(linecard);
> +	mlxsw_linecard_bdev_del(linecard);
>  	devlink_linecard_destroy(linecard->devlink_linecard);
>  	mutex_destroy(&linecard->lock);
>  }
> -- 
> 2.35.3
> 

Re: [patch net-next v2 03/12] mlxsw: core_linecards: Introduce per line card auxiliary device

[Jiri Pirko]

Wed, Jul 20, 2022 at 10:45:30AM CEST, idosch@nvidia.com wrote:
>On Tue, Jul 19, 2022 at 08:48:38AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> In order to be eventually able to expose line card gearbox version and
>> possibility to flash FW, model the line card as a separate device on
>> auxiliary bus.
>
>What is the reason for adding the auxiliary device for the line card
>when it becomes 'provisioned' and not when it becomes 'active'? It needs
>to be explained in the commit message

Okay, done.


>
>> 
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>> v1->v2:
>> - added auxdev removal to mlxsw_linecard_fini()
>> - adjusted mlxsw_linecard_bdev_del() to cope with bdev == NULL
>> ---
>>  drivers/net/ethernet/mellanox/mlxsw/Kconfig   |   1 +
>>  drivers/net/ethernet/mellanox/mlxsw/Makefile  |   2 +-
>>  drivers/net/ethernet/mellanox/mlxsw/core.c    |  13 +-
>>  drivers/net/ethernet/mellanox/mlxsw/core.h    |  10 ++
>>  .../mellanox/mlxsw/core_linecard_dev.c        | 155 ++++++++++++++++++
>>  .../ethernet/mellanox/mlxsw/core_linecards.c  |  11 ++
>>  6 files changed, 189 insertions(+), 3 deletions(-)
>>  create mode 100644 drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
>> 
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
>> index 4683312861ac..a510bf2cff2f 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
>> @@ -7,6 +7,7 @@ config MLXSW_CORE
>>  	tristate "Mellanox Technologies Switch ASICs support"
>>  	select NET_DEVLINK
>>  	select MLXFW
>> +	select AUXILIARY_BUS
>>  	help
>>  	  This driver supports Mellanox Technologies Switch ASICs family.
>>  
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/Makefile b/drivers/net/ethernet/mellanox/mlxsw/Makefile
>> index c2d6d64ffe4b..3ca9fce759ea 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/Makefile
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/Makefile
>> @@ -2,7 +2,7 @@
>>  obj-$(CONFIG_MLXSW_CORE)	+= mlxsw_core.o
>>  mlxsw_core-objs			:= core.o core_acl_flex_keys.o \
>>  				   core_acl_flex_actions.o core_env.o \
>> -				   core_linecards.o
>> +				   core_linecards.o core_linecard_dev.o
>>  mlxsw_core-$(CONFIG_MLXSW_CORE_HWMON) += core_hwmon.o
>>  mlxsw_core-$(CONFIG_MLXSW_CORE_THERMAL) += core_thermal.o
>>  obj-$(CONFIG_MLXSW_PCI)		+= mlxsw_pci.o
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
>> index 61eb96b93889..831b0d3472c6 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/core.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
>> @@ -3334,9 +3334,15 @@ static int __init mlxsw_core_module_init(void)
>>  {
>>  	int err;
>>  
>> +	err = mlxsw_linecard_driver_register();
>> +	if (err)
>> +		return err;
>> +
>>  	mlxsw_wq = alloc_workqueue(mlxsw_core_driver_name, 0, 0);
>> -	if (!mlxsw_wq)
>> -		return -ENOMEM;
>> +	if (!mlxsw_wq) {
>> +		err = -ENOMEM;
>> +		goto err_alloc_workqueue;
>> +	}
>>  	mlxsw_owq = alloc_ordered_workqueue("%s_ordered", 0,
>>  					    mlxsw_core_driver_name);
>>  	if (!mlxsw_owq) {
>> @@ -3347,6 +3353,8 @@ static int __init mlxsw_core_module_init(void)
>>  
>>  err_alloc_ordered_workqueue:
>>  	destroy_workqueue(mlxsw_wq);
>> +err_alloc_workqueue:
>> +	mlxsw_linecard_driver_unregister();
>>  	return err;
>>  }
>>  
>> @@ -3354,6 +3362,7 @@ static void __exit mlxsw_core_module_exit(void)
>>  {
>>  	destroy_workqueue(mlxsw_owq);
>>  	destroy_workqueue(mlxsw_wq);
>> +	mlxsw_linecard_driver_unregister();
>>  }
>>  
>>  module_init(mlxsw_core_module_init);
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
>> index a3491ef2aa7e..b22db13fa547 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/core.h
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
>> @@ -12,6 +12,7 @@
>>  #include <linux/skbuff.h>
>>  #include <linux/workqueue.h>
>>  #include <linux/net_namespace.h>
>> +#include <linux/auxiliary_bus.h>
>>  #include <net/devlink.h>
>>  
>>  #include "trap.h"
>> @@ -561,6 +562,8 @@ enum mlxsw_linecard_status_event_type {
>>  	MLXSW_LINECARD_STATUS_EVENT_TYPE_UNPROVISION,
>>  };
>>  
>> +struct mlxsw_linecard_bdev;
>> +
>>  struct mlxsw_linecard {
>>  	u8 slot_index;
>>  	struct mlxsw_linecards *linecards;
>> @@ -575,6 +578,7 @@ struct mlxsw_linecard {
>>  	   active:1;
>>  	u16 hw_revision;
>>  	u16 ini_version;
>> +	struct mlxsw_linecard_bdev *bdev;
>>  };
>>  
>>  struct mlxsw_linecard_types_info;
>> @@ -614,4 +618,10 @@ void mlxsw_linecards_event_ops_unregister(struct mlxsw_core *mlxsw_core,
>>  					  struct mlxsw_linecards_event_ops *ops,
>>  					  void *priv);
>>  
>> +int mlxsw_linecard_bdev_add(struct mlxsw_linecard *linecard);
>> +void mlxsw_linecard_bdev_del(struct mlxsw_linecard *linecard);
>> +
>> +int mlxsw_linecard_driver_register(void);
>> +void mlxsw_linecard_driver_unregister(void);
>> +
>>  #endif
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
>> new file mode 100644
>> index 000000000000..bb6068b62df0
>> --- /dev/null
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
>> @@ -0,0 +1,155 @@
>> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
>> +/* Copyright (c) 2022 NVIDIA Corporation and Mellanox Technologies. All rights reserved */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/err.h>
>> +#include <linux/types.h>
>> +#include <linux/err.h>
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/idr.h>
>> +#include <linux/gfp.h>
>> +#include <linux/slab.h>
>> +#include <net/devlink.h>
>> +#include "core.h"
>> +
>> +#define MLXSW_LINECARD_DEV_ID_NAME "lc"
>> +
>> +struct mlxsw_linecard_dev {
>> +	struct mlxsw_linecard *linecard;
>> +};
>> +
>> +struct mlxsw_linecard_bdev {
>> +	struct auxiliary_device adev;
>> +	struct mlxsw_linecard *linecard;
>> +	struct mlxsw_linecard_dev *linecard_dev;
>> +};
>> +
>> +static DEFINE_IDA(mlxsw_linecard_bdev_ida);
>> +
>> +static int mlxsw_linecard_bdev_id_alloc(void)
>> +{
>> +	return ida_alloc(&mlxsw_linecard_bdev_ida, GFP_KERNEL);
>> +}
>> +
>> +static void mlxsw_linecard_bdev_id_free(int id)
>> +{
>> +	ida_free(&mlxsw_linecard_bdev_ida, id);
>> +}
>> +
>> +static void mlxsw_linecard_bdev_release(struct device *device)
>> +{
>> +	struct auxiliary_device *adev =
>> +			container_of(device, struct auxiliary_device, dev);
>> +	struct mlxsw_linecard_bdev *linecard_bdev =
>> +			container_of(adev, struct mlxsw_linecard_bdev, adev);
>> +
>> +	mlxsw_linecard_bdev_id_free(adev->id);
>> +	kfree(linecard_bdev);
>> +}
>> +
>> +int mlxsw_linecard_bdev_add(struct mlxsw_linecard *linecard)
>> +{
>> +	struct mlxsw_linecard_bdev *linecard_bdev;
>> +	int err;
>> +	int id;
>> +
>> +	id = mlxsw_linecard_bdev_id_alloc();
>> +	if (id < 0)
>> +		return id;
>> +
>> +	linecard_bdev = kzalloc(sizeof(*linecard_bdev), GFP_KERNEL);
>> +	if (!linecard_bdev) {
>> +		mlxsw_linecard_bdev_id_free(id);
>> +		return -ENOMEM;
>> +	}
>> +	linecard_bdev->adev.id = id;
>> +	linecard_bdev->adev.name = MLXSW_LINECARD_DEV_ID_NAME;
>> +	linecard_bdev->adev.dev.release = mlxsw_linecard_bdev_release;
>> +	linecard_bdev->adev.dev.parent = linecard->linecards->bus_info->dev;
>> +	linecard_bdev->linecard = linecard;
>> +
>> +	err = auxiliary_device_init(&linecard_bdev->adev);
>> +	if (err) {
>> +		mlxsw_linecard_bdev_id_free(id);
>> +		kfree(linecard_bdev);
>> +		return err;
>> +	}
>> +
>> +	err = auxiliary_device_add(&linecard_bdev->adev);
>> +	if (err) {
>> +		auxiliary_device_uninit(&linecard_bdev->adev);
>> +		return err;
>> +	}
>> +
>> +	linecard->bdev = linecard_bdev;
>> +	return 0;
>> +}
>> +
>> +void mlxsw_linecard_bdev_del(struct mlxsw_linecard *linecard)
>> +{
>> +	struct mlxsw_linecard_bdev *linecard_bdev = linecard->bdev;
>> +
>> +	if (!linecard_bdev)
>
>This check is needed because of the invocation from
>mlxsw_linecard_fini() ? If so, please add a comment. Something like:
>
>/* Unprovisioned line cards do not have an auxiliary device. */

Okay, added.


>
>> +		return;
>> +	auxiliary_device_delete(&linecard_bdev->adev);
>> +	auxiliary_device_uninit(&linecard_bdev->adev);
>> +	linecard->bdev = NULL;
>> +}
>> +
>> +static const struct devlink_ops mlxsw_linecard_dev_devlink_ops = {
>> +};
>> +
>> +static int mlxsw_linecard_bdev_probe(struct auxiliary_device *adev,
>> +				     const struct auxiliary_device_id *id)
>> +{
>> +	struct mlxsw_linecard_bdev *linecard_bdev =
>> +			container_of(adev, struct mlxsw_linecard_bdev, adev);
>> +	struct mlxsw_linecard_dev *linecard_dev;
>> +	struct devlink *devlink;
>> +
>> +	devlink = devlink_alloc(&mlxsw_linecard_dev_devlink_ops,
>> +				sizeof(*linecard_dev), &adev->dev);
>> +	if (!devlink)
>> +		return -ENOMEM;
>> +	linecard_dev = devlink_priv(devlink);
>> +	linecard_dev->linecard = linecard_bdev->linecard;
>> +	linecard_bdev->linecard_dev = linecard_dev;
>> +
>> +	devlink_register(devlink);
>> +	return 0;
>> +}
>> +
>> +static void mlxsw_linecard_bdev_remove(struct auxiliary_device *adev)
>> +{
>> +	struct mlxsw_linecard_bdev *linecard_bdev =
>> +			container_of(adev, struct mlxsw_linecard_bdev, adev);
>> +	struct devlink *devlink = priv_to_devlink(linecard_bdev->linecard_dev);
>> +
>> +	devlink_unregister(devlink);
>> +	devlink_free(devlink);
>> +}
>> +
>> +static const struct auxiliary_device_id mlxsw_linecard_bdev_id_table[] = {
>> +	{ .name = KBUILD_MODNAME "." MLXSW_LINECARD_DEV_ID_NAME },
>> +	{},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(auxiliary, mlxsw_linecard_bdev_id_table);
>> +
>> +static struct auxiliary_driver mlxsw_linecard_driver = {
>> +	.name = MLXSW_LINECARD_DEV_ID_NAME,
>> +	.probe = mlxsw_linecard_bdev_probe,
>> +	.remove = mlxsw_linecard_bdev_remove,
>> +	.id_table = mlxsw_linecard_bdev_id_table,
>> +};
>> +
>> +int mlxsw_linecard_driver_register(void)
>> +{
>> +	return auxiliary_driver_register(&mlxsw_linecard_driver);
>> +}
>> +
>> +void mlxsw_linecard_driver_unregister(void)
>> +{
>> +	auxiliary_driver_unregister(&mlxsw_linecard_driver);
>> +}
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
>> index 5c9869dcf674..43696d8badca 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
>> @@ -232,6 +232,7 @@ mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
>>  {
>>  	struct mlxsw_linecards *linecards = linecard->linecards;
>>  	const char *type;
>> +	int err;
>>  
>>  	type = mlxsw_linecard_types_lookup(linecards, card_type);
>>  	mlxsw_linecard_status_event_done(linecard,
>> @@ -252,6 +253,14 @@ mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
>>  	linecard->provisioned = true;
>>  	linecard->hw_revision = hw_revision;
>>  	linecard->ini_version = ini_version;
>> +
>> +	err = mlxsw_linecard_bdev_add(linecard);
>> +	if (err) {
>> +		linecard->provisioned = false;
>> +		mlxsw_linecard_provision_fail(linecard);
>> +		return err;
>> +	}
>> +
>>  	devlink_linecard_provision_set(linecard->devlink_linecard, type);
>>  	return 0;
>>  }
>> @@ -260,6 +269,7 @@ static void mlxsw_linecard_provision_clear(struct mlxsw_linecard *linecard)
>>  {
>>  	mlxsw_linecard_status_event_done(linecard,
>>  					 MLXSW_LINECARD_STATUS_EVENT_TYPE_UNPROVISION);
>> +	mlxsw_linecard_bdev_del(linecard);
>>  	linecard->provisioned = false;
>>  	devlink_linecard_provision_clear(linecard->devlink_linecard);
>>  }
>> @@ -885,6 +895,7 @@ static void mlxsw_linecard_fini(struct mlxsw_core *mlxsw_core,
>>  	mlxsw_core_flush_owq();
>>  	if (linecard->active)
>>  		mlxsw_linecard_active_clear(linecard);
>> +	mlxsw_linecard_bdev_del(linecard);
>>  	devlink_linecard_destroy(linecard->devlink_linecard);
>>  	mutex_destroy(&linecard->lock);
>>  }
>> -- 
>> 2.35.3
>> 

[patch net-next v2 04/12] mlxsw: core_linecard_dev: Set nested devlink relationship for a line card

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

For newly created line card device devlink instance, set the
relationship with the parent line card object.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
index bb6068b62df0..f41662936a2b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
@@ -105,6 +105,7 @@ static int mlxsw_linecard_bdev_probe(struct auxiliary_device *adev,
 {
 	struct mlxsw_linecard_bdev *linecard_bdev =
 			container_of(adev, struct mlxsw_linecard_bdev, adev);
+	struct mlxsw_linecard *linecard = linecard_bdev->linecard;
 	struct mlxsw_linecard_dev *linecard_dev;
 	struct devlink *devlink;
 
@@ -117,6 +118,7 @@ static int mlxsw_linecard_bdev_probe(struct auxiliary_device *adev,
 	linecard_bdev->linecard_dev = linecard_dev;
 
 	devlink_register(devlink);
+	devlink_linecard_nested_dl_set(linecard->devlink_linecard, devlink);
 	return 0;
 }
 
@@ -125,7 +127,9 @@ static void mlxsw_linecard_bdev_remove(struct auxiliary_device *adev)
 	struct mlxsw_linecard_bdev *linecard_bdev =
 			container_of(adev, struct mlxsw_linecard_bdev, adev);
 	struct devlink *devlink = priv_to_devlink(linecard_bdev->linecard_dev);
+	struct mlxsw_linecard *linecard = linecard_bdev->linecard;
 
+	devlink_linecard_nested_dl_set(linecard->devlink_linecard, NULL);
 	devlink_unregister(devlink);
 	devlink_free(devlink);
 }
-- 
2.35.3

Re: [patch net-next v2 04/12] mlxsw: core_linecard_dev: Set nested devlink relationship for a line card

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:39AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> For newly created line card device devlink instance, set the
> relationship with the parent line card object.

Can be squashed to previous patch

[patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Implement info_get() to expose HW revision of a linecard and loaded INI
version.

Example:

$ devlink dev info auxiliary/mlxsw_core.lc.0
auxiliary/mlxsw_core.lc.0:
  versions:
      fixed:
        hw.revision 0
      running:
        ini.version 4

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/networking/devlink/mlxsw.rst    | 18 ++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  4 +++
 .../mellanox/mlxsw/core_linecard_dev.c        | 11 ++++++++
 .../ethernet/mellanox/mlxsw/core_linecards.c  | 28 +++++++++++++++++++
 4 files changed, 61 insertions(+)

diff --git a/Documentation/networking/devlink/mlxsw.rst b/Documentation/networking/devlink/mlxsw.rst
index cf857cb4ba8f..aededcf68df4 100644
--- a/Documentation/networking/devlink/mlxsw.rst
+++ b/Documentation/networking/devlink/mlxsw.rst
@@ -58,6 +58,24 @@ The ``mlxsw`` driver reports the following versions
      - running
      - Three digit firmware version
 
+Line card auxiliary device info versions
+========================================
+
+The ``mlxsw`` driver reports the following versions for line card auxiliary device
+
+.. list-table:: devlink info versions implemented
+   :widths: 5 5 90
+
+   * - Name
+     - Type
+     - Description
+   * - ``hw.revision``
+     - fixed
+     - The hardware revision for this line card
+   * - ``ini.version``
+     - running
+     - Version of line card INI loaded
+
 Driver-specific Traps
 =====================
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index b22db13fa547..87c58b512536 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -599,6 +599,10 @@ mlxsw_linecard_get(struct mlxsw_linecards *linecards, u8 slot_index)
 	return &linecards->linecards[slot_index - 1];
 }
 
+int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
+				    struct devlink_info_req *req,
+				    struct netlink_ext_ack *extack);
+
 int mlxsw_linecards_init(struct mlxsw_core *mlxsw_core,
 			 const struct mlxsw_bus_info *bus_info);
 void mlxsw_linecards_fini(struct mlxsw_core *mlxsw_core);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
index f41662936a2b..d0ecefee587b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
@@ -97,7 +97,18 @@ void mlxsw_linecard_bdev_del(struct mlxsw_linecard *linecard)
 	linecard->bdev = NULL;
 }
 
+static int mlxsw_linecard_dev_devlink_info_get(struct devlink *devlink,
+					       struct devlink_info_req *req,
+					       struct netlink_ext_ack *extack)
+{
+	struct mlxsw_linecard_dev *linecard_dev = devlink_priv(devlink);
+	struct mlxsw_linecard *linecard = linecard_dev->linecard;
+
+	return mlxsw_linecard_devlink_info_get(linecard, req, extack);
+}
+
 static const struct devlink_ops mlxsw_linecard_dev_devlink_ops = {
+	.info_get			= mlxsw_linecard_dev_devlink_info_get,
 };
 
 static int mlxsw_linecard_bdev_probe(struct auxiliary_device *adev,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
index 43696d8badca..c427e07b25dd 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
@@ -226,6 +226,34 @@ void mlxsw_linecards_event_ops_unregister(struct mlxsw_core *mlxsw_core,
 }
 EXPORT_SYMBOL(mlxsw_linecards_event_ops_unregister);
 
+int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
+				    struct devlink_info_req *req,
+				    struct netlink_ext_ack *extack)
+{
+	char buf[32];
+	int err;
+
+	mutex_lock(&linecard->lock);
+	if (WARN_ON(!linecard->provisioned)) {
+		err = 0;
+		goto unlock;
+	}
+
+	sprintf(buf, "%d", linecard->hw_revision);
+	err = devlink_info_version_fixed_put(req, "hw.revision", buf);
+	if (err)
+		goto unlock;
+
+	sprintf(buf, "%d", linecard->ini_version);
+	err = devlink_info_version_running_put(req, "ini.version", buf);
+	if (err)
+		goto unlock;
+
+unlock:
+	mutex_unlock(&linecard->lock);
+	return err;
+}
+
 static int
 mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
 			     u16 hw_revision, u16 ini_version)
-- 
2.35.3

Re: [patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:40AM +0200, Jiri Pirko wrote:
> +int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
> +				    struct devlink_info_req *req,
> +				    struct netlink_ext_ack *extack)
> +{
> +	char buf[32];
> +	int err;
> +
> +	mutex_lock(&linecard->lock);
> +	if (WARN_ON(!linecard->provisioned)) {
> +		err = 0;

Why not:

err = -EINVAL;

?

> +		goto unlock;
> +	}
> +
> +	sprintf(buf, "%d", linecard->hw_revision);
> +	err = devlink_info_version_fixed_put(req, "hw.revision", buf);
> +	if (err)
> +		goto unlock;
> +
> +	sprintf(buf, "%d", linecard->ini_version);
> +	err = devlink_info_version_running_put(req, "ini.version", buf);
> +	if (err)
> +		goto unlock;
> +
> +unlock:
> +	mutex_unlock(&linecard->lock);
> +	return err;
> +}
> +
>  static int
>  mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
>  			     u16 hw_revision, u16 ini_version)
> -- 
> 2.35.3
> 

Re: [patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Jiri Pirko]

Wed, Jul 20, 2022 at 10:56:35AM CEST, idosch@nvidia.com wrote:
>On Tue, Jul 19, 2022 at 08:48:40AM +0200, Jiri Pirko wrote:
>> +int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
>> +				    struct devlink_info_req *req,
>> +				    struct netlink_ext_ack *extack)
>> +{
>> +	char buf[32];
>> +	int err;
>> +
>> +	mutex_lock(&linecard->lock);
>> +	if (WARN_ON(!linecard->provisioned)) {
>> +		err = 0;
>
>Why not:
>
>err = -EINVAL;
>
>?

Well, a) this should not happen. No need to push error to the user for
this as the rest of the info message is still fine.


>
>> +		goto unlock;
>> +	}
>> +
>> +	sprintf(buf, "%d", linecard->hw_revision);
>> +	err = devlink_info_version_fixed_put(req, "hw.revision", buf);
>> +	if (err)
>> +		goto unlock;
>> +
>> +	sprintf(buf, "%d", linecard->ini_version);
>> +	err = devlink_info_version_running_put(req, "ini.version", buf);
>> +	if (err)
>> +		goto unlock;
>> +
>> +unlock:
>> +	mutex_unlock(&linecard->lock);
>> +	return err;
>> +}
>> +
>>  static int
>>  mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
>>  			     u16 hw_revision, u16 ini_version)
>> -- 
>> 2.35.3
>> 

Re: [patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Ido Schimmel]

On Wed, Jul 20, 2022 at 02:27:40PM +0200, Jiri Pirko wrote:
> Wed, Jul 20, 2022 at 10:56:35AM CEST, idosch@nvidia.com wrote:
> >On Tue, Jul 19, 2022 at 08:48:40AM +0200, Jiri Pirko wrote:
> >> +int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
> >> +				    struct devlink_info_req *req,
> >> +				    struct netlink_ext_ack *extack)
> >> +{
> >> +	char buf[32];
> >> +	int err;
> >> +
> >> +	mutex_lock(&linecard->lock);
> >> +	if (WARN_ON(!linecard->provisioned)) {
> >> +		err = 0;
> >
> >Why not:
> >
> >err = -EINVAL;
> >
> >?
> 
> Well, a) this should not happen. No need to push error to the user for
> this as the rest of the info message is still fine.

Not sure what you mean by "the rest of the info message is still fine".
Which info message? If the line card is not provisioned, then it
shouldn't even have a devlink instance and it will not appear in
"devlink dev info" dump.

I still do not understand why this error is severe enough to print a
WARNING to the kernel log, but not emit an error code to user space.

> 
> 
> >
> >> +		goto unlock;
> >> +	}
> >> +
> >> +	sprintf(buf, "%d", linecard->hw_revision);
> >> +	err = devlink_info_version_fixed_put(req, "hw.revision", buf);
> >> +	if (err)
> >> +		goto unlock;
> >> +
> >> +	sprintf(buf, "%d", linecard->ini_version);
> >> +	err = devlink_info_version_running_put(req, "ini.version", buf);
> >> +	if (err)
> >> +		goto unlock;
> >> +
> >> +unlock:
> >> +	mutex_unlock(&linecard->lock);
> >> +	return err;
> >> +}
> >> +
> >>  static int
> >>  mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
> >>  			     u16 hw_revision, u16 ini_version)
> >> -- 
> >> 2.35.3
> >> 

Re: [patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Ido Schimmel]

On Wed, Jul 20, 2022 at 03:43:17PM +0300, Ido Schimmel wrote:
> On Wed, Jul 20, 2022 at 02:27:40PM +0200, Jiri Pirko wrote:
> > Wed, Jul 20, 2022 at 10:56:35AM CEST, idosch@nvidia.com wrote:
> > >On Tue, Jul 19, 2022 at 08:48:40AM +0200, Jiri Pirko wrote:
> > >> +int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
> > >> +				    struct devlink_info_req *req,
> > >> +				    struct netlink_ext_ack *extack)
> > >> +{
> > >> +	char buf[32];
> > >> +	int err;
> > >> +
> > >> +	mutex_lock(&linecard->lock);
> > >> +	if (WARN_ON(!linecard->provisioned)) {
> > >> +		err = 0;
> > >
> > >Why not:
> > >
> > >err = -EINVAL;
> > >
> > >?
> > 
> > Well, a) this should not happen. No need to push error to the user for
> > this as the rest of the info message is still fine.
> 
> Not sure what you mean by "the rest of the info message is still fine".
> Which info message? If the line card is not provisioned, then it
> shouldn't even have a devlink instance and it will not appear in
> "devlink dev info" dump.

How about returning '-EOPNOTSUPP'? Looks like devlink will skip it in a
dump

Re: [patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Jiri Pirko]

Wed, Jul 20, 2022 at 02:50:09PM CEST, idosch@nvidia.com wrote:
>On Wed, Jul 20, 2022 at 03:43:17PM +0300, Ido Schimmel wrote:
>> On Wed, Jul 20, 2022 at 02:27:40PM +0200, Jiri Pirko wrote:
>> > Wed, Jul 20, 2022 at 10:56:35AM CEST, idosch@nvidia.com wrote:
>> > >On Tue, Jul 19, 2022 at 08:48:40AM +0200, Jiri Pirko wrote:
>> > >> +int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
>> > >> +				    struct devlink_info_req *req,
>> > >> +				    struct netlink_ext_ack *extack)
>> > >> +{
>> > >> +	char buf[32];
>> > >> +	int err;
>> > >> +
>> > >> +	mutex_lock(&linecard->lock);
>> > >> +	if (WARN_ON(!linecard->provisioned)) {
>> > >> +		err = 0;
>> > >
>> > >Why not:
>> > >
>> > >err = -EINVAL;
>> > >
>> > >?
>> > 
>> > Well, a) this should not happen. No need to push error to the user for
>> > this as the rest of the info message is still fine.
>> 
>> Not sure what you mean by "the rest of the info message is still fine".
>> Which info message? If the line card is not provisioned, then it
>> shouldn't even have a devlink instance and it will not appear in
>> "devlink dev info" dump.
>
>How about returning '-EOPNOTSUPP'? Looks like devlink will skip it in a
>dump

Okay.

Re: [patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Jiri Pirko]

Wed, Jul 20, 2022 at 02:43:17PM CEST, idosch@nvidia.com wrote:
>On Wed, Jul 20, 2022 at 02:27:40PM +0200, Jiri Pirko wrote:
>> Wed, Jul 20, 2022 at 10:56:35AM CEST, idosch@nvidia.com wrote:
>> >On Tue, Jul 19, 2022 at 08:48:40AM +0200, Jiri Pirko wrote:
>> >> +int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
>> >> +				    struct devlink_info_req *req,
>> >> +				    struct netlink_ext_ack *extack)
>> >> +{
>> >> +	char buf[32];
>> >> +	int err;
>> >> +
>> >> +	mutex_lock(&linecard->lock);
>> >> +	if (WARN_ON(!linecard->provisioned)) {
>> >> +		err = 0;
>> >
>> >Why not:
>> >
>> >err = -EINVAL;
>> >
>> >?
>> 
>> Well, a) this should not happen. No need to push error to the user for
>> this as the rest of the info message is still fine.
>
>Not sure what you mean by "the rest of the info message is still fine".
>Which info message? If the line card is not provisioned, then it
>shouldn't even have a devlink instance and it will not appear in
>"devlink dev info" dump.
>
>I still do not understand why this error is severe enough to print a
>WARNING to the kernel log, but not emit an error code to user space.

As I wrote, WARN_ON was a leftover.

>
>> 
>> 
>> >
>> >> +		goto unlock;
>> >> +	}
>> >> +
>> >> +	sprintf(buf, "%d", linecard->hw_revision);
>> >> +	err = devlink_info_version_fixed_put(req, "hw.revision", buf);
>> >> +	if (err)
>> >> +		goto unlock;
>> >> +
>> >> +	sprintf(buf, "%d", linecard->ini_version);
>> >> +	err = devlink_info_version_running_put(req, "ini.version", buf);
>> >> +	if (err)
>> >> +		goto unlock;
>> >> +
>> >> +unlock:
>> >> +	mutex_unlock(&linecard->lock);
>> >> +	return err;
>> >> +}
>> >> +
>> >>  static int
>> >>  mlxsw_linecard_provision_set(struct mlxsw_linecard *linecard, u8 card_type,
>> >>  			     u16 hw_revision, u16 ini_version)
>> >> -- 
>> >> 2.35.3
>> >> 

Re: [patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Ido Schimmel]

On Wed, Jul 20, 2022 at 04:59:03PM +0200, Jiri Pirko wrote:
> Wed, Jul 20, 2022 at 02:43:17PM CEST, idosch@nvidia.com wrote:
> >On Wed, Jul 20, 2022 at 02:27:40PM +0200, Jiri Pirko wrote:
> >> Wed, Jul 20, 2022 at 10:56:35AM CEST, idosch@nvidia.com wrote:
> >> >On Tue, Jul 19, 2022 at 08:48:40AM +0200, Jiri Pirko wrote:
> >> >> +int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
> >> >> +				    struct devlink_info_req *req,
> >> >> +				    struct netlink_ext_ack *extack)
> >> >> +{
> >> >> +	char buf[32];
> >> >> +	int err;
> >> >> +
> >> >> +	mutex_lock(&linecard->lock);
> >> >> +	if (WARN_ON(!linecard->provisioned)) {
> >> >> +		err = 0;
> >> >
> >> >Why not:
> >> >
> >> >err = -EINVAL;
> >> >
> >> >?
> >> 
> >> Well, a) this should not happen. No need to push error to the user for
> >> this as the rest of the info message is still fine.
> >
> >Not sure what you mean by "the rest of the info message is still fine".
> >Which info message? If the line card is not provisioned, then it
> >shouldn't even have a devlink instance and it will not appear in
> >"devlink dev info" dump.
> >
> >I still do not understand why this error is severe enough to print a
> >WARNING to the kernel log, but not emit an error code to user space.
> 
> As I wrote, WARN_ON was a leftover.

It was a leftover in patch #10 where you checked '!linecard->ready'.
Here I think it's actually correct because it shouldn't happen unless
I'm missing something

Re: [patch net-next v2 05/12] mlxsw: core_linecards: Expose HW revision and INI version

[Jiri Pirko]

Wed, Jul 20, 2022 at 05:01:12PM CEST, idosch@nvidia.com wrote:
>On Wed, Jul 20, 2022 at 04:59:03PM +0200, Jiri Pirko wrote:
>> Wed, Jul 20, 2022 at 02:43:17PM CEST, idosch@nvidia.com wrote:
>> >On Wed, Jul 20, 2022 at 02:27:40PM +0200, Jiri Pirko wrote:
>> >> Wed, Jul 20, 2022 at 10:56:35AM CEST, idosch@nvidia.com wrote:
>> >> >On Tue, Jul 19, 2022 at 08:48:40AM +0200, Jiri Pirko wrote:
>> >> >> +int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
>> >> >> +				    struct devlink_info_req *req,
>> >> >> +				    struct netlink_ext_ack *extack)
>> >> >> +{
>> >> >> +	char buf[32];
>> >> >> +	int err;
>> >> >> +
>> >> >> +	mutex_lock(&linecard->lock);
>> >> >> +	if (WARN_ON(!linecard->provisioned)) {
>> >> >> +		err = 0;
>> >> >
>> >> >Why not:
>> >> >
>> >> >err = -EINVAL;
>> >> >
>> >> >?
>> >> 
>> >> Well, a) this should not happen. No need to push error to the user for
>> >> this as the rest of the info message is still fine.
>> >
>> >Not sure what you mean by "the rest of the info message is still fine".
>> >Which info message? If the line card is not provisioned, then it
>> >shouldn't even have a devlink instance and it will not appear in
>> >"devlink dev info" dump.
>> >
>> >I still do not understand why this error is severe enough to print a
>> >WARNING to the kernel log, but not emit an error code to user space.
>> 
>> As I wrote, WARN_ON was a leftover.
>
>It was a leftover in patch #10 where you checked '!linecard->ready'.
>Here I think it's actually correct because it shouldn't happen unless
>I'm missing something

Correct, I confused myself :)

[patch net-next v2 06/12] mlxsw: reg: Extend MDDQ by device_info

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Extend existing MDDQ register by possibility to query information about
devices residing on a line card.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h | 83 ++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 17ce28e65464..76caf06b17d6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -11297,7 +11297,11 @@ MLXSW_ITEM32(reg, mddq, sie, 0x00, 31, 1);
 
 enum mlxsw_reg_mddq_query_type {
 	MLXSW_REG_MDDQ_QUERY_TYPE_SLOT_INFO = 1,
-	MLXSW_REG_MDDQ_QUERY_TYPE_SLOT_NAME = 3,
+	MLXSW_REG_MDDQ_QUERY_TYPE_DEVICE_INFO, /* If there are no devices
+						* on the slot, data_valid
+						* will be '0'.
+						*/
+	MLXSW_REG_MDDQ_QUERY_TYPE_SLOT_NAME,
 };
 
 /* reg_mddq_query_type
@@ -11311,6 +11315,28 @@ MLXSW_ITEM32(reg, mddq, query_type, 0x00, 16, 8);
  */
 MLXSW_ITEM32(reg, mddq, slot_index, 0x00, 0, 4);
 
+/* reg_mddq_response_msg_seq
+ * Response message sequential number. For a specific request, the response
+ * message sequential number is the following one. In addition, the last
+ * message should be 0.
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mddq, response_msg_seq, 0x04, 16, 8);
+
+/* reg_mddq_request_msg_seq
+ * Request message sequential number.
+ * The first message number should be 0.
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mddq, request_msg_seq, 0x04, 0, 8);
+
+/* reg_mddq_data_valid
+ * If set, the data in the data field is valid and contain the information
+ * for the queried index.
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mddq, data_valid, 0x08, 31, 1);
+
 /* reg_mddq_slot_info_provisioned
  * If set, the INI file is applied and the card is provisioned.
  * Access: RO
@@ -11397,6 +11423,61 @@ mlxsw_reg_mddq_slot_info_unpack(const char *payload, u8 *p_slot_index,
 	*p_card_type = mlxsw_reg_mddq_slot_info_card_type_get(payload);
 }
 
+/* reg_mddq_device_info_flash_owner
+ * If set, the device is the flash owner. Otherwise, a shared flash
+ * is used by this device (another device is the flash owner).
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mddq, device_info_flash_owner, 0x10, 30, 1);
+
+/* reg_mddq_device_info_device_index
+ * Device index. The first device should number 0.
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mddq, device_info_device_index, 0x10, 0, 8);
+
+/* reg_mddq_device_info_fw_major
+ * Major FW version number.
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mddq, device_info_fw_major, 0x14, 16, 16);
+
+/* reg_mddq_device_info_fw_minor
+ * Minor FW version number.
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mddq, device_info_fw_minor, 0x18, 16, 16);
+
+/* reg_mddq_device_info_fw_sub_minor
+ * Sub-minor FW version number.
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mddq, device_info_fw_sub_minor, 0x18, 0, 16);
+
+static inline void
+mlxsw_reg_mddq_device_info_pack(char *payload, u8 slot_index,
+				u8 request_msg_seq)
+{
+	__mlxsw_reg_mddq_pack(payload, slot_index,
+			      MLXSW_REG_MDDQ_QUERY_TYPE_DEVICE_INFO);
+	mlxsw_reg_mddq_request_msg_seq_set(payload, request_msg_seq);
+}
+
+static inline void
+mlxsw_reg_mddq_device_info_unpack(const char *payload, u8 *p_response_msg_seq,
+				  bool *p_data_valid, bool *p_flash_owner,
+				  u8 *p_device_index, u16 *p_fw_major,
+				  u16 *p_fw_minor, u16 *p_fw_sub_minor)
+{
+	*p_response_msg_seq = mlxsw_reg_mddq_response_msg_seq_get(payload);
+	*p_data_valid = mlxsw_reg_mddq_data_valid_get(payload);
+	*p_flash_owner = mlxsw_reg_mddq_device_info_flash_owner_get(payload);
+	*p_device_index = mlxsw_reg_mddq_device_info_device_index_get(payload);
+	*p_fw_major = mlxsw_reg_mddq_device_info_fw_major_get(payload);
+	*p_fw_minor = mlxsw_reg_mddq_device_info_fw_minor_get(payload);
+	*p_fw_sub_minor = mlxsw_reg_mddq_device_info_fw_sub_minor_get(payload);
+}
+
 #define MLXSW_REG_MDDQ_SLOT_ASCII_NAME_LEN 20
 
 /* reg_mddq_slot_ascii_name
-- 
2.35.3

Re: [patch net-next v2 06/12] mlxsw: reg: Extend MDDQ by device_info

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:41AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Extend existing MDDQ register by possibility to query information about
> devices residing on a line card.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

I don't think this changed since internal review, so:

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

[patch net-next v2 07/12] mlxsw: core_linecards: Probe provisioned line cards for devices and expose FW version

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

In case the line card is provisioned, go over all possible existing
devices (gearboxes) on it and expose FW version of the flashable one.

Example:

$ devlink dev info auxiliary/mlxsw_core.lc.0
auxiliary/mlxsw_core.lc.0:
  versions:
      fixed:
        hw.revision 0
      running:
        ini.version 4
        fw 19.2010.1312

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/networking/devlink/mlxsw.rst    |  3 +
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  9 +++
 .../ethernet/mellanox/mlxsw/core_linecards.c  | 57 +++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/Documentation/networking/devlink/mlxsw.rst b/Documentation/networking/devlink/mlxsw.rst
index aededcf68df4..65ceed98f94d 100644
--- a/Documentation/networking/devlink/mlxsw.rst
+++ b/Documentation/networking/devlink/mlxsw.rst
@@ -75,6 +75,9 @@ The ``mlxsw`` driver reports the following versions for line card auxiliary devi
    * - ``ini.version``
      - running
      - Version of line card INI loaded
+   * - ``fw.version``
+     - running
+     - Three digit firmware version of line card device
 
 Driver-specific Traps
 =====================
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 87c58b512536..e19860c05e75 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -564,6 +564,12 @@ enum mlxsw_linecard_status_event_type {
 
 struct mlxsw_linecard_bdev;
 
+struct mlxsw_linecard_device_info {
+	u16 fw_major;
+	u16 fw_minor;
+	u16 fw_sub_minor;
+};
+
 struct mlxsw_linecard {
 	u8 slot_index;
 	struct mlxsw_linecards *linecards;
@@ -579,6 +585,9 @@ struct mlxsw_linecard {
 	u16 hw_revision;
 	u16 ini_version;
 	struct mlxsw_linecard_bdev *bdev;
+	struct {
+		struct mlxsw_linecard_device_info info;
+	} device;
 };
 
 struct mlxsw_linecard_types_info;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
index c427e07b25dd..bd8f43e21212 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
@@ -87,6 +87,47 @@ static const char *mlxsw_linecard_type_name(struct mlxsw_linecard *linecard)
 	return linecard->name;
 }
 
+static int mlxsw_linecard_device_info_update(struct mlxsw_linecard *linecard)
+{
+	struct mlxsw_core *mlxsw_core = linecard->linecards->mlxsw_core;
+	bool flashable_found = false;
+	u8 msg_seq = 0;
+
+	do {
+		struct mlxsw_linecard_device_info info;
+		char mddq_pl[MLXSW_REG_MDDQ_LEN];
+		bool flash_owner;
+		bool data_valid;
+		u8 device_index;
+		int err;
+
+		mlxsw_reg_mddq_device_info_pack(mddq_pl, linecard->slot_index,
+						msg_seq);
+		err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mddq), mddq_pl);
+		if (err)
+			return err;
+		mlxsw_reg_mddq_device_info_unpack(mddq_pl, &msg_seq,
+						  &data_valid, &flash_owner,
+						  &device_index,
+						  &info.fw_major,
+						  &info.fw_minor,
+						  &info.fw_sub_minor);
+		if (!data_valid)
+			break;
+		if (!flash_owner) /* We care only about flashable ones. */
+			continue;
+		if (flashable_found) {
+			dev_warn_once(linecard->linecards->bus_info->dev, "linecard %u: More flashable devices present, exposing only the first one\n",
+				      linecard->slot_index);
+			return 0;
+		}
+		linecard->device.info = info;
+		flashable_found = true;
+	} while (msg_seq);
+
+	return 0;
+}
+
 static void mlxsw_linecard_provision_fail(struct mlxsw_linecard *linecard)
 {
 	linecard->provisioned = false;
@@ -249,6 +290,18 @@ int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
 	if (err)
 		goto unlock;
 
+	if (linecard->ready) {
+		struct mlxsw_linecard_device_info *info = &linecard->device.info;
+
+		sprintf(buf, "%u.%u.%u", info->fw_major, info->fw_minor,
+			info->fw_sub_minor);
+		err = devlink_info_version_running_put(req,
+						       DEVLINK_INFO_VERSION_GENERIC_FW,
+						       buf);
+		if (err)
+			goto unlock;
+	}
+
 unlock:
 	mutex_unlock(&linecard->lock);
 	return err;
@@ -308,6 +361,10 @@ static int mlxsw_linecard_ready_set(struct mlxsw_linecard *linecard)
 	char mddc_pl[MLXSW_REG_MDDC_LEN];
 	int err;
 
+	err = mlxsw_linecard_device_info_update(linecard);
+	if (err)
+		return err;
+
 	mlxsw_reg_mddc_pack(mddc_pl, linecard->slot_index, false, true);
 	err = mlxsw_reg_write(mlxsw_core, MLXSW_REG(mddc), mddc_pl);
 	if (err)
-- 
2.35.3

Re: [patch net-next v2 07/12] mlxsw: core_linecards: Probe provisioned line cards for devices and expose FW version

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:42AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> In case the line card is provisioned, go over all possible existing
> devices (gearboxes) on it and expose FW version of the flashable one.

I think this is a bit misleading. The FW version is only exposed for
line cards that are "ready", which is a temporary state between
"provisioned" and "active".

Any reason not to expose the FW version only when the line card is
"active"? At least this state is exposed to user space.

Re: [patch net-next v2 07/12] mlxsw: core_linecards: Probe provisioned line cards for devices and expose FW version

[Jiri Pirko]

Wed, Jul 20, 2022 at 11:14:42AM CEST, idosch@nvidia.com wrote:
>On Tue, Jul 19, 2022 at 08:48:42AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> In case the line card is provisioned, go over all possible existing
>> devices (gearboxes) on it and expose FW version of the flashable one.
>
>I think this is a bit misleading. The FW version is only exposed for
>line cards that are "ready", which is a temporary state between
>"provisioned" and "active".
>
>Any reason not to expose the FW version only when the line card is
>"active"? At least this state is exposed to user space.

When it is active, the ready bool is still set. So it is "since ready".

Re: [patch net-next v2 07/12] mlxsw: core_linecards: Probe provisioned line cards for devices and expose FW version

[Ido Schimmel]

On Wed, Jul 20, 2022 at 12:51:33PM +0200, Jiri Pirko wrote:
> Wed, Jul 20, 2022 at 11:14:42AM CEST, idosch@nvidia.com wrote:
> >On Tue, Jul 19, 2022 at 08:48:42AM +0200, Jiri Pirko wrote:
> >> From: Jiri Pirko <jiri@nvidia.com>
> >> 
> >> In case the line card is provisioned, go over all possible existing
> >> devices (gearboxes) on it and expose FW version of the flashable one.
> >
> >I think this is a bit misleading. The FW version is only exposed for
> >line cards that are "ready", which is a temporary state between
> >"provisioned" and "active".
> >
> >Any reason not to expose the FW version only when the line card is
> >"active"? At least this state is exposed to user space.
> 
> When it is active, the ready bool is still set. So it is "since ready".

My point is that sometimes while the line card is still "provisioned"
you will get the FW version and sometimes not. Because it is enabled
based on a transient state not exposed to user space. If you enable it
based on "linecard->active", then there is no race.

Re: [patch net-next v2 07/12] mlxsw: core_linecards: Probe provisioned line cards for devices and expose FW version

[Jiri Pirko]

Wed, Jul 20, 2022 at 01:16:37PM CEST, idosch@nvidia.com wrote:
>On Wed, Jul 20, 2022 at 12:51:33PM +0200, Jiri Pirko wrote:
>> Wed, Jul 20, 2022 at 11:14:42AM CEST, idosch@nvidia.com wrote:
>> >On Tue, Jul 19, 2022 at 08:48:42AM +0200, Jiri Pirko wrote:
>> >> From: Jiri Pirko <jiri@nvidia.com>
>> >> 
>> >> In case the line card is provisioned, go over all possible existing
>> >> devices (gearboxes) on it and expose FW version of the flashable one.
>> >
>> >I think this is a bit misleading. The FW version is only exposed for
>> >line cards that are "ready", which is a temporary state between
>> >"provisioned" and "active".
>> >
>> >Any reason not to expose the FW version only when the line card is
>> >"active"? At least this state is exposed to user space.
>> 
>> When it is active, the ready bool is still set. So it is "since ready".
>
>My point is that sometimes while the line card is still "provisioned"
>you will get the FW version and sometimes not. Because it is enabled
>based on a transient state not exposed to user space. If you enable it
>based on "linecard->active", then there is no race.

Okay, will do.

[patch net-next v2 08/12] mlxsw: reg: Add Management DownStream Device Tunneling Register

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

The MDDT register allows to deliver query and request messages (PRM
registers, commands) to a DownStream device.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h | 90 +++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 76caf06b17d6..e45df09df757 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -11276,6 +11276,95 @@ mlxsw_reg_mbct_unpack(const char *payload, u8 *p_slot_index,
 		*p_fsm_state = mlxsw_reg_mbct_fsm_state_get(payload);
 }
 
+/* MDDT - Management DownStream Device Tunneling Register
+ * ------------------------------------------------------
+ * This register allows to deliver query and request messages (PRM registers,
+ * commands) to a DownStream device.
+ */
+#define MLXSW_REG_MDDT_ID 0x9160
+#define MLXSW_REG_MDDT_LEN 0x110
+
+MLXSW_REG_DEFINE(mddt, MLXSW_REG_MDDT_ID, MLXSW_REG_MDDT_LEN);
+
+/* reg_mddt_slot_index
+ * Slot index.
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mddt, slot_index, 0x00, 8, 4);
+
+/* reg_mddt_device_index
+ * Device index.
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mddt, device_index, 0x00, 0, 8);
+
+/* reg_mddt_read_size
+ * Read size in D-Words.
+ * Access: OP
+ */
+MLXSW_ITEM32(reg, mddt, read_size, 0x04, 24, 8);
+
+/* reg_mddt_write_size
+ * Write size in D-Words.
+ * Access: OP
+ */
+MLXSW_ITEM32(reg, mddt, write_size, 0x04, 16, 8);
+
+enum mlxsw_reg_mddt_status {
+	MLXSW_REG_MDDT_STATUS_OK,
+};
+
+/* reg_mddt_status
+ * Return code of the Downstream Device to the register that was sent.
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mddt, status, 0x0C, 24, 8);
+
+enum mlxsw_reg_mddt_method {
+	MLXSW_REG_MDDT_METHOD_QUERY,
+	MLXSW_REG_MDDT_METHOD_WRITE,
+};
+
+/* reg_mddt_method
+ * Access: OP
+ */
+MLXSW_ITEM32(reg, mddt, method, 0x0C, 22, 2);
+
+/* reg_mddt_register_id
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mddt, register_id, 0x0C, 0, 16);
+
+#define MLXSW_REG_MDDT_PAYLOAD_OFFSET 0x0C
+#define MLXSW_REG_MDDT_PRM_REGISTER_HEADER_LEN 4
+
+static inline char *mlxsw_reg_mddt_inner_payload(char *payload)
+{
+	return payload + MLXSW_REG_MDDT_PAYLOAD_OFFSET +
+	       MLXSW_REG_MDDT_PRM_REGISTER_HEADER_LEN;
+}
+
+static inline void mlxsw_reg_mddt_pack(char *payload, u8 slot_index,
+				       u8 device_index,
+				       enum mlxsw_reg_mddt_method method,
+				       const struct mlxsw_reg_info *reg,
+				       char **inner_payload)
+{
+	int len = reg->len + MLXSW_REG_MDDT_PRM_REGISTER_HEADER_LEN;
+
+	if (WARN_ON(len + MLXSW_REG_MDDT_PAYLOAD_OFFSET > MLXSW_REG_MDDT_LEN))
+		len = MLXSW_REG_MDDT_LEN - MLXSW_REG_MDDT_PAYLOAD_OFFSET;
+
+	MLXSW_REG_ZERO(mddt, payload);
+	mlxsw_reg_mddt_slot_index_set(payload, slot_index);
+	mlxsw_reg_mddt_device_index_set(payload, device_index);
+	mlxsw_reg_mddt_method_set(payload, method);
+	mlxsw_reg_mddt_register_id_set(payload, reg->id);
+	mlxsw_reg_mddt_read_size_set(payload, len / 4);
+	mlxsw_reg_mddt_write_size_set(payload, len / 4);
+	*inner_payload = mlxsw_reg_mddt_inner_payload(payload);
+}
+
 /* MDDQ - Management DownStream Device Query Register
  * --------------------------------------------------
  * This register allows to query the DownStream device properties. The desired
@@ -12854,6 +12943,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
 	MLXSW_REG(mfgd),
 	MLXSW_REG(mgpir),
 	MLXSW_REG(mbct),
+	MLXSW_REG(mddt),
 	MLXSW_REG(mddq),
 	MLXSW_REG(mddc),
 	MLXSW_REG(mfde),
-- 
2.35.3

[patch net-next v2 09/12] mlxsw: core_linecards: Expose device PSID over device info

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Used tunneled MGIR to obtain PSID of line card device and extend
device_info_get() op to fill up the info with that.

Example:

$ devlink dev info auxiliary/mlxsw_core.lc.0
auxiliary/mlxsw_core.lc.0:
  versions:
      fixed:
        hw.revision 0
        fw.psid MT_0000000749
      running:
        ini.version 4
        fw 19.2010.1312

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/networking/devlink/mlxsw.rst    |  3 ++
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  1 +
 .../ethernet/mellanox/mlxsw/core_linecards.c  | 31 +++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/Documentation/networking/devlink/mlxsw.rst b/Documentation/networking/devlink/mlxsw.rst
index 65ceed98f94d..433962225bd4 100644
--- a/Documentation/networking/devlink/mlxsw.rst
+++ b/Documentation/networking/devlink/mlxsw.rst
@@ -75,6 +75,9 @@ The ``mlxsw`` driver reports the following versions for line card auxiliary devi
    * - ``ini.version``
      - running
      - Version of line card INI loaded
+   * - ``fw.psid``
+     - fixed
+     - Line card device PSID
    * - ``fw.version``
      - running
      - Three digit firmware version of line card device
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index e19860c05e75..a3246082219d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -568,6 +568,7 @@ struct mlxsw_linecard_device_info {
 	u16 fw_major;
 	u16 fw_minor;
 	u16 fw_sub_minor;
+	char psid[MLXSW_REG_MGIR_FW_INFO_PSID_SIZE];
 };
 
 struct mlxsw_linecard {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
index bd8f43e21212..471f07bc5c2f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
@@ -87,6 +87,27 @@ static const char *mlxsw_linecard_type_name(struct mlxsw_linecard *linecard)
 	return linecard->name;
 }
 
+static int mlxsw_linecard_device_psid_get(struct mlxsw_linecard *linecard,
+					  u8 device_index, char *psid)
+{
+	struct mlxsw_core *mlxsw_core = linecard->linecards->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	char *mgir_pl;
+	int err;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index, device_index,
+			    MLXSW_REG_MDDT_METHOD_QUERY,
+			    MLXSW_REG(mgir), &mgir_pl);
+
+	mlxsw_reg_mgir_pack(mgir_pl);
+	err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+	if (err)
+		return err;
+
+	mlxsw_reg_mgir_fw_info_psid_memcpy_from(mgir_pl, psid);
+	return 0;
+}
+
 static int mlxsw_linecard_device_info_update(struct mlxsw_linecard *linecard)
 {
 	struct mlxsw_core *mlxsw_core = linecard->linecards->mlxsw_core;
@@ -121,6 +142,12 @@ static int mlxsw_linecard_device_info_update(struct mlxsw_linecard *linecard)
 				      linecard->slot_index);
 			return 0;
 		}
+
+		err = mlxsw_linecard_device_psid_get(linecard, device_index,
+						     info.psid);
+		if (err)
+			return err;
+
 		linecard->device.info = info;
 		flashable_found = true;
 	} while (msg_seq);
@@ -293,6 +320,10 @@ int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
 	if (linecard->ready) {
 		struct mlxsw_linecard_device_info *info = &linecard->device.info;
 
+		err = devlink_info_version_fixed_put(req,
+						     DEVLINK_INFO_VERSION_GENERIC_FW_PSID,
+						     info->psid);
+
 		sprintf(buf, "%u.%u.%u", info->fw_major, info->fw_minor,
 			info->fw_sub_minor);
 		err = devlink_info_version_running_put(req,
-- 
2.35.3

Re: [patch net-next v2 09/12] mlxsw: core_linecards: Expose device PSID over device info

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:44AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Used tunneled MGIR to obtain PSID of line card device and extend

s/Used/Use/

> device_info_get() op to fill up the info with that.

[patch net-next v2 10/12] mlxsw: core_linecards: Implement line card device flashing

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Implement flash_update() devlink op for the line card devlink instance
to allow user to update line card gearbox FW using MDDT register
and mlxfw.

Example:
$ devlink dev flash auxiliary/mlxsw_core.lc.0 file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  31 +-
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  11 +
 .../mellanox/mlxsw/core_linecard_dev.c        |  13 +
 .../ethernet/mellanox/mlxsw/core_linecards.c  | 277 ++++++++++++++++++
 4 files changed, 322 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 831b0d3472c6..abc9680527d8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -951,6 +951,20 @@ static struct mlxsw_driver *mlxsw_core_driver_get(const char *kind)
 	return mlxsw_driver;
 }
 
+int mlxsw_core_fw_flash(struct mlxsw_core *mlxsw_core,
+			struct mlxfw_dev *mlxfw_dev,
+			const struct firmware *firmware,
+			struct netlink_ext_ack *extack)
+{
+	int err;
+
+	mlxsw_core->fw_flash_in_progress = true;
+	err = mlxfw_firmware_flash(mlxfw_dev, firmware, extack);
+	mlxsw_core->fw_flash_in_progress = false;
+
+	return err;
+}
+
 struct mlxsw_core_fw_info {
 	struct mlxfw_dev mlxfw_dev;
 	struct mlxsw_core *mlxsw_core;
@@ -1105,8 +1119,9 @@ static const struct mlxfw_dev_ops mlxsw_core_fw_mlxsw_dev_ops = {
 	.fsm_release		= mlxsw_core_fw_fsm_release,
 };
 
-static int mlxsw_core_fw_flash(struct mlxsw_core *mlxsw_core, const struct firmware *firmware,
-			       struct netlink_ext_ack *extack)
+static int mlxsw_core_dev_fw_flash(struct mlxsw_core *mlxsw_core,
+				   const struct firmware *firmware,
+				   struct netlink_ext_ack *extack)
 {
 	struct mlxsw_core_fw_info mlxsw_core_fw_info = {
 		.mlxfw_dev = {
@@ -1117,13 +1132,9 @@ static int mlxsw_core_fw_flash(struct mlxsw_core *mlxsw_core, const struct firmw
 		},
 		.mlxsw_core = mlxsw_core
 	};
-	int err;
 
-	mlxsw_core->fw_flash_in_progress = true;
-	err = mlxfw_firmware_flash(&mlxsw_core_fw_info.mlxfw_dev, firmware, extack);
-	mlxsw_core->fw_flash_in_progress = false;
-
-	return err;
+	return mlxsw_core_fw_flash(mlxsw_core, &mlxsw_core_fw_info.mlxfw_dev,
+				   firmware, extack);
 }
 
 static int mlxsw_core_fw_rev_validate(struct mlxsw_core *mlxsw_core,
@@ -1169,7 +1180,7 @@ static int mlxsw_core_fw_rev_validate(struct mlxsw_core *mlxsw_core,
 		return err;
 	}
 
-	err = mlxsw_core_fw_flash(mlxsw_core, firmware, NULL);
+	err = mlxsw_core_dev_fw_flash(mlxsw_core, firmware, NULL);
 	release_firmware(firmware);
 	if (err)
 		dev_err(mlxsw_bus_info->dev, "Could not upgrade firmware\n");
@@ -1187,7 +1198,7 @@ static int mlxsw_core_fw_flash_update(struct mlxsw_core *mlxsw_core,
 				      struct devlink_flash_update_params *params,
 				      struct netlink_ext_ack *extack)
 {
-	return mlxsw_core_fw_flash(mlxsw_core, params->fw, extack);
+	return mlxsw_core_dev_fw_flash(mlxsw_core, params->fw, extack);
 }
 
 static int mlxsw_core_devlink_param_fw_load_policy_validate(struct devlink *devlink, u32 id,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index a3246082219d..39c4a139188f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -19,6 +19,7 @@
 #include "reg.h"
 #include "cmd.h"
 #include "resources.h"
+#include "../mlxfw/mlxfw.h"
 
 enum mlxsw_core_resource_id {
 	MLXSW_CORE_RESOURCE_PORTS = 1,
@@ -48,6 +49,11 @@ mlxsw_core_fw_rev_minor_subminor_validate(const struct mlxsw_fw_rev *rev,
 int mlxsw_core_driver_register(struct mlxsw_driver *mlxsw_driver);
 void mlxsw_core_driver_unregister(struct mlxsw_driver *mlxsw_driver);
 
+int mlxsw_core_fw_flash(struct mlxsw_core *mlxsw_core,
+			struct mlxfw_dev *mlxfw_dev,
+			const struct firmware *firmware,
+			struct netlink_ext_ack *extack);
+
 int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 				   const struct mlxsw_bus *mlxsw_bus,
 				   void *bus_priv, bool reload,
@@ -588,6 +594,7 @@ struct mlxsw_linecard {
 	struct mlxsw_linecard_bdev *bdev;
 	struct {
 		struct mlxsw_linecard_device_info info;
+		u8 index;
 	} device;
 };
 
@@ -612,6 +619,10 @@ mlxsw_linecard_get(struct mlxsw_linecards *linecards, u8 slot_index)
 int mlxsw_linecard_devlink_info_get(struct mlxsw_linecard *linecard,
 				    struct devlink_info_req *req,
 				    struct netlink_ext_ack *extack);
+int mlxsw_linecard_flash_update(struct devlink *linecard_devlink,
+				struct mlxsw_linecard *linecard,
+				const struct firmware *firmware,
+				struct netlink_ext_ack *extack);
 
 int mlxsw_linecards_init(struct mlxsw_core *mlxsw_core,
 			 const struct mlxsw_bus_info *bus_info);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
index d0ecefee587b..e0cf6ab7d828 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
@@ -107,8 +107,21 @@ static int mlxsw_linecard_dev_devlink_info_get(struct devlink *devlink,
 	return mlxsw_linecard_devlink_info_get(linecard, req, extack);
 }
 
+static int
+mlxsw_linecard_dev_devlink_flash_update(struct devlink *devlink,
+					struct devlink_flash_update_params *params,
+					struct netlink_ext_ack *extack)
+{
+	struct mlxsw_linecard_dev *linecard_dev = devlink_priv(devlink);
+	struct mlxsw_linecard *linecard = linecard_dev->linecard;
+
+	return mlxsw_linecard_flash_update(devlink, linecard,
+					   params->fw, extack);
+}
+
 static const struct devlink_ops mlxsw_linecard_dev_devlink_ops = {
 	.info_get			= mlxsw_linecard_dev_devlink_info_get,
+	.flash_update			= mlxsw_linecard_dev_devlink_flash_update,
 };
 
 static int mlxsw_linecard_bdev_probe(struct auxiliary_device *adev,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
index 471f07bc5c2f..a01beaadf06e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c
@@ -13,6 +13,7 @@
 #include <linux/vmalloc.h>
 
 #include "core.h"
+#include "../mlxfw/mlxfw.h"
 
 struct mlxsw_linecard_ini_file {
 	__le16 size;
@@ -87,6 +88,281 @@ static const char *mlxsw_linecard_type_name(struct mlxsw_linecard *linecard)
 	return linecard->name;
 }
 
+struct mlxsw_linecard_device_fw_info {
+	struct mlxfw_dev mlxfw_dev;
+	struct mlxsw_core *mlxsw_core;
+	struct mlxsw_linecard *linecard;
+};
+
+static int mlxsw_linecard_device_fw_component_query(struct mlxfw_dev *mlxfw_dev,
+						    u16 component_index,
+						    u32 *p_max_size,
+						    u8 *p_align_bits,
+						    u16 *p_max_write_size)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	char *mcqi_pl;
+	int err;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_QUERY,
+			    MLXSW_REG(mcqi), &mcqi_pl);
+
+	mlxsw_reg_mcqi_pack(mcqi_pl, component_index);
+	err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+	if (err)
+		return err;
+	mlxsw_reg_mcqi_unpack(mcqi_pl, p_max_size, p_align_bits,
+			      p_max_write_size);
+
+	*p_align_bits = max_t(u8, *p_align_bits, 2);
+	*p_max_write_size = min_t(u16, *p_max_write_size,
+				  MLXSW_REG_MCDA_MAX_DATA_LEN);
+	return 0;
+}
+
+static int mlxsw_linecard_device_fw_fsm_lock(struct mlxfw_dev *mlxfw_dev,
+					     u32 *fwhandle)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	u8 control_state;
+	char *mcc_pl;
+	int err;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_QUERY,
+			    MLXSW_REG(mcc), &mcc_pl);
+	mlxsw_reg_mcc_pack(mcc_pl, 0, 0, 0, 0);
+	err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+	if (err)
+		return err;
+
+	mlxsw_reg_mcc_unpack(mcc_pl, fwhandle, NULL, &control_state);
+	if (control_state != MLXFW_FSM_STATE_IDLE)
+		return -EBUSY;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_WRITE,
+			    MLXSW_REG(mcc), &mcc_pl);
+	mlxsw_reg_mcc_pack(mcc_pl, MLXSW_REG_MCC_INSTRUCTION_LOCK_UPDATE_HANDLE,
+			   0, *fwhandle, 0);
+	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+}
+
+static int
+mlxsw_linecard_device_fw_fsm_component_update(struct mlxfw_dev *mlxfw_dev,
+					      u32 fwhandle,
+					      u16 component_index,
+					      u32 component_size)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	char *mcc_pl;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_WRITE,
+			    MLXSW_REG(mcc), &mcc_pl);
+	mlxsw_reg_mcc_pack(mcc_pl, MLXSW_REG_MCC_INSTRUCTION_UPDATE_COMPONENT,
+			   component_index, fwhandle, component_size);
+	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+}
+
+static int
+mlxsw_linecard_device_fw_fsm_block_download(struct mlxfw_dev *mlxfw_dev,
+					    u32 fwhandle, u8 *data,
+					    u16 size, u32 offset)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	char *mcda_pl;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_WRITE,
+			    MLXSW_REG(mcda), &mcda_pl);
+	mlxsw_reg_mcda_pack(mcda_pl, fwhandle, offset, size, data);
+	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+}
+
+static int
+mlxsw_linecard_device_fw_fsm_component_verify(struct mlxfw_dev *mlxfw_dev,
+					      u32 fwhandle, u16 component_index)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	char *mcc_pl;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_WRITE,
+			    MLXSW_REG(mcc), &mcc_pl);
+	mlxsw_reg_mcc_pack(mcc_pl, MLXSW_REG_MCC_INSTRUCTION_VERIFY_COMPONENT,
+			   component_index, fwhandle, 0);
+	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+}
+
+static int mlxsw_linecard_device_fw_fsm_activate(struct mlxfw_dev *mlxfw_dev,
+						 u32 fwhandle)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	char *mcc_pl;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_WRITE,
+			    MLXSW_REG(mcc), &mcc_pl);
+	mlxsw_reg_mcc_pack(mcc_pl, MLXSW_REG_MCC_INSTRUCTION_ACTIVATE,
+			   0, fwhandle, 0);
+	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+}
+
+static int
+mlxsw_linecard_device_fw_fsm_query_state(struct mlxfw_dev *mlxfw_dev,
+					 u32 fwhandle,
+					 enum mlxfw_fsm_state *fsm_state,
+					 enum mlxfw_fsm_state_err *fsm_state_err)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	u8 control_state;
+	u8 error_code;
+	char *mcc_pl;
+	int err;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_QUERY,
+			    MLXSW_REG(mcc), &mcc_pl);
+	mlxsw_reg_mcc_pack(mcc_pl, 0, 0, fwhandle, 0);
+	err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+	if (err)
+		return err;
+
+	mlxsw_reg_mcc_unpack(mcc_pl, NULL, &error_code, &control_state);
+	*fsm_state = control_state;
+	*fsm_state_err = min_t(enum mlxfw_fsm_state_err, error_code,
+			       MLXFW_FSM_STATE_ERR_MAX);
+	return 0;
+}
+
+static void mlxsw_linecard_device_fw_fsm_cancel(struct mlxfw_dev *mlxfw_dev,
+						u32 fwhandle)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	char *mcc_pl;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_WRITE,
+			    MLXSW_REG(mcc), &mcc_pl);
+	mlxsw_reg_mcc_pack(mcc_pl, MLXSW_REG_MCC_INSTRUCTION_CANCEL,
+			   0, fwhandle, 0);
+	mlxsw_reg_write(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+}
+
+static void mlxsw_linecard_device_fw_fsm_release(struct mlxfw_dev *mlxfw_dev,
+						 u32 fwhandle)
+{
+	struct mlxsw_linecard_device_fw_info *info =
+		container_of(mlxfw_dev, struct mlxsw_linecard_device_fw_info,
+			     mlxfw_dev);
+	struct mlxsw_linecard *linecard = info->linecard;
+	struct mlxsw_core *mlxsw_core = info->mlxsw_core;
+	char mddt_pl[MLXSW_REG_MDDT_LEN];
+	char *mcc_pl;
+
+	mlxsw_reg_mddt_pack(mddt_pl, linecard->slot_index,
+			    linecard->device.index,
+			    MLXSW_REG_MDDT_METHOD_WRITE,
+			    MLXSW_REG(mcc), &mcc_pl);
+	mlxsw_reg_mcc_pack(mcc_pl,
+			   MLXSW_REG_MCC_INSTRUCTION_RELEASE_UPDATE_HANDLE,
+			   0, fwhandle, 0);
+	mlxsw_reg_write(mlxsw_core, MLXSW_REG(mddt), mddt_pl);
+}
+
+static const struct mlxfw_dev_ops mlxsw_linecard_device_dev_ops = {
+	.component_query	= mlxsw_linecard_device_fw_component_query,
+	.fsm_lock		= mlxsw_linecard_device_fw_fsm_lock,
+	.fsm_component_update	= mlxsw_linecard_device_fw_fsm_component_update,
+	.fsm_block_download	= mlxsw_linecard_device_fw_fsm_block_download,
+	.fsm_component_verify	= mlxsw_linecard_device_fw_fsm_component_verify,
+	.fsm_activate		= mlxsw_linecard_device_fw_fsm_activate,
+	.fsm_query_state	= mlxsw_linecard_device_fw_fsm_query_state,
+	.fsm_cancel		= mlxsw_linecard_device_fw_fsm_cancel,
+	.fsm_release		= mlxsw_linecard_device_fw_fsm_release,
+};
+
+int mlxsw_linecard_flash_update(struct devlink *linecard_devlink,
+				struct mlxsw_linecard *linecard,
+				const struct firmware *firmware,
+				struct netlink_ext_ack *extack)
+{
+	struct mlxsw_core *mlxsw_core = linecard->linecards->mlxsw_core;
+	struct mlxsw_linecard_device_fw_info info = {
+		.mlxfw_dev = {
+			.ops = &mlxsw_linecard_device_dev_ops,
+			.psid = linecard->device.info.psid,
+			.psid_size = strlen(linecard->device.info.psid),
+			.devlink = linecard_devlink,
+		},
+		.mlxsw_core = mlxsw_core,
+		.linecard = linecard,
+	};
+	int err;
+
+	mutex_lock(&linecard->lock);
+	if (WARN_ON(!linecard->ready)) {
+		err = -EINVAL;
+		goto unlock;
+	}
+	err = mlxsw_core_fw_flash(mlxsw_core, &info.mlxfw_dev,
+				  firmware, extack);
+unlock:
+	mutex_unlock(&linecard->lock);
+	return err;
+}
+
 static int mlxsw_linecard_device_psid_get(struct mlxsw_linecard *linecard,
 					  u8 device_index, char *psid)
 {
@@ -149,6 +425,7 @@ static int mlxsw_linecard_device_info_update(struct mlxsw_linecard *linecard)
 			return err;
 
 		linecard->device.info = info;
+		linecard->device.index = device_index;
 		flashable_found = true;
 	} while (msg_seq);
 
-- 
2.35.3

Re: [patch net-next v2 10/12] mlxsw: core_linecards: Implement line card device flashing

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:45AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Implement flash_update() devlink op for the line card devlink instance
> to allow user to update line card gearbox FW using MDDT register
> and mlxfw.
> 
> Example:
> $ devlink dev flash auxiliary/mlxsw_core.lc.0 file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2

Need to mention that this is only possible when line card is
active/ready

> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

[...]

> +int mlxsw_linecard_flash_update(struct devlink *linecard_devlink,
> +				struct mlxsw_linecard *linecard,
> +				const struct firmware *firmware,
> +				struct netlink_ext_ack *extack)
> +{
> +	struct mlxsw_core *mlxsw_core = linecard->linecards->mlxsw_core;
> +	struct mlxsw_linecard_device_fw_info info = {
> +		.mlxfw_dev = {
> +			.ops = &mlxsw_linecard_device_dev_ops,
> +			.psid = linecard->device.info.psid,
> +			.psid_size = strlen(linecard->device.info.psid),
> +			.devlink = linecard_devlink,
> +		},
> +		.mlxsw_core = mlxsw_core,
> +		.linecard = linecard,
> +	};
> +	int err;
> +
> +	mutex_lock(&linecard->lock);
> +	if (WARN_ON(!linecard->ready)) {

Can't this be easily triggered from user space when executing the above
command for a provisioned line card? If so, please remove the WARN_ON()
and add an extack

> +		err = -EINVAL;
> +		goto unlock;
> +	}
> +	err = mlxsw_core_fw_flash(mlxsw_core, &info.mlxfw_dev,
> +				  firmware, extack);
> +unlock:
> +	mutex_unlock(&linecard->lock);
> +	return err;
> +}

Re: [patch net-next v2 10/12] mlxsw: core_linecards: Implement line card device flashing

[Jiri Pirko]

Wed, Jul 20, 2022 at 12:46:25PM CEST, idosch@nvidia.com wrote:
>On Tue, Jul 19, 2022 at 08:48:45AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> Implement flash_update() devlink op for the line card devlink instance
>> to allow user to update line card gearbox FW using MDDT register
>> and mlxfw.
>> 
>> Example:
>> $ devlink dev flash auxiliary/mlxsw_core.lc.0 file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2
>
>Need to mention that this is only possible when line card is
>active/ready

I don't see the need. This is an example. When user issues it and device
is not ready, he gets back an error. As with any other example.


>
>> 
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>
>[...]
>
>> +int mlxsw_linecard_flash_update(struct devlink *linecard_devlink,
>> +				struct mlxsw_linecard *linecard,
>> +				const struct firmware *firmware,
>> +				struct netlink_ext_ack *extack)
>> +{
>> +	struct mlxsw_core *mlxsw_core = linecard->linecards->mlxsw_core;
>> +	struct mlxsw_linecard_device_fw_info info = {
>> +		.mlxfw_dev = {
>> +			.ops = &mlxsw_linecard_device_dev_ops,
>> +			.psid = linecard->device.info.psid,
>> +			.psid_size = strlen(linecard->device.info.psid),
>> +			.devlink = linecard_devlink,
>> +		},
>> +		.mlxsw_core = mlxsw_core,
>> +		.linecard = linecard,
>> +	};
>> +	int err;
>> +
>> +	mutex_lock(&linecard->lock);
>> +	if (WARN_ON(!linecard->ready)) {
>
>Can't this be easily triggered from user space when executing the above
>command for a provisioned line card? If so, please remove the WARN_ON()
>and add an extack

Yep, you are correct, this is leftover I missed to fix. Will do.


>
>> +		err = -EINVAL;
>> +		goto unlock;
>> +	}
>> +	err = mlxsw_core_fw_flash(mlxsw_core, &info.mlxfw_dev,
>> +				  firmware, extack);
>> +unlock:
>> +	mutex_unlock(&linecard->lock);
>> +	return err;
>> +}

[patch net-next v2 11/12] selftests: mlxsw: Check line card info on provisioned line card

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Once line card is provisioned, check if HW revision and INI version
are exposed on associated nested auxiliary device.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../drivers/net/mlxsw/devlink_linecard.sh     | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh b/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
index 08a922d8b86a..ca4e9b08a105 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
@@ -84,6 +84,13 @@ lc_wait_until_port_count_is()
 	busywait "$timeout" until_lc_port_count_is "$port_count" lc_port_count_get "$lc"
 }
 
+lc_nested_devlink_dev_get()
+{
+	local lc=$1
+
+	devlink lc show $DEVLINK_DEV lc $lc -j | jq -e -r ".[][][].nested_devlink"
+}
+
 PROV_UNPROV_TIMEOUT=8000 # ms
 POST_PROV_ACT_TIMEOUT=2000 # ms
 PROV_PORTS_INSTANTIATION_TIMEOUT=15000 # ms
@@ -191,12 +198,30 @@ ports_check()
 	check_err $? "Unexpected port count linecard $lc (got $port_count, expected $expected_port_count)"
 }
 
+lc_dev_info_provisioned_check()
+{
+	local lc=$1
+	local nested_devlink_dev=$2
+	local fixed_hw_revision
+	local running_ini_version
+
+	fixed_hw_revision=$(devlink dev info $nested_devlink_dev -j | \
+			    jq -e -r '.[][].versions.fixed."hw.revision"')
+	check_err $? "Failed to get linecard $lc fixed.hw.revision"
+	log_info "Linecard $lc fixed.hw.revision: \"$fixed_hw_revision\""
+	running_ini_version=$(devlink dev info $nested_devlink_dev -j | \
+			      jq -e -r '.[][].versions.running."ini.version"')
+	check_err $? "Failed to get linecard $lc running.ini.version"
+	log_info "Linecard $lc running.ini.version: \"$running_ini_version\""
+}
+
 provision_test()
 {
 	RET=0
 	local lc
 	local type
 	local state
+	local nested_devlink_dev
 
 	lc=$LC_SLOT
 	supported_types_check $lc
@@ -207,6 +232,11 @@ provision_test()
 	fi
 	provision_one $lc $LC_16X100G_TYPE
 	ports_check $lc $LC_16X100G_PORT_COUNT
+
+	nested_devlink_dev=$(lc_nested_devlink_dev_get $lc)
+	check_err $? "Failed to get nested devlink handle of linecard $lc"
+	lc_dev_info_provisioned_check $lc $nested_devlink_dev
+
 	log_test "Provision"
 }
 
-- 
2.35.3

[patch net-next v2 12/12] selftests: mlxsw: Check line card info on activated line card

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Once line card is activated, check the FW version and PSID are exposed.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../drivers/net/mlxsw/devlink_linecard.sh     | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh b/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
index ca4e9b08a105..224ca3695c89 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
@@ -250,12 +250,32 @@ interface_check()
 	setup_wait
 }
 
+lc_dev_info_active_check()
+{
+	local lc=$1
+	local nested_devlink_dev=$2
+	local fixed_device_fw_psid
+	local running_device_fw
+
+	fixed_device_fw_psid=$(devlink dev info $nested_devlink_dev -j | \
+			       jq -e -r ".[][].versions.fixed" | \
+			       jq -e -r '."fw.psid"')
+	check_err $? "Failed to get linecard $lc fixed fw PSID"
+	log_info "Linecard $lc fixed.fw.psid: \"$fixed_device_fw_psid\""
+
+	running_device_fw=$(devlink dev info $nested_devlink_dev -j | \
+			    jq -e -r ".[][].versions.running.fw")
+	check_err $? "Failed to get linecard $lc running.fw.version"
+	log_info "Linecard $lc running.fw: \"$running_device_fw\""
+}
+
 activation_16x100G_test()
 {
 	RET=0
 	local lc
 	local type
 	local state
+	local nested_devlink_dev
 
 	lc=$LC_SLOT
 	type=$LC_16X100G_TYPE
@@ -268,6 +288,10 @@ activation_16x100G_test()
 
 	interface_check
 
+	nested_devlink_dev=$(lc_nested_devlink_dev_get $lc)
+	check_err $? "Failed to get nested devlink handle of linecard $lc"
+	lc_dev_info_active_check $lc $nested_devlink_dev
+
 	log_test "Activation 16x100G"
 }
 
-- 
2.35.3

Re: [patch net-next v2 00/12] mlxsw: Implement dev info and dev flash for line cards

[Ido Schimmel]

On Tue, Jul 19, 2022 at 08:48:35AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> This patchset implements two features:
> 1) "devlink dev info" is exposed for line card (patches 5-9)
> 2) "devlink dev flash" is implemented for line card gearbox
>    flashing (patch 10)
> 
> For every line card, "a nested" auxiliary device is created which
> allows to bind the features mentioned above (patch 3).
> 
> The relationship between line card and its auxiliary dev devlink
> is carried over extra line card netlink attribute (patches 2 and 4).
> 
> The first patch removes devlink_mutex from devlink_register/unregister()
> which eliminates possible deadlock during devlink reload command.

Will review today/tomorrow. In the meantime I have applied it for
testing.