From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C77A6C25B50 for ; Fri, 20 Jan 2023 11:02:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230055AbjATLCk (ORCPT ); Fri, 20 Jan 2023 06:02:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33016 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229947AbjATLCj (ORCPT ); Fri, 20 Jan 2023 06:02:39 -0500 Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F25CA2D4C for ; Fri, 20 Jan 2023 03:02:35 -0800 (PST) Received: by mail-ed1-x52d.google.com with SMTP id g11so800134eda.12 for ; Fri, 20 Jan 2023 03:02:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=w3BQGuS5wR8NiA6sT4SKojEob1eFqC1AJe9Sq0A+Q5o=; b=kkEB009RJIiBrnEhLTuanYu7zoM9xYU5bFgJ32BNK/2ozzOuakzW5U1EyMIeDR9kFo N/ozXdBAPRN6FHS/2AFLzlbS8Iw+WTj0Cqrph7VPxNPtPJeaZ3hvOzzTvDdDX3ZjqNsz MnRyPtxpDbE9QPPgEgLoEzArOqOqlsxiD6wqWYgu/NgKjUY6/0TKmpos7VZnhSEFUDN3 5ceTnfHKXkkkUGFIA35P289YX6Wic6E2+FtNbncn3DAxoRTV6imXOqCC9PKCqB4Fd//1 7uetnQJpVvjoTmY/nErWdNXh3gCp+f0R59lHdW/ZmX0KM40Ee2reRX6lr29bLmN1ZZMt 0Mcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=w3BQGuS5wR8NiA6sT4SKojEob1eFqC1AJe9Sq0A+Q5o=; b=nUZ5T5ozuHYFFCulVyd+MMX0182BBaF+Wm8iwY2PAOqy84W1/aHsIs2nALklX8AqcR Md8/C5HAkEMn3ahbl9LzVgkQAREBkoM/TuwzZVobXE6erGmKvR39D4IaiAS7pKrL+knN 2+h/xeuUSPRZY/+PNaDYCmWGpELL9fqcQo8rS/AvLzrzHswzeOYEJ5q0achjvulvxxpU fXpUHmqFL5mfmzrndTDdTSJbQaiAWLy9zn1u17fgzsYCvBmWc/9iDXKpvi3ssxW7wnRK xdsrOe0bXvM+XSuFRoiIBLkwCRnzECvulDbm2JCSU+afBP4pSGRvfXQEH6leKAOgk7s6 nJqQ== X-Gm-Message-State: AFqh2ko8XHRjhawm6K239Mi62txezJg6mCQDnay2yTPYG94lBIsmGMyJ o4bi9C3BN2gur0Sc1deMdDJzCg== X-Google-Smtp-Source: AMrXdXtAddiSr7xyX1k9NCPPD2YzCCeRnl28gXzO0ov+spVfP4XAOvGkzClMYN5e4tt6yEOea5GLrw== X-Received: by 2002:a05:6402:524f:b0:49e:910:5706 with SMTP id t15-20020a056402524f00b0049e09105706mr20591317edd.2.1674212554321; Fri, 20 Jan 2023 03:02:34 -0800 (PST) Received: from localhost ([217.111.27.204]) by smtp.gmail.com with ESMTPSA id v15-20020a056402184f00b0046c5baa1f58sm16980263edy.97.2023.01.20.03.02.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Jan 2023 03:02:33 -0800 (PST) Date: Fri, 20 Jan 2023 12:02:32 +0100 From: Jiri Pirko To: "Lucero Palau, Alejandro" Cc: "netdev@vger.kernel.org" , "linux-net-drivers (AMD-Xilinx)" , "davem@davemloft.net" , "kuba@kernel.org" , "pabeni@redhat.com" , "edumazet@google.com" , "habetsm@gmail.com" , "ecree.xilinx@gmail.com" Subject: Re: [PATCH net-next 1/7] sfc: add devlink support for ef100 Message-ID: References: <20230119113140.20208-1-alejandro.lucero-palau@amd.com> <20230119113140.20208-2-alejandro.lucero-palau@amd.com> <11c59e1f-0465-09c1-638b-d79447752735@amd.com> <29526f6b-5c63-be03-69c6-11b6ed3158aa@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <29526f6b-5c63-be03-69c6-11b6ed3158aa@amd.com> Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Thu, Jan 19, 2023 at 05:29:37PM CET, alejandro.lucero-palau@amd.com wrote: > >On 1/19/23 14:51, Lucero Palau, Alejandro wrote: >> On 1/19/23 12:14, Jiri Pirko wrote: >>> Thu, Jan 19, 2023 at 12:31:34PM CET, alejandro.lucero-palau@amd.com wrote: >>>> From: Alejandro Lucero >>>> >>>> Basic support for devlink info command. >>>> >>>> Signed-off-by: Alejandro Lucero >>>> --- >>>> drivers/net/ethernet/sfc/Kconfig | 1 + >>>> drivers/net/ethernet/sfc/Makefile | 3 +- >>>> drivers/net/ethernet/sfc/ef100_nic.c | 6 + >>>> drivers/net/ethernet/sfc/efx_devlink.c | 427 +++++++++++++++++++++++++ >>>> drivers/net/ethernet/sfc/efx_devlink.h | 20 ++ >>>> drivers/net/ethernet/sfc/mcdi.c | 72 +++++ >>>> drivers/net/ethernet/sfc/mcdi.h | 3 + >>>> drivers/net/ethernet/sfc/net_driver.h | 2 + >>>> 8 files changed, 533 insertions(+), 1 deletion(-) >>>> create mode 100644 drivers/net/ethernet/sfc/efx_devlink.c >>>> create mode 100644 drivers/net/ethernet/sfc/efx_devlink.h >>> Could you please split? >>> >>> At least the devlink introduction part and info implementation. >>> >> Sure. > >Just for being sure, would it be fine to have a simple info >implementation first along with the devlink infrastructure then a second >patch adding the more complex devlink info support? I guess I need at >least one operation supported initially. No, it does not need to support any op. > >>>> diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig >>>> index 0950e6b0508f..4af36ba8906b 100644 >>>> --- a/drivers/net/ethernet/sfc/Kconfig >>>> +++ b/drivers/net/ethernet/sfc/Kconfig >>>> @@ -22,6 +22,7 @@ config SFC >>>> depends on PTP_1588_CLOCK_OPTIONAL >>>> select MDIO >>>> select CRC32 >>>> + select NET_DEVLINK >>>> help >>>> This driver supports 10/40-gigabit Ethernet cards based on >>>> the Solarflare SFC9100-family controllers. >>>> diff --git a/drivers/net/ethernet/sfc/Makefile b/drivers/net/ethernet/sfc/Makefile >>>> index 712a48d00069..55b9c73cd8ef 100644 >>>> --- a/drivers/net/ethernet/sfc/Makefile >>>> +++ b/drivers/net/ethernet/sfc/Makefile >>>> @@ -6,7 +6,8 @@ sfc-y += efx.o efx_common.o efx_channels.o nic.o \ >>>> mcdi.o mcdi_port.o mcdi_port_common.o \ >>>> mcdi_functions.o mcdi_filters.o mcdi_mon.o \ >>>> ef100.o ef100_nic.o ef100_netdev.o \ >>>> - ef100_ethtool.o ef100_rx.o ef100_tx.o >>>> + ef100_ethtool.o ef100_rx.o ef100_tx.o \ >>>> + efx_devlink.o >>>> sfc-$(CONFIG_SFC_MTD) += mtd.o >>>> sfc-$(CONFIG_SFC_SRIOV) += sriov.o ef10_sriov.o ef100_sriov.o ef100_rep.o \ >>>> mae.o tc.o tc_bindings.o tc_counters.o >>>> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c >>>> index ad686c671ab8..14af8f314b83 100644 >>>> --- a/drivers/net/ethernet/sfc/ef100_nic.c >>>> +++ b/drivers/net/ethernet/sfc/ef100_nic.c >>>> @@ -27,6 +27,7 @@ >>>> #include "tc.h" >>>> #include "mae.h" >>>> #include "rx_common.h" >>>> +#include "efx_devlink.h" >>>> >>>> #define EF100_MAX_VIS 4096 >>>> #define EF100_NUM_MCDI_BUFFERS 1 >>>> @@ -1124,6 +1125,10 @@ int ef100_probe_netdev_pf(struct efx_nic *efx) >>>> netif_warn(efx, probe, net_dev, >>>> "Failed to probe base mport rc %d; representors will not function\n", >>>> rc); >>>> + } else { >>>> + if (efx_probe_devlink(efx)) >>> I don't understand why the devlink register call is here. >>> 1) You can registers it for PF and VF. I don't follow why you do it only >>> for PF. >> We discuss the possibility of creating the devlink interface for VFs, >> but we decided not to. Arguably, this should not be available for VFs >> owned by VMs/containers, or at least in our case, since the interface is >> being used for getting information about the whole device or for >> getting/setting the MAC address. We plan to support more devlink >> functionalities in the near future, so maybe we add such support then if >> we consider it is needed. >> >>> 2) It should be done as the first thing in the probe flow. Then devlink >>> port register, only after that netdev. It makes sense from the >>> perspective of object hierarchy. >> I can not see a problem here. It is not the first thing done in the >> probe function, but I can not see any dependency for being so. And it is >> done before the devlink port registration and before the netdev >> registration what seems to be what other drivers do. What am I missing >> here? >> >>> It is also usual to have the devlink priv as the "main" driver >>> structure storage, see how that is done in mlx5 of mlxsw for example. >> I will check the way others drivers use the devlink priv. > > >Interestingly, there are 26 calls to devlink_alloc with half of them >using the main driver structure and the other half using a specific >devlink private struct. This is a huge responsibility now for me! > >I'm afraid, for the shake of having this merged as soon as possible, >I'll keep the current allocation. I did not suffer any problem with this >approach while implementing the current support. If, for the further >devlink support we plan to add, it turns out to be an issue, we will to >the changes then. I would say it is a main change now or then, and >current work does not justify it now. Up to you, it will eventualy bite you in the *** when you implement devlink reload for example. > > >>> >>>> + netif_warn(efx, probe, net_dev, >>>> + "Failed to register devlink\n"); >>>> } >>>> >>>> rc = efx_init_tc(efx); >>>> @@ -1157,6 +1162,7 @@ void ef100_remove(struct efx_nic *efx) >>>> { >>>> struct ef100_nic_data *nic_data = efx->nic_data; >>>> >>>> + efx_fini_devlink(efx); >>>> efx_mcdi_detach(efx); >>>> efx_mcdi_fini(efx); >>>> if (nic_data) >>>> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c >>>> new file mode 100644 >>>> index 000000000000..c506f8f35d25 >>>> --- /dev/null >>>> +++ b/drivers/net/ethernet/sfc/efx_devlink.c >>>> @@ -0,0 +1,427 @@ >>>> +// SPDX-License-Identifier: GPL-2.0-only >>>> +/**************************************************************************** >>>> + * Driver for AMD network controllers and boards >>>> + * Copyright (C) 2023, Advanced Micro Devices, Inc. >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify it >>>> + * under the terms of the GNU General Public License version 2 as published >>>> + * by the Free Software Foundation, incorporated herein by reference. >>>> + */ >>>> + >>>> +#include >>>> +#include "net_driver.h" >>>> +#include "ef100_nic.h" >>>> +#include "efx_devlink.h" >>>> +#include "nic.h" >>>> +#include "mcdi.h" >>>> +#include "mcdi_functions.h" >>>> +#include "mcdi_pcol.h" >>>> + >>>> +/* Custom devlink-info version object names for details that do not map to the >>>> + * generic standardized names. >>>> + */ >>>> +#define EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC "fw.mgmt.suc" >>>> +#define EFX_DEVLINK_INFO_VERSION_FW_MGMT_CMC "fw.mgmt.cmc" >>>> +#define EFX_DEVLINK_INFO_VERSION_FPGA_REV "fpga.rev" >>>> +#define EFX_DEVLINK_INFO_VERSION_DATAPATH_HW "fpga.app" >>>> +#define EFX_DEVLINK_INFO_VERSION_DATAPATH_FW DEVLINK_INFO_VERSION_GENERIC_FW_APP >>>> +#define EFX_DEVLINK_INFO_VERSION_SOC_BOOT "coproc.boot" >>>> +#define EFX_DEVLINK_INFO_VERSION_SOC_UBOOT "coproc.uboot" >>>> +#define EFX_DEVLINK_INFO_VERSION_SOC_MAIN "coproc.main" >>>> +#define EFX_DEVLINK_INFO_VERSION_SOC_RECOVERY "coproc.recovery" >>>> +#define EFX_DEVLINK_INFO_VERSION_FW_EXPROM "fw.exprom" >>>> +#define EFX_DEVLINK_INFO_VERSION_FW_UEFI "fw.uefi" >>>> + >>>> +#define EFX_MAX_VERSION_INFO_LEN 64 >>>> + >>>> +static int efx_devlink_info_nvram_partition(struct efx_nic *efx, >>>> + struct devlink_info_req *req, >>>> + unsigned int partition_type, >>>> + const char *version_name) >>>> +{ >>>> + char buf[EFX_MAX_VERSION_INFO_LEN]; >>>> + u16 version[4]; >>>> + int rc; >>>> + >>>> + rc = efx_mcdi_nvram_metadata(efx, partition_type, NULL, version, NULL, >>>> + 0); >>>> + if (rc) >>>> + return rc; >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", version[0], >>>> + version[1], version[2], version[3]); >>>> + devlink_info_version_stored_put(req, version_name, buf); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static void efx_devlink_info_stored_versions(struct efx_nic *efx, >>>> + struct devlink_info_req *req) >>>> +{ >>>> + efx_devlink_info_nvram_partition(efx, req, NVRAM_PARTITION_TYPE_BUNDLE, >>>> + DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID); >>>> + efx_devlink_info_nvram_partition(efx, req, >>>> + NVRAM_PARTITION_TYPE_MC_FIRMWARE, >>>> + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT); >>>> + efx_devlink_info_nvram_partition(efx, req, >>>> + NVRAM_PARTITION_TYPE_SUC_FIRMWARE, >>>> + EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC); >>>> + efx_devlink_info_nvram_partition(efx, req, >>>> + NVRAM_PARTITION_TYPE_EXPANSION_ROM, >>>> + EFX_DEVLINK_INFO_VERSION_FW_EXPROM); >>>> + efx_devlink_info_nvram_partition(efx, req, >>>> + NVRAM_PARTITION_TYPE_EXPANSION_UEFI, >>>> + EFX_DEVLINK_INFO_VERSION_FW_UEFI); >>>> +} >>>> + >>>> +#define EFX_MAX_SERIALNUM_LEN (ETH_ALEN * 2 + 1) >>>> + >>>> +static void efx_devlink_info_board_cfg(struct efx_nic *efx, >>>> + struct devlink_info_req *req) >>>> +{ >>>> + char sn[EFX_MAX_SERIALNUM_LEN]; >>>> + u8 mac_address[ETH_ALEN]; >>>> + int rc; >>>> + >>>> + rc = efx_mcdi_get_board_cfg(efx, (u8 *)mac_address, NULL, NULL); >>>> + if (!rc) { >>>> + snprintf(sn, EFX_MAX_SERIALNUM_LEN, "%pm", mac_address); >>>> + devlink_info_serial_number_put(req, sn); >>>> + } >>>> +} >>>> + >>>> +#define EFX_VER_FLAG(_f) \ >>>> + (MC_CMD_GET_VERSION_V5_OUT_ ## _f ## _PRESENT_LBN) >>>> + >>>> +static void efx_devlink_info_running_versions(struct efx_nic *efx, >>>> + struct devlink_info_req *req) >>>> +{ >>>> + MCDI_DECLARE_BUF(outbuf, MC_CMD_GET_VERSION_V5_OUT_LEN); >>>> + MCDI_DECLARE_BUF(inbuf, MC_CMD_GET_VERSION_EXT_IN_LEN); >>>> + char buf[EFX_MAX_VERSION_INFO_LEN]; >>>> + unsigned int flags, build_id; >>>> + union { >>>> + const __le32 *dwords; >>>> + const __le16 *words; >>>> + const char *str; >>>> + } ver; >>>> + struct rtc_time build_date; >>>> + size_t outlength, offset; >>>> + u64 tstamp; >>>> + int rc; >>>> + >>>> + rc = efx_mcdi_rpc(efx, MC_CMD_GET_VERSION, inbuf, sizeof(inbuf), >>>> + outbuf, sizeof(outbuf), &outlength); >>>> + if (rc || outlength < MC_CMD_GET_VERSION_OUT_LEN) >>>> + return; >>>> + >>>> + /* Handle previous output */ >>>> + if (outlength < MC_CMD_GET_VERSION_V2_OUT_LEN) { >>>> + ver.words = (__le16 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_EXT_OUT_VERSION); >>>> + offset = snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le16_to_cpu(ver.words[0]), >>>> + le16_to_cpu(ver.words[1]), >>>> + le16_to_cpu(ver.words[2]), >>>> + le16_to_cpu(ver.words[3])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, >>>> + buf); >>>> + return; >>>> + } >>>> + >>>> + /* Handle V2 additions */ >>>> + flags = MCDI_DWORD(outbuf, GET_VERSION_V2_OUT_FLAGS); >>>> + if (flags & BIT(EFX_VER_FLAG(BOARD_EXT_INFO))) { >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%s", >>>> + MCDI_PTR(outbuf, GET_VERSION_V2_OUT_BOARD_NAME)); >>>> + devlink_info_version_fixed_put(req, >>>> + DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, >>>> + buf); >>>> + >>>> + /* Favour full board version if present (in V5 or later) */ >>>> + if (~flags & BIT(EFX_VER_FLAG(BOARD_VERSION))) { >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u", >>>> + MCDI_DWORD(outbuf, >>>> + GET_VERSION_V2_OUT_BOARD_REVISION)); >>>> + devlink_info_version_fixed_put(req, >>>> + DEVLINK_INFO_VERSION_GENERIC_BOARD_REV, >>>> + buf); >>>> + } >>>> + >>>> + ver.str = MCDI_PTR(outbuf, GET_VERSION_V2_OUT_BOARD_SERIAL); >>>> + if (ver.str[0]) >>>> + devlink_info_board_serial_number_put(req, ver.str); >>>> + } >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(FPGA_EXT_INFO))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V2_OUT_FPGA_VERSION); >>>> + offset = snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u_%c%u", >>>> + le32_to_cpu(ver.dwords[0]), >>>> + 'A' + le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2])); >>>> + >>>> + ver.str = MCDI_PTR(outbuf, GET_VERSION_V2_OUT_FPGA_EXTRA); >>>> + if (ver.str[0]) >>>> + snprintf(&buf[offset], EFX_MAX_VERSION_INFO_LEN - offset, >>>> + " (%s)", ver.str); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_FPGA_REV, >>>> + buf); >>>> + } >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(CMC_EXT_INFO))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V2_OUT_CMCFW_VERSION); >>>> + offset = snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), >>>> + le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), >>>> + le32_to_cpu(ver.dwords[3])); >>>> + >>>> + tstamp = MCDI_QWORD(outbuf, >>>> + GET_VERSION_V2_OUT_CMCFW_BUILD_DATE); >>>> + if (tstamp) { >>>> + rtc_time64_to_tm(tstamp, &build_date); >>>> + snprintf(&buf[offset], EFX_MAX_VERSION_INFO_LEN - offset, >>>> + " (%ptRd)", &build_date); >>>> + } >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_FW_MGMT_CMC, >>>> + buf); >>>> + } >>>> + >>>> + ver.words = (__le16 *)MCDI_PTR(outbuf, GET_VERSION_V2_OUT_VERSION); >>>> + offset = snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le16_to_cpu(ver.words[0]), le16_to_cpu(ver.words[1]), >>>> + le16_to_cpu(ver.words[2]), le16_to_cpu(ver.words[3])); >>>> + if (flags & BIT(EFX_VER_FLAG(MCFW_EXT_INFO))) { >>>> + build_id = MCDI_DWORD(outbuf, GET_VERSION_V2_OUT_MCFW_BUILD_ID); >>>> + snprintf(&buf[offset], EFX_MAX_VERSION_INFO_LEN - offset, >>>> + " (%x) %s", build_id, >>>> + MCDI_PTR(outbuf, GET_VERSION_V2_OUT_MCFW_BUILD_NAME)); >>>> + } >>>> + devlink_info_version_running_put(req, >>>> + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, >>>> + buf); >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(SUCFW_EXT_INFO))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V2_OUT_SUCFW_VERSION); >>>> + tstamp = MCDI_QWORD(outbuf, >>>> + GET_VERSION_V2_OUT_SUCFW_BUILD_DATE); >>>> + rtc_time64_to_tm(tstamp, &build_date); >>>> + build_id = MCDI_DWORD(outbuf, GET_VERSION_V2_OUT_SUCFW_CHIP_ID); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, >>>> + "%u.%u.%u.%u type %x (%ptRd)", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), le32_to_cpu(ver.dwords[3]), >>>> + build_id, &build_date); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC, >>>> + buf); >>>> + } >>>> + >>>> + if (outlength < MC_CMD_GET_VERSION_V3_OUT_LEN) >>>> + return; >>>> + >>>> + /* Handle V3 additions */ >>>> + if (flags & BIT(EFX_VER_FLAG(DATAPATH_HW_VERSION))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V3_OUT_DATAPATH_HW_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_DATAPATH_HW, >>>> + buf); >>>> + } >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(DATAPATH_FW_VERSION))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V3_OUT_DATAPATH_FW_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_DATAPATH_FW, >>>> + buf); >>>> + } >>>> + >>>> + if (outlength < MC_CMD_GET_VERSION_V4_OUT_LEN) >>>> + return; >>>> + >>>> + /* Handle V4 additions */ >>>> + if (flags & BIT(EFX_VER_FLAG(SOC_BOOT_VERSION))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V4_OUT_SOC_BOOT_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), >>>> + le32_to_cpu(ver.dwords[3])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_SOC_BOOT, >>>> + buf); >>>> + } >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(SOC_UBOOT_VERSION))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V4_OUT_SOC_UBOOT_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), >>>> + le32_to_cpu(ver.dwords[3])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_SOC_UBOOT, >>>> + buf); >>>> + } >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(SOC_MAIN_ROOTFS_VERSION))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V4_OUT_SOC_MAIN_ROOTFS_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), >>>> + le32_to_cpu(ver.dwords[3])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_SOC_MAIN, >>>> + buf); >>>> + } >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(SOC_RECOVERY_BUILDROOT_VERSION))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V4_OUT_SOC_RECOVERY_BUILDROOT_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), >>>> + le32_to_cpu(ver.dwords[3])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_SOC_RECOVERY, >>>> + buf); >>>> + } >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(SUCFW_VERSION)) && >>>> + ~flags & BIT(EFX_VER_FLAG(SUCFW_EXT_INFO))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V4_OUT_SUCFW_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), >>>> + le32_to_cpu(ver.dwords[3])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC, >>>> + buf); >>>> + } >>>> + >>>> + if (outlength < MC_CMD_GET_VERSION_V5_OUT_LEN) >>>> + return; >>>> + >>>> + /* Handle V5 additions */ >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(BOARD_VERSION))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V5_OUT_BOARD_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), >>>> + le32_to_cpu(ver.dwords[3])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + DEVLINK_INFO_VERSION_GENERIC_BOARD_REV, >>>> + buf); >>>> + } >>>> + >>>> + if (flags & BIT(EFX_VER_FLAG(BUNDLE_VERSION))) { >>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf, >>>> + GET_VERSION_V5_OUT_BUNDLE_VERSION); >>>> + >>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", >>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]), >>>> + le32_to_cpu(ver.dwords[2]), >>>> + le32_to_cpu(ver.dwords[3])); >>>> + >>>> + devlink_info_version_running_put(req, >>>> + DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID, >>>> + buf); >>>> + } >>>> +} >>>> + >>>> +#undef EFX_VER_FLAG >>>> + >>>> +static void efx_devlink_info_query_all(struct efx_nic *efx, >>>> + struct devlink_info_req *req) >>>> +{ >>>> + efx_devlink_info_board_cfg(efx, req); >>>> + efx_devlink_info_stored_versions(efx, req); >>>> + efx_devlink_info_running_versions(efx, req); >>>> +} >>>> + >>>> +struct efx_devlink { >>>> + struct efx_nic *efx; >>>> +}; >>>> + >>>> +static int efx_devlink_info_get(struct devlink *devlink, >>>> + struct devlink_info_req *req, >>>> + struct netlink_ext_ack *extack) >>>> +{ >>>> + struct efx_devlink *devlink_private = devlink_priv(devlink); >>>> + struct efx_nic *efx = devlink_private->efx; >>>> + >>>> + efx_devlink_info_query_all(efx, req); >>> I don't understand the reason for having efx_devlink_info_query_all() as >>> a separate function in compare to inline its code here. >>> >> Yes, it could be do as you say. I'll do. >> >> Thanks >> >>>> + return 0; >>>> +} >>>> + >>>> +static const struct devlink_ops sfc_devlink_ops = { >>>> + .info_get = efx_devlink_info_get, >>>> +}; >>>> + >>>> +void efx_fini_devlink(struct efx_nic *efx) >>>> +{ >>>> + if (efx->devlink) { >>>> + struct efx_devlink *devlink_private; >>>> + >>>> + devlink_private = devlink_priv(efx->devlink); >>>> + >>>> + devlink_unregister(efx->devlink); >>>> + devlink_free(efx->devlink); >>>> + efx->devlink = NULL; >>>> + } >>>> +} >>>> + >>>> +int efx_probe_devlink(struct efx_nic *efx) >>>> +{ >>>> + struct efx_devlink *devlink_private; >>>> + >>>> + efx->devlink = devlink_alloc(&sfc_devlink_ops, >>>> + sizeof(struct efx_devlink), >>>> + &efx->pci_dev->dev); >>>> + if (!efx->devlink) >>>> + return -ENOMEM; >>>> + devlink_private = devlink_priv(efx->devlink); >>>> + devlink_private->efx = efx; >>>> + >>>> + devlink_register(efx->devlink); >>>> + >>>> + return 0; >>>> +} >>>> diff --git a/drivers/net/ethernet/sfc/efx_devlink.h b/drivers/net/ethernet/sfc/efx_devlink.h >>>> new file mode 100644 >>>> index 000000000000..997f878aea93 >>>> --- /dev/null >>>> +++ b/drivers/net/ethernet/sfc/efx_devlink.h >>>> @@ -0,0 +1,20 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>>> +/**************************************************************************** >>>> + * Driver for AMD network controllers and boards >>>> + * Copyright (C) 2023, Advanced Micro Devices, Inc. >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify it >>>> + * under the terms of the GNU General Public License version 2 as published >>>> + * by the Free Software Foundation, incorporated herein by reference. >>>> + */ >>>> + >>>> +#ifndef _EFX_DEVLINK_H >>>> +#define _EFX_DEVLINK_H >>>> + >>>> +#include "net_driver.h" >>>> +#include >>>> + >>>> +int efx_probe_devlink(struct efx_nic *efx); >>>> +void efx_fini_devlink(struct efx_nic *efx); >>>> + >>>> +#endif /* _EFX_DEVLINK_H */ >>>> diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c >>>> index af338208eae9..328cae82a7d8 100644 >>>> --- a/drivers/net/ethernet/sfc/mcdi.c >>>> +++ b/drivers/net/ethernet/sfc/mcdi.c >>>> @@ -2308,6 +2308,78 @@ static int efx_mcdi_nvram_update_finish(struct efx_nic *efx, unsigned int type) >>>> return rc; >>>> } >>>> >>>> +int efx_mcdi_nvram_metadata(struct efx_nic *efx, unsigned int type, >>>> + u32 *subtype, u16 version[4], char *desc, >>>> + size_t descsize) >>>> +{ >>>> + MCDI_DECLARE_BUF(inbuf, MC_CMD_NVRAM_METADATA_IN_LEN); >>>> + efx_dword_t *outbuf; >>>> + size_t outlen; >>>> + u32 flags; >>>> + int rc; >>>> + >>>> + outbuf = kzalloc(MC_CMD_NVRAM_METADATA_OUT_LENMAX_MCDI2, GFP_KERNEL); >>>> + if (!outbuf) >>>> + return -ENOMEM; >>>> + >>>> + MCDI_SET_DWORD(inbuf, NVRAM_METADATA_IN_TYPE, type); >>>> + >>>> + rc = efx_mcdi_rpc_quiet(efx, MC_CMD_NVRAM_METADATA, inbuf, >>>> + sizeof(inbuf), outbuf, >>>> + MC_CMD_NVRAM_METADATA_OUT_LENMAX_MCDI2, >>>> + &outlen); >>>> + if (rc) >>>> + goto out_free; >>>> + if (outlen < MC_CMD_NVRAM_METADATA_OUT_LENMIN) { >>>> + rc = -EIO; >>>> + goto out_free; >>>> + } >>>> + >>>> + flags = MCDI_DWORD(outbuf, NVRAM_METADATA_OUT_FLAGS); >>>> + >>>> + if (desc && descsize > 0) { >>>> + if (flags & BIT(MC_CMD_NVRAM_METADATA_OUT_DESCRIPTION_VALID_LBN)) { >>>> + if (descsize <= >>>> + MC_CMD_NVRAM_METADATA_OUT_DESCRIPTION_NUM(outlen)) { >>>> + rc = -E2BIG; >>>> + goto out_free; >>>> + } >>>> + >>>> + strncpy(desc, >>>> + MCDI_PTR(outbuf, NVRAM_METADATA_OUT_DESCRIPTION), >>>> + MC_CMD_NVRAM_METADATA_OUT_DESCRIPTION_NUM(outlen)); >>>> + desc[MC_CMD_NVRAM_METADATA_OUT_DESCRIPTION_NUM(outlen)] = '\0'; >>>> + } else { >>>> + desc[0] = '\0'; >>>> + } >>>> + } >>>> + >>>> + if (subtype) { >>>> + if (flags & BIT(MC_CMD_NVRAM_METADATA_OUT_SUBTYPE_VALID_LBN)) >>>> + *subtype = MCDI_DWORD(outbuf, NVRAM_METADATA_OUT_SUBTYPE); >>>> + else >>>> + *subtype = 0; >>>> + } >>>> + >>>> + if (version) { >>>> + if (flags & BIT(MC_CMD_NVRAM_METADATA_OUT_VERSION_VALID_LBN)) { >>>> + version[0] = MCDI_WORD(outbuf, NVRAM_METADATA_OUT_VERSION_W); >>>> + version[1] = MCDI_WORD(outbuf, NVRAM_METADATA_OUT_VERSION_X); >>>> + version[2] = MCDI_WORD(outbuf, NVRAM_METADATA_OUT_VERSION_Y); >>>> + version[3] = MCDI_WORD(outbuf, NVRAM_METADATA_OUT_VERSION_Z); >>>> + } else { >>>> + version[0] = 0; >>>> + version[1] = 0; >>>> + version[2] = 0; >>>> + version[3] = 0; >>>> + } >>>> + } >>>> + >>>> +out_free: >>>> + kfree(outbuf); >>>> + return rc; >>>> +} >>>> + >>>> int efx_mcdi_mtd_read(struct mtd_info *mtd, loff_t start, >>>> size_t len, size_t *retlen, u8 *buffer) >>>> { >>>> diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h >>>> index 7e35fec9da35..63b090587f7a 100644 >>>> --- a/drivers/net/ethernet/sfc/mcdi.h >>>> +++ b/drivers/net/ethernet/sfc/mcdi.h >>>> @@ -379,6 +379,9 @@ int efx_mcdi_nvram_info(struct efx_nic *efx, unsigned int type, >>>> bool *protected_out); >>>> int efx_new_mcdi_nvram_test_all(struct efx_nic *efx); >>>> int efx_mcdi_nvram_test_all(struct efx_nic *efx); >>>> +int efx_mcdi_nvram_metadata(struct efx_nic *efx, unsigned int type, >>>> + u32 *subtype, u16 version[4], char *desc, >>>> + size_t descsize); >>>> int efx_mcdi_handle_assertion(struct efx_nic *efx); >>>> int efx_mcdi_set_id_led(struct efx_nic *efx, enum efx_led_mode mode); >>>> int efx_mcdi_wol_filter_set_magic(struct efx_nic *efx, const u8 *mac, >>>> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h >>>> index 3b49e216768b..d036641dc043 100644 >>>> --- a/drivers/net/ethernet/sfc/net_driver.h >>>> +++ b/drivers/net/ethernet/sfc/net_driver.h >>>> @@ -994,6 +994,7 @@ enum efx_xdp_tx_queues_mode { >>>> * xdp_rxq_info structures? >>>> * @netdev_notifier: Netdevice notifier. >>>> * @tc: state for TC offload (EF100). >>>> + * @devlink: reference to devlink structure owned by this device >>>> * @mem_bar: The BAR that is mapped into membase. >>>> * @reg_base: Offset from the start of the bar to the function control window. >>>> * @monitor_work: Hardware monitor workitem >>>> @@ -1179,6 +1180,7 @@ struct efx_nic { >>>> struct notifier_block netdev_notifier; >>>> struct efx_tc_state *tc; >>>> >>>> + struct devlink *devlink; >>>> unsigned int mem_bar; >>>> u32 reg_base; >>>> >>>> -- >>>> 2.17.1 >>>> >