Re: [PATCH v2 for-next 09/17] RDMA/bnxt_re: add helper function __poll_for_resp

[Leon Romanovsky <leon@kernel.org>]

On Fri, Jun 09, 2023 at 04:01:46AM -0700, Selvin Xavier wrote:
> From: Kashyap Desai <kashyap.desai@broadcom.com>
> 
> This interface will be used if the driver has not enabled interrupt
> and/or interrupt is disabled for a short period of time.
> Completion is not possible from interrupt so this interface does
> self-polling.
> 
> Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 44 +++++++++++++++++++++++++++++-
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  1 +
>  2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> index 15f6793..3215f8a 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> @@ -260,6 +260,44 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
>  	return 0;
>  }
>  
> +/**
> + * __poll_for_resp   -	self poll completion for rcfw command
> + * @rcfw      -   rcfw channel instance of rdev
> + * @cookie    -   cookie to track the command
> + * @opcode    -   rcfw submitted for given opcode
> + *
> + * It works same as __wait_for_resp except this function will
> + * do self polling in sort interval since interrupt is disabled.
> + * This function can not be called from non-sleepable context.
> + *
> + * Returns:
> + * -ETIMEOUT if command is not completed in specific time interval.
> + * 0 if command is completed by firmware.
> + */
> +static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
> +			   u8 opcode)
> +{
> +	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
> +	unsigned long issue_time;
> +	u16 cbit;
> +
> +	cbit = cookie % rcfw->cmdq_depth;
> +	issue_time = jiffies;
> +
> +	do {
> +		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
> +			return bnxt_qplib_map_rc(opcode);
> +
> +		usleep_range(1000, 1001);
> +
> +		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
> +		if (!test_bit(cbit, cmdq->cmdq_bitmap))
> +			return 0;
> +		if (jiffies_to_msecs(jiffies - issue_time) > 10000)
> +			return -ETIMEDOUT;
> +	} while (true);
> +};
> +
>  static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
>  				       struct bnxt_qplib_cmdqmsg *msg)
>  {
> @@ -328,8 +366,10 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
>  
>  	if (msg->block)
>  		rc = __block_for_resp(rcfw, cookie, opcode);
> -	else
> +	else if (atomic_read(&rcfw->rcfw_intr_enabled))

Why atomic_t? It doesn't eliminate the need of locking and if locking
exists, you don't need atomic_t.

>  		rc = __wait_for_resp(rcfw, cookie, opcode);
> +	else
> +		rc = __poll_for_resp(rcfw, cookie, opcode);
>  	if (rc) {
>  		/* timed out */
>  		dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
> @@ -796,6 +836,7 @@ void bnxt_qplib_rcfw_stop_irq(struct bnxt_qplib_rcfw *rcfw, bool kill)
>  	kfree(creq->irq_name);
>  	creq->irq_name = NULL;
>  	creq->requested = false;
> +	atomic_set(&rcfw->rcfw_intr_enabled, 0);
>  }
>  
>  void bnxt_qplib_disable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw)
> @@ -857,6 +898,7 @@ int bnxt_qplib_rcfw_start_irq(struct bnxt_qplib_rcfw *rcfw, int msix_vector,
>  	creq->requested = true;
>  
>  	bnxt_qplib_ring_nq_db(&creq->creq_db.dbinfo, res->cctx, true);
> +	atomic_inc(&rcfw->rcfw_intr_enabled);
>  
>  	return 0;
>  }
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> index b7bbbae..089e616 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> @@ -221,6 +221,7 @@ struct bnxt_qplib_rcfw {
>  	u64 oos_prev;
>  	u32 init_oos_stats;
>  	u32 cmdq_depth;
> +	atomic_t rcfw_intr_enabled;
>  	struct semaphore rcfw_inflight;
>  };
>  
> -- 
> 2.5.5
>