Re: [Intel-wired-lan] [PATCH iwl-next] ice: add SBQ posted writes with non-posted support for CGU

[Jacob Keller <jacob.e.keller@intel.com>]

On 4/15/2026 4:27 AM, Przemyslaw Korba wrote:
> From: Karol Kolacinski <karol.kolacinski@intel.com>
> 
> Sideband queue (SBQ) is a HW queue with very short completion time. All
> SBQ writes were posted by default, which means that the driver did not
> have to wait for completion from the neighbor device, because there was
> none. This introduced unnecessary delays, where only those delays were
> "ensuring" that the command is "completed" and this was a potential race
> condition.
> 
> Add the possibility to perform non-posted writes where it's necessary to
> wait for completion, instead of relying on fake completion from the FW,
> where only the delays are guarding the writes.
> 
> Flush the SBQ by reading address 0 from the PHY 0 before issuing SYNC
> command to ensure that writes to all PHYs were completed and skip SBQ
> message completion if it's posted.
> 
> To analyze if delays are gone, look for and compare time spent in
> ice_sq_send_cmd — posted writes should return immediately after the wr32.
> That can be done for example by adjusting phc time with phc_ctl on E830
> device, for less than 2 seconds to use this new mechanism. Without it,
> command below will fail.
> 
> Reproduction steps:
> phc_ctl eth13 adj 1
> phc_ctl[4478170.994]: adjusted clock by 1.000000 seconds
> 
> Check trace for timing for comparisions:
> echo ice_sbq_send_cmd > /sys/kernel/debug/tracing/set_ftrace_filter
> echo function_graph > /sys/kernel/debug/tracing/current_tracer
> cat /sys/kernel/debug/tracing/trace
> 
> Tested on:
>   - Intel E830 NIC (FW version 1.00)
>   - Kernel 6.19.0+
> 
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> ---

This doesn't appear to apply clean to the tip of Intel Wired LAN
dev-queue, nor to net-next/main...

>  drivers/net/ethernet/intel/ice/ice_common.c  | 18 ++++--
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c  | 64 ++++++++++++--------
>  drivers/net/ethernet/intel/ice/ice_sbq_cmd.h |  5 +-
>  3 files changed, 53 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index f84990996530..2cd3d6d450a9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -1777,23 +1777,29 @@ int ice_sbq_rw_reg(struct ice_hw *hw, struct ice_sbq_msg_input *in, u16 flags)
>  	msg.msg_addr_low = cpu_to_le16(in->msg_addr_low);
>  	msg.msg_addr_high = cpu_to_le32(in->msg_addr_high);
>  
> -	if (in->opcode)
> +	switch (in->opcode) {
> +	case ice_sbq_msg_wr_p:
> +	case ice_sbq_msg_wr_np:
>  		msg.data = cpu_to_le32(in->data);
> -	else
> +		break;
> +	case ice_sbq_msg_rd:
>  		/* data read comes back in completion, so shorten the struct by
>  		 * sizeof(msg.data)
>  		 */
>  		msg_len -= sizeof(msg.data);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
>  
> -	if (in->opcode == ice_sbq_msg_wr)
> -		cd.posted = 1;

It looks like this code in the upstream version doesn't have the cd
structure on this function.

> +	cd.posted = in->opcode == ice_sbq_msg_wr_p;
> 

It looks like this is based on top of "ice: fix posted write support for
sideband queue operations"? That was dropped from the queue because of
our discussion that you would re-submit a fixed version.

Since that didn't get applied, this won't apply clean either. Do you
still want the part that fixes E830 to go to net? Or do you just want to
implement posted writes all together in one patch?

Either way, could you please re-submit the work either as 2 patches or
as a single combined patch?

Thanks,
Jake