| Message ID | 20220719064847.3688226-8-jiri@resnulli.us (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mlxsw: Implement dev info and dev flash for line cards | expand |
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.
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".
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.
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.
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)