Message-ID: <cba05d39-7660-4b46-9e16-6e78a5bd68e9@intel.com>
Date: Mon, 4 Aug 2025 11:29:36 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jacob Keller <jacob.e.keller@...el.com>, Grzegorz Nitka
	<grzegorz.nitka@...el.com>, Anthony Nguyen <anthony.l.nguyen@...el.com>
CC: Intel Wired LAN <intel-wired-lan@...ts.osuosl.org>, Sergey Temerkhanov
	<sergey.temerkhanov@...el.com>, <netdev@...r.kernel.org>, Aleksandr Loktionov
	<aleksandr.loktionov@...el.com>
Subject: Re: [PATCH iwl-net] ice: use fixed adapter index for E825C embedded
 devices

On 8/2/25 00:27, Jacob Keller wrote:
> The ice_adapter structure is used by the ice driver to connect multiple
> physical functions of a device in software. It was introduced by
> commit 0e2bddf9e5f9 ("ice: add ice_adapter for shared data across PFs on
> the same NIC") and is primarily used for PTP support, as well as for
> handling certain cross-PF synchronization.
> 
> The original design of ice_adapter used PCI address information to
> determine which devices should be connected. This was extended to support
> E825C devices by commit fdb7f54700b1 ("ice: Initial support for E825C
> hardware in ice_adapter"), which used the device ID for E825C devices
> instead of the PCI address.
> 
> Later, commit 0093cb194a75 ("ice: use DSN instead of PCI BDF for
> ice_adapter index") replaced the use of Bus/Device/Function addressing with
> use of the device serial number.
> 
> E825C devices may appear in "Dual NAC" configuration which has multiple
> physical devices tied to the same clock source and which need to use the
> same ice_adapter. Unfortunately, each "NAC" has its own NVM which has its
> own unique Device Serial Number. Thus, use of the DSN for connecting
> ice_adapter does not work properly. It "worked" in the pre-production
> systems because the DSN was not initialized on the test NVMs and all the
> NACs had the same zero'd serial number.
> 
> Since we cannot rely on the DSN, lets fall back to the logic in the
> original E825C support which used the device ID. This is safe for E825C
> only because of the embedded nature of the device. It isn't a discreet
> adapter that can be plugged into an arbitrary system. All E825C devices on
> a given system are connected to the same clock source and need to be
> configured through the same PTP clock.
> 
> To make this separation clear, reserve bit 63 of the 64-bit index values as
> a "fixed index" indicator. Always clear this bit when using the device
> serial number as an index.
> 
> For E825C, use a fixed value defined as the 0x579C E825C backplane device
> ID bitwise ORed with the fixed index indicator. This is slightly different
> than the original logic of just using the device ID directly. Doing so
> prevents a potential issue with systems where only one of the NACs is
> connected with an external PHY over SGMII. In that case, one NAC would
> have the E825C_SGMII device ID, but the other would not.
> 
> Separate the determination of the full 64-bit index from the 32-bit
> reduction logic. Provide both ice_adapter_index() and a wrapping
> ice_adapter_xa_index() which handles reducing the index to a long on 32-bit
> systems. As before, cache the full index value in the adapter structure to
> warn about collisions.

thank you for the fix, I believe it is a very good solution with NVM
not open for change

It's nice that reserving one bit for special handling works also for
32bit case (despite being xored with lower part of DSN), :)

given BIT_ULL() fixed as you mentioned,
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>

> 
> This fixes issues with E825C not initializing PTP on both NACs, due to
> failure to connect the appropriate devices to the same ice_adapter.
> 
> Fixes: 0093cb194a75 ("ice: use DSN instead of PCI BDF for ice_adapter index")
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Reviewed-by: Grzegorz Nitka <grzegorz.nitka@...el.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> ---
> It turns out that using the device serial number does not work for E825C
> boards. I spoke with the team involved in the NVM image generation, and its
> not feasible at this point to change the process for generating the NVMs
> for E825C. We're stuck with the case that E825C Dual-NAC boards will have
> independent DSN for each NAC.
> 
> As far as I can tell, the only suitable fallback is to rely on the embedded
> nature of the E825C device. We know that all current systems with E825C
> need to have their ice_adapter connected. There are no plans to build
> platforms with multiple E825C devices. The E825C variant is not a discreet
> board, so customers can't simply plug an extra in. Thus, this change
> reverts back to using the device ID for E825C systems, instead of the
> serial number.

Powered by blists - more mailing lists