Re: [Intel-wired-lan] [PATCH net-next 3/5] i40e: Add handler for devlink .info_get

From: Jacob Keller <jacob.e.keller@intel.com>
To: Jakub Kicinski <kuba@kernel.org>, Ivan Vecera <ivecera@redhat.com>
Cc: netdev@vger.kernel.org,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	linux-kernel@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	intel-wired-lan@lists.osuosl.org, Paolo Abeni <pabeni@redhat.com>,
	"David S . Miller" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [PATCH net-next 3/5] i40e: Add handler for devlink .info_get
Date: Tue, 17 Oct 2023 10:05:13 -0700	[thread overview]
Message-ID: <75d7c5fb-2cbd-479c-bc9b-3730223e77db@intel.com> (raw)
In-Reply-To: <20231017082120.1d1246f6@kernel.org>

On 10/17/2023 8:21 AM, Jakub Kicinski wrote:
> On Tue, 17 Oct 2023 11:56:20 +0200 Ivan Vecera wrote:
>>> Your board reports "fw.psid 9.30", this may not be right,
>>> PSID is more of a board+customer ID, IIUC. 9.30 looks like
>>> a version, not an ID.  
>>
>> Maybe plain 'fw' should be used for this '9.30' as this is a version
>> of the whole software package provided by Intel for these adapters
>> (e.g. 
>> https://www.intel.com/content/www/us/en/download/18190/non-volatile-memory-nvm-update-utility-for-intel-ethernet-network-adapter-700-series.html).
>>
>> Thoughts?
> 
> Hm, that could be better, yes.
> 
> Jake, any guidance?

Hm. The ice driver has 'fw.psid.api' which is documented as:

    * - ``fw.psid.api``
      - running
      - 0.80
      - Version defining the format of the flash contents.

I think we settled on this as well back when I was working on the ice
version.

See
https://lore.kernel.org/netdev/70001e87-b369-bab4-f318-ad4514e7dcfb@intel.com/

However, looking at the code more closely, this does appear to match the
ice driver's implementation if you use "fw.psid.api". I believe the
intent for this is a version that indicates the format or layout of the
NVM contents.

Given that ice uses fw.psid.api for what appears to be the same purpose
I would propose that here as well.

> 
>>> UNDI means PXE. Is that whave "combo image" means for Intel?  
>>
>> Combo image version (aka CIVD) is reported by nvmupdate tool and this
>> should be version of OROM that contains PXE, EFI images that each of
>> them can have specific version but this CIVD should be overall OROM 
>> version for this combination of PXE and EFI. I hope I'm right.
> 
> Sounds good then!

Yes that sounds correct. That's what we do in ice as well.

I'm going to review the whole patch now since I hadn't noticed this
previously.

Thanks,
Jake
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan