On Tue, 15 Jan 2019 11:18:18 +0100, Jiri Pirko wrote:
> Tue, Jan 15, 2019 at 01:50:06AM CET, jakub.kicin...@netronome.com wrote:
> >For now we only use HWinfo for fixed versions.
> >
> >Signed-off-by: Jakub Kicinski <jakub.kicin...@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 ;)
