All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: Shannon Nelson <shannon.nelson@amd.com>, <netdev@vger.kernel.org>,
	<davem@davemloft.net>, <jiri@nvidia.com>
Subject: Re: [PATCH net-next 1/2] devlink: add fw bank select parameter
Date: Thu, 8 Dec 2022 17:24:22 -0800	[thread overview]
Message-ID: <20221208172422.37423144@kernel.org> (raw)
In-Reply-To: <d194be5e-886b-d69b-7d8d-3894354abe7f@intel.com>

On Thu, 8 Dec 2022 16:47:31 -0800 Jacob Keller wrote:
> This is what I was thinking of and looks good to me. As for how to add 
> attributes to get us from the current netlink API to this, I'm not 100% 
> sure.
> 
> I think we can mostly just add the bank ID and flags to indicate which 
> one is active and which one will be programmed next.

Why flags, tho?

The current nesting is:

  DEVLINK_ATTR_INFO_DRIVER_NAME		[str]
  DEVLINK_ATTR_INFO_SERIAL_NUMBER	[str]
  DEVLINK_ATTR_INFO_BOARD_SERIAL_NUMBER	[str]

  DEVLINK_ATTR_INFO_VERSION_FIXED	[nest] // multiple VERSION_* nests follow
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_FIXED	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_RUNNING	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_RUNNING	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]


Now we'd throw the bank into the nests, and add root attrs for the
current / flash / active as top level attrs:

  DEVLINK_ATTR_INFO_DRIVER_NAME		[str]
  DEVLINK_ATTR_INFO_SERIAL_NUMBER	[str]
  DEVLINK_ATTR_INFO_BOARD_SERIAL_NUMBER	[str]
  DEVLINK_ATTR_INFO_BANK_ACTIVE		[u32] // << optional
  DEVLINK_ATTR_INFO_BANK_UPDATE_TGT	[u32] // << optional

  DEVLINK_ATTR_INFO_VERSION_FIXED	[nest] // multiple VERSION_* nests follow
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_FIXED	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_RUNNING	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_RUNNING	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_STORED	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
    DEVLINK_ATTR_INFO_VERSION_BANK	[u32] // << optional
  DEVLINK_ATTR_INFO_VERSION_STORED	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE     [str]
    DEVLINK_ATTR_INFO_VERSION_BANK	[u32] // << optional

> I think we could also add a new attribute to both reload and flash which 
> specify which bank to use. For flash, this would be which bank to 
> program, and for update this would be which bank to load the firmware 
> from when doing a "fw_activate".

SG!

> Is that reasonable? Do you still need a permanent "use this bank by 
> default" parameter as well?

I hope we cover all cases, so no param needed?

  reply	other threads:[~2022-12-09  1:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-05 17:26 [PATCH net-next 0/2] devlink: add params FW_BANK and ENABLE_MIGRATION Shannon Nelson
2022-12-05 17:26 ` [PATCH net-next 1/2] devlink: add fw bank select parameter Shannon Nelson
2022-12-06  9:07   ` Jiri Pirko
2022-12-06 18:18     ` Shannon Nelson
2022-12-07 13:34       ` Jiri Pirko
2022-12-07  1:41   ` Jakub Kicinski
2022-12-07 19:29     ` Shannon Nelson
2022-12-08  0:36       ` Jakub Kicinski
2022-12-08 18:44         ` Shannon Nelson
2022-12-09  0:47           ` Jacob Keller
2022-12-09  1:24             ` Jakub Kicinski [this message]
2022-12-12 18:04               ` Jacob Keller
2022-12-12 18:34                 ` Jakub Kicinski
2022-12-09  1:15           ` Jakub Kicinski
2022-12-05 17:26 ` [PATCH net-next 2/2] devlink: add enable_migration parameter Shannon Nelson
2022-12-06  9:04   ` Jiri Pirko
2022-12-06 18:28     ` Shannon Nelson
2022-12-07 13:33       ` Jiri Pirko
2022-12-05 18:22 ` [PATCH net-next 0/2] devlink: add params FW_BANK and ENABLE_MIGRATION Leon Romanovsky
2022-12-05 18:55   ` Shannon Nelson
2022-12-06  8:13     ` Leon Romanovsky
2022-12-06  9:00 ` Jiri Pirko
2022-12-06 18:21   ` Shannon Nelson
2022-12-07 13:32     ` Jiri Pirko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221208172422.37423144@kernel.org \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=jacob.e.keller@intel.com \
    --cc=jiri@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=shannon.nelson@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.