Re: [PATCH V2 2/3] scsi: ufs-qcom: Add support for dumping MCQ registers

[Bart Van Assche <bvanassche@xxxxxxx>]

On 3/5/25 4:03 AM, Manish Pandey wrote:
> +static void ufs_qcom_dump_mcq_hci_regs(struct ufs_hba *hba)
> +{
> +	/* RES_MCQ_1 */
> +	ufshcd_dump_regs(hba, 0x0, 256 * 4, "MCQ HCI 1da0000-1da03f0 ");
> +	usleep_range(1000, 1100);

Please add a comment that explains why the usleep_range() calls are
present.

> @@ -1624,6 +1670,19 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)
>   
>   	reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_TMRLUT);
>   	ufshcd_dump_regs(hba, reg, 9 * 4, "UFS_DBG_RD_REG_TMRLUT ");
> +
> +	if (hba->mcq_enabled) {
> +		reg = ufs_qcom_get_debug_reg_offset(host, UFS_RD_REG_MCQ);
> +		ufshcd_dump_regs(hba, reg, 64 * 4, "HCI MCQ Debug Registers ");
> +	}
> +
> +	if (in_task()) {
> +		/* Dump MCQ Host Vendor Specific Registers */
> +		if (hba->mcq_enabled) {
> +			ufs_qcom_dump_mcq_hci_regs(hba);
> +			usleep_range(1000, 1100);
> +		}
> +	}
>   }

Please either combine the two "if (hba->mcq_enabled)" tests or combine
the "in_task()" and "hba->mcq_enabled" tests.

Please also add a comment that explains why the in_task() call is
present and a comment that explains why the usleep_range() call is
present.

Thanks,

Bart.