From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Subject: Re: [RFC net-next 4/6] nfp: devlink: report fixed versions
Date: Tue, 15 Jan 2019 10:09:13 -0800
Message-ID: <20190115100913.206d9672@cakuba.netronome.com>
References: <20190115005009.16025-1-jakub.kicinski@netronome.com>
        <20190115005009.16025-5-jakub.kicinski@netronome.com>
        <20190115101818.GE2290@nanopsycho>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: davem@davemloft.net, netdev@vger.kernel.org,
        oss-drivers@netronome.com
To: Jiri Pirko <jiri@resnulli.us>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-qt1-f196.google.com ([209.85.160.196]:41137 "EHLO
        mail-qt1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1731310AbfAOSJS (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 15 Jan 2019 13:09:18 -0500
Received: by mail-qt1-f196.google.com with SMTP id l12so4001761qtf.8
        for <netdev@vger.kernel.org>; Tue, 15 Jan 2019 10:09:18 -0800 (PST)
In-Reply-To: <20190115101818.GE2290@nanopsycho>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Tue, 15 Jan 2019 11:18:18 +0100, Jiri Pirko wrote:
> 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 <jakub.kicinski@netronome.com>
> >---
> > .../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.

Ack, I will add those as defines in the devlink header plus some docs.
Let's see how it goes..

> >+};
> >+
> >+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.

Tmp var makes it look more like the other function (from next patch)...
and it makes the line fit in 80 char ;)