From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [RFC net-next 4/6] nfp: devlink: report fixed versions Date: Tue, 15 Jan 2019 11:18:18 +0100 Message-ID: <20190115101818.GE2290@nanopsycho> References: <20190115005009.16025-1-jakub.kicinski@netronome.com> <20190115005009.16025-5-jakub.kicinski@netronome.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: davem@davemloft.net, netdev@vger.kernel.org, oss-drivers@netronome.com To: Jakub Kicinski Return-path: Received: from mail-wm1-f65.google.com ([209.85.128.65]:40725 "EHLO mail-wm1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727369AbfAOK0w (ORCPT ); Tue, 15 Jan 2019 05:26:52 -0500 Received: by mail-wm1-f65.google.com with SMTP id f188so2579812wmf.5 for ; Tue, 15 Jan 2019 02:26:51 -0800 (PST) Content-Disposition: inline In-Reply-To: <20190115005009.16025-5-jakub.kicinski@netronome.com> Sender: netdev-owner@vger.kernel.org List-ID: Tue, Jan 15, 2019 at 01:50:06AM CET, jakub.kicinski@netronome.com wrote: >For now we only use HWinfo for fixed versions. > >Signed-off-by: Jakub Kicinski >--- > .../net/ethernet/netronome/nfp/nfp_devlink.c | 72 +++++++++++++++++++ > 1 file changed, 72 insertions(+) > >diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c >index 4422da939937..c7759564316d 100644 >--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c >+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c >@@ -192,6 +192,77 @@ nfp_devlink_serial_get(struct devlink *devlink, u8 *buf, size_t buf_len, > return 0; > } > >+static const struct nfp_devlink_versions_simple { >+ const char *key; >+ const char *hwinfo; >+} nfp_devlink_versions_hwinfo[] = { >+ { "board.model", "assembly.model", }, >+ { "board.partno", "assembly.partno", }, >+ { "board.revision", "assembly.revision", }, >+ { "board.vendor", "assembly.vendor", }, That means that every driver would re-define these strings. Would it make sense to have them (at least some of them) defined in a generic way? Something in a sense of devlink params, where we have generic and driver-specific ones. >+}; >+ >+static int >+nfp_devlink_versions_get_hwinfo(struct sk_buff *skb, struct nfp_nsp *nsp, >+ struct netlink_ext_ack *extack) >+{ >+ unsigned int i; >+ int attr; >+ int err; >+ >+ attr = DEVLINK_ATTR_INFO_VERSIONS_FIXED; >+ >+ for (i = 0; i < ARRAY_SIZE(nfp_devlink_versions_hwinfo); i++) { >+ const struct nfp_devlink_versions_simple *info; >+ char buf[256] = {}; >+ >+ info = &nfp_devlink_versions_hwinfo[i]; >+ strcpy(buf, info->hwinfo); >+ >+ err = nfp_nsp_hwinfo_lookup(nsp, buf, sizeof(buf)); >+ if (err == -ENOENT) >+ continue; >+ if (err) { >+ NL_SET_ERR_MSG_MOD(extack, >+ "error reading versions string from FW"); >+ return err; >+ } >+ >+ err = devlink_versions_report(skb, attr, info->key, buf); So this is always "DEVLINK_ATTR_INFO_VERSIONS_FIXED". I don't understand. >+ if (err) >+ return err; >+ } >+ >+ return 0; >+} >+ >+static int >+nfp_devlink_versions_get(struct devlink *devlink, struct sk_buff *skb, >+ struct netlink_ext_ack *extack) >+{ >+ struct nfp_pf *pf = devlink_priv(devlink); >+ struct nfp_nsp *nsp; >+ int err; >+ >+ nsp = nfp_nsp_open(pf->cpp); >+ if (IS_ERR(nsp)) { >+ NL_SET_ERR_MSG_MOD(extack, "can't access NSP"); >+ return PTR_ERR(nsp); >+ } >+ >+ err = nfp_devlink_versions_get_hwinfo(skb, nsp, extack); >+ if (err) >+ goto err_close_nsp; >+ >+ nfp_nsp_close(nsp); >+ >+ return 0; >+ >+err_close_nsp: >+ nfp_nsp_close(nsp); >+ return err; >+} >+ > const struct devlink_ops nfp_devlink_ops = { > .port_split = nfp_devlink_port_split, > .port_unsplit = nfp_devlink_port_unsplit, >@@ -200,6 +271,7 @@ const struct devlink_ops nfp_devlink_ops = { > .eswitch_mode_get = nfp_devlink_eswitch_mode_get, > .eswitch_mode_set = nfp_devlink_eswitch_mode_set, > .serial_get = nfp_devlink_serial_get, >+ .versions_get = nfp_devlink_versions_get, > }; > > int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port) >-- >2.19.2 >