[v4,00/16] Add Multi Circular Queue Support

UFS Multi-Circular Queue (MCQ) has been added in UFSHCI v4.0 to improve storage performance.
The implementation uses the shared tagging mechanism so that tags are shared
among the hardware queues. The number of hardware queues is configurable.
This series doesn't include the ESI implementation for completion handling.

This implementation has been verified by booting on a Qualcomm platform.
During testing, all low power modes were disabled and it was in HS-G1 mode.

Please take a look and let us know your thoughts.

v3 -> v4:
- Added a kernel module parameter to disable MCQ mode
- Added Bart's reviewed-by tag for some patches
- Addressed Bart's comments

v2 -> v3:
- Split ufshcd_config_mcq() into ufshcd_alloc_mcq() and ufshcd_config_mcq()
- Use devm_kzalloc() in ufshcd_mcq_init()
- Free memory and resource allocation on error paths
- Corrected typos in code comments

v1 -> v2:
- Added a non MCQ related change to use a function to extrace ufs extended
feature
- Addressed Mani's comments
- Addressed Bart's comments

v1:
- Split the changes
- Addressed Bart's comments
- Addressed Bean's comments

* RFC versions:
v2 -> v3:
- Split the changes based on functionality
- Addressed queue configuration issues
- Faster SQE tail pointer increments
- Addressed comments from Bart and Manivannan

v1 -> v2:
- Enabled host_tagset
- Added queue num configuration support
- Added one more vops to allow vendor provide the wanted MAC
- Determine nutrs and can_queue by considering both MAC, bqueuedepth and EXT_IID support
- Postponed MCQ initialization and scsi_add_host() to async probe
- Used (EXT_IID, Task Tag) tuple to support up to 4096 tasks (theoretically)


Asutosh Das (16):
  ufs: core: Optimize duplicate code to read extended feature
  ufs: core: Probe for ext_iid support
  ufs: core: Introduce Multi-circular queue capability
  ufs: core: Defer adding host to scsi if mcq is supported
  ufs: core: mcq: Add Multi Circular Queue support
  ufs: core: mcq: Configure resource regions
  ufs: core: mcq: Calculate queue depth
  ufs: core: mcq: Allocate memory for mcq mode
  ufs: core: mcq: Configure operation and runtime interface
  ufs: core: mcq: Use shared tags for MCQ mode
  ufs: core: Prepare ufshcd_send_command for mcq
  ufs: core: mcq: Find hardware queue to queue request
  ufs: core: Prepare for completion in mcq
  ufs: mcq: Add completion support of a cqe
  ufs: core: mcq: Add completion support in poll
  ufs: core: mcq: Enable Multi Circular Queue

 drivers/ufs/core/Makefile      |   2 +-
 drivers/ufs/core/ufs-mcq.c     | 494 +++++++++++++++++++++++++++++++++++++++++
 drivers/ufs/core/ufshcd-priv.h |  84 ++++++-
 drivers/ufs/core/ufshcd.c      | 381 +++++++++++++++++++++++++------
 drivers/ufs/host/ufs-qcom.c    |  48 ++++
 drivers/ufs/host/ufs-qcom.h    |   4 +
 include/ufs/ufs.h              |   6 +
 include/ufs/ufshcd.h           | 125 +++++++++++
 include/ufs/ufshci.h           |  63 ++++++
 9 files changed, 1140 insertions(+), 67 deletions(-)
 create mode 100644 drivers/ufs/core/ufs-mcq.c

On 11/9/22 11:41, Asutosh Das wrote:
> The code to parse the extended feature is duplicated twice
> in the ufs core. Replace the duplicated code with a
> function.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

On 11/9/22 11:41, Asutosh Das wrote:
> +/* UFSHC 4.0 compliant HC support this mode, refer param_set_mcq_mode() */
> +static bool mcq_mode = true;

Please rename both the variable and the kernel module parameter 
"mcq_mode" into "use_mcq_mode".

> +module_param_cb(mcq_mode, &mcq_mode_ops, &mcq_mode, 0644);
> +MODULE_PARM_DESC(mcq_mode, "Disable MCQ mode for UFSHCI 4.0 controllers");

The description of this parameter is confusing since it suggests that 
setting this parameter to 1 disables MCQ mode.

Thanks,

Bart.

On 11/9/22 11:41, Asutosh Das wrote:
> +	/* Manually allocate MCQ resource from ufs_mem */

Manually -> explicitly? The meaning of "manually" is "by hand and not by 
machine". Software is always executed by a CPU (machine) so nothing 
happens manually in software.

> +	ret = insert_resource(&iomem_resource, res_mcq);
> +	if (ret) {
> +		dev_err(hba->dev, "Failed to insert MCQ resource, err=%d\n",
> +			ret);
> +		devm_kfree(hba->dev, res_mcq);
> +		return ret;
> +	}
> +
> +	res->base = devm_ioremap_resource(hba->dev, res_mcq);
> +	if (IS_ERR(res->base)) {
> +		dev_err(hba->dev, "MCQ registers mapping failed, err=%d\n",
> +			(int)PTR_ERR(res->base));
> +		ret = PTR_ERR(res->base);
> +		res->base = NULL;
> +		remove_resource(res_mcq);
> +		devm_kfree(hba->dev, res_mcq);
> +		return ret;
> +	}

I see duplicated code in error paths. Please move the code for freeing 
resources past the "return 0" statement and use goto statements to jump 
to that code in case of an error.

Thanks,

Bart.