Re: [PATCH v3 2/7] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs

[Bart Van Assche]

On 10/20/26 1:21 AM, Stanley Jhu wrote:
> +static unsigned long __ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
> +						     unsigned long mask,
> +						     bool from_irq)
> +	__must_hold(&hba->outstanding_lock)
> +{
> +	unsigned long completed_reqs = mask & hba->outstanding_reqs;
> +
> +	if (from_irq) {
> +		/* Do not complete polled requests from interrupt context. */
> +		ufshcd_clear_polled(hba, &completed_reqs);
> +	}
> +	hba->outstanding_reqs &= ~completed_reqs;
> +
> +	return completed_reqs;
> +}
> +
> +static unsigned long ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
> +						   unsigned long mask)
> +{
> +	guard(spinlock_irqsave)(&hba->outstanding_lock);
> +	return __ufshcd_clear_outstanding_reqs(hba, mask, false);
> +}

Please add a short comment above the above two functions that explains 
the meaning of the return value.

>   /*
>    * Return: > 0 if one or more commands have been completed or 0 if no
>    * requests have been completed.
> @@ -6071,12 +6094,9 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
>   
>   	spin_lock_irqsave(&hba->outstanding_lock, flags);
>   	tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> -	completed_reqs = ~tr_doorbell & hba->outstanding_reqs;
> -	if (queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT) {
> -		/* Do not complete polled requests from interrupt context. */
> -		ufshcd_clear_polled(hba, &completed_reqs);
> -	}
> -	hba->outstanding_reqs &= ~completed_reqs;
> +	completed_reqs =
> +		__ufshcd_clear_outstanding_reqs(hba, ~tr_doorbell,
> +						queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);

The above line is about 98 columns wide. That's too much - all other
code in this driver is 80 columns wide. One way to address this is by
introducing a local "from_irq" boolean variable:

+	from_irq = queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT;
+	completed_reqs = __ufshcd_clear_outstanding_reqs(hba, ~tr_doorbell,
+							 from_irq);

Otherwise this patch looks good to me.

Bart.