From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Subject: Re: [PATCH net-next 0/8] bnxt_en: devlink param updates
Date: Tue, 11 Sep 2018 13:33:51 +0200
Message-ID: <20180911133215.5798ed2a@cakuba>
References: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: davem@davemloft.net, michael.chan@broadcom.com,
        netdev@vger.kernel.org, Alexander Duyck <alexander.duyck@gmail.com>
To: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pg1-f196.google.com ([209.85.215.196]:35155 "EHLO
        mail-pg1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726601AbeIKQcz (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 11 Sep 2018 12:32:55 -0400
Received: by mail-pg1-f196.google.com with SMTP id 7-v6so12134251pgf.2
        for <netdev@vger.kernel.org>; Tue, 11 Sep 2018 04:33:59 -0700 (PDT)
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Tue, 11 Sep 2018 14:14:57 +0530, Vasundhara Volam wrote:
> This patchset adds support for 4 generic and 1 driver-specific devlink
> parameters.
> 
> Also, this patchset adds support to return proper error code if
> HWRM_NVM_GET/SET_VARIABLE commands return error code
> HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED.
> 
> Vasundhara Volam (8):
>   devlink: Add generic parameter hw_tc_offload

Much like Jiri, I can't help but wonder why do you need this?

>   devlink: Add generic parameter ignore_ari
>   devlink: Add generic parameter msix_vec_per_pf_max
>   devlink: Add generic parameter msix_vec_per_pf_min

IMHO more structured API would be preferable if possible.  The string
keys won't scale if you want to set the parameters per PF, and
creating more structured API for PCIe which is a relatively slow
moving HW spec seems tractable.

Not to mention the question Alex posed before about where this knobs
should actually live.  I'm personally fine with devlink.

>   bnxt_en: Use hw_tc_offload and ignore_ari devlink parameters
>   bnxt_en: return proper error when FW returns
>     HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED
>   bnxt_en: Use msix_vec_per_pf_max and msix_vec_per_pf_min devlink
>     params.
>   bnxt_en: Add a driver specific devlink parameter.

The details about the device specific devlink parameter are very much
lacking.  Why does the patch subject not mention any specifics?  What's
GRE in the first place?

You really need to provide more details and docs for all these
parameters.