Re: [PATCH net v2 2/3] octeon_ep: ensure dbell BADDR updation

[Paolo Abeni]

On 12/19/25 11:07 AM, Vimlesh Kumar wrote:
> @@ -327,11 +328,13 @@ static void octep_setup_iq_regs_cnxk_pf(struct octep_device *oct, int iq_no)
>  }
>  
>  /* Setup registers for a hardware Rx Queue  */
> -static void octep_setup_oq_regs_cnxk_pf(struct octep_device *oct, int oq_no)
> +static int octep_setup_oq_regs_cnxk_pf(struct octep_device *oct, int oq_no)
>  {
>  	u64 reg_val;
>  	u64 oq_ctl = 0ULL;
> +	u64 reg_ba_val;
>  	u32 time_threshold = 0;
> +	unsigned long t_out_jiffies;
>  	struct octep_oq *oq = oct->oq[oq_no];

Ouch, the above variable declaration follows the exact opposite of the
mandated style, please use the reverse christmas tree order.

Similar nit on the next patch.

>  
>  	oq_no += CFG_GET_PORTS_PF_SRN(oct->conf);
> @@ -343,6 +346,33 @@ static void octep_setup_oq_regs_cnxk_pf(struct octep_device *oct, int oq_no)
>  			reg_val = octep_read_csr64(oct, CNXK_SDP_R_OUT_CONTROL(oq_no));
>  		} while (!(reg_val & CNXK_R_OUT_CTL_IDLE));
>  	}
> +	octep_write_csr64(oct, CNXK_SDP_R_OUT_WMARK(oq_no),  oq->max_count);
> +	/* Wait for WMARK to get applied */
> +	usleep_range(10, 15);
> +
> +	octep_write_csr64(oct, CNXK_SDP_R_OUT_SLIST_BADDR(oq_no),
> +			  oq->desc_ring_dma);
> +	octep_write_csr64(oct, CNXK_SDP_R_OUT_SLIST_RSIZE(oq_no),
> +			  oq->max_count);
> +	reg_ba_val = octep_read_csr64(oct, CNXK_SDP_R_OUT_SLIST_BADDR(oq_no));
> +
> +	if (reg_ba_val != oq->desc_ring_dma) {
> +		t_out_jiffies = jiffies + 10 * HZ;
> +		do {
> +			if (reg_ba_val == ULLONG_MAX)
> +				return -EFAULT;
> +			octep_write_csr64(oct,
> +					  CNXK_SDP_R_OUT_SLIST_BADDR(oq_no),
> +					  oq->desc_ring_dma);
> +			octep_write_csr64(oct,
> +					  CNXK_SDP_R_OUT_SLIST_RSIZE(oq_no),
> +					  oq->max_count);
> +			reg_ba_val =
> +			octep_read_csr64(oct,
> +					 CNXK_SDP_R_OUT_SLIST_BADDR(oq_no));
> +		} while ((reg_ba_val != oq->desc_ring_dma) &&
> +			  time_before(jiffies, t_out_jiffies));

If a timeout happens - !time_before(jiffies, t_out_jiffies) - execution
proceed even if reg_ba_val != oq->desc_ring_dma; I guess you should
error out istead?!? Otherwise you should at least add a comment for
future memory explaining why that is correct.

Similar question on the next patch.

/P