[PATCH 0/3] ufs: pci: Add support UFSHCI 4.0 MCQ

[PATCH 0/3] ufs: pci: Add support UFSHCI 4.0 MCQ

[Minwoo Im]

This patchset introduces add support for MCQ introduced in UFSHCI 4.0.  The
first patch adds a simple helper to get the address of MCQ queue config
registers.  The second one enables MCQ feature by adding mandatory vops
callback functions required at MCQ initialization phase.  The last one is to
prevent a case where number of MCQ is given 1 since driver allocates poll_queues
first rather than I/O queues to handle device commands.  Instead of causing
exception handlers due to no I/O queue, failfast during the initialization time.

Minwoo Im (3):
  ufs: mcq: Add ufshcd_mcq_queue_cfg_addr helper
  ufs: pci: Add support MCQ for QEMU-based UFS
  ufs: mcq: Prevent no I/O queue case for MCQ

 drivers/ufs/core/ufs-mcq.c    | 23 +++++++++++++++++
 drivers/ufs/host/ufshcd-pci.c | 48 ++++++++++++++++++++++++++++++++++-
 include/ufs/ufshcd.h          |  1 +
 3 files changed, 71 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/3] ufs: mcq: Add ufshcd_mcq_queue_cfg_addr helper

[Minwoo Im]

This helper returns an offset address of MCQ queue configuration
registers.  This is a prep patch for the following patch.

Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
---
 drivers/ufs/core/ufs-mcq.c | 14 ++++++++++++++
 include/ufs/ufshcd.h       |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 52210c4c20dc..46faa54aea94 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -18,6 +18,7 @@
 #include <linux/iopoll.h>
 
 #define MAX_QUEUE_SUP GENMASK(7, 0)
+#define QCFGPTR GENMASK(23, 16)
 #define UFS_MCQ_MIN_RW_QUEUES 2
 #define UFS_MCQ_MIN_READ_QUEUES 0
 #define UFS_MCQ_MIN_POLL_QUEUES 0
@@ -116,6 +117,19 @@ struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
 	return &hba->uhq[hwq];
 }
 
+/**
+ * ufshcd_mcq_queue_cfg_addr - get an start address of the MCQ Queue Config
+ * Registers.
+ * @hba: per adapter instance
+ *
+ * Return: Start address of MCQ Queue Config Registers in HCI
+ */
+unsigned int ufshcd_mcq_queue_cfg_addr(struct ufs_hba *hba)
+{
+	return FIELD_GET(QCFGPTR, hba->mcq_capabilities) * 0x200;
+}
+EXPORT_SYMBOL_GPL(ufshcd_mcq_queue_cfg_addr);
+
 /**
  * ufshcd_mcq_decide_queue_depth - decide the queue depth
  * @hba: per adapter instance
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index df68fb1d4f3f..9e0581115b34 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1278,6 +1278,7 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val);
 void ufshcd_hba_stop(struct ufs_hba *hba);
 void ufshcd_schedule_eh_work(struct ufs_hba *hba);
 void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
+unsigned int ufshcd_mcq_queue_cfg_addr(struct ufs_hba *hba);
 u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i);
 void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
 unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
-- 
2.34.1

Re: [PATCH 1/3] ufs: mcq: Add ufshcd_mcq_queue_cfg_addr helper

[Bart Van Assche]

On 5/31/24 03:38, Minwoo Im wrote:
> This helper returns an offset address of MCQ queue configuration
> registers.  This is a prep patch for the following patch.
> 
> Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
> ---
>   drivers/ufs/core/ufs-mcq.c | 14 ++++++++++++++
>   include/ufs/ufshcd.h       |  1 +
>   2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 52210c4c20dc..46faa54aea94 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -18,6 +18,7 @@
>   #include <linux/iopoll.h>
>   
>   #define MAX_QUEUE_SUP GENMASK(7, 0)
> +#define QCFGPTR GENMASK(23, 16)
>   #define UFS_MCQ_MIN_RW_QUEUES 2
>   #define UFS_MCQ_MIN_READ_QUEUES 0
>   #define UFS_MCQ_MIN_POLL_QUEUES 0
> @@ -116,6 +117,19 @@ struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
>   	return &hba->uhq[hwq];
>   }
>   
> +/**
> + * ufshcd_mcq_queue_cfg_addr - get an start address of the MCQ Queue Config
> + * Registers.
> + * @hba: per adapter instance
> + *
> + * Return: Start address of MCQ Queue Config Registers in HCI
> + */
> +unsigned int ufshcd_mcq_queue_cfg_addr(struct ufs_hba *hba)
> +{
> +	return FIELD_GET(QCFGPTR, hba->mcq_capabilities) * 0x200;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_mcq_queue_cfg_addr);
> +
>   /**
>    * ufshcd_mcq_decide_queue_depth - decide the queue depth
>    * @hba: per adapter instance
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index df68fb1d4f3f..9e0581115b34 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1278,6 +1278,7 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val);
>   void ufshcd_hba_stop(struct ufs_hba *hba);
>   void ufshcd_schedule_eh_work(struct ufs_hba *hba);
>   void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
> +unsigned int ufshcd_mcq_queue_cfg_addr(struct ufs_hba *hba);
>   u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i);
>   void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
>   unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,

New functions should not be introduced as a separate patch but instead should
be introduced in the first patch that adds a caller to the new function.

Thanks,

Bart.

Re: [PATCH 1/3] ufs: mcq: Add ufshcd_mcq_queue_cfg_addr helper

[Minwoo Im]

[-- Attachment #1: Type: text/plain, Size: 2486 bytes --]

On 24-05-31 13:15:11, Bart Van Assche wrote:
> On 5/31/24 03:38, Minwoo Im wrote:
> > This helper returns an offset address of MCQ queue configuration
> > registers.  This is a prep patch for the following patch.
> > 
> > Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
> > ---
> >   drivers/ufs/core/ufs-mcq.c | 14 ++++++++++++++
> >   include/ufs/ufshcd.h       |  1 +
> >   2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> > index 52210c4c20dc..46faa54aea94 100644
> > --- a/drivers/ufs/core/ufs-mcq.c
> > +++ b/drivers/ufs/core/ufs-mcq.c
> > @@ -18,6 +18,7 @@
> >   #include <linux/iopoll.h>
> >   #define MAX_QUEUE_SUP GENMASK(7, 0)
> > +#define QCFGPTR GENMASK(23, 16)
> >   #define UFS_MCQ_MIN_RW_QUEUES 2
> >   #define UFS_MCQ_MIN_READ_QUEUES 0
> >   #define UFS_MCQ_MIN_POLL_QUEUES 0
> > @@ -116,6 +117,19 @@ struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
> >   	return &hba->uhq[hwq];
> >   }
> > +/**
> > + * ufshcd_mcq_queue_cfg_addr - get an start address of the MCQ Queue Config
> > + * Registers.
> > + * @hba: per adapter instance
> > + *
> > + * Return: Start address of MCQ Queue Config Registers in HCI
> > + */
> > +unsigned int ufshcd_mcq_queue_cfg_addr(struct ufs_hba *hba)
> > +{
> > +	return FIELD_GET(QCFGPTR, hba->mcq_capabilities) * 0x200;
> > +}
> > +EXPORT_SYMBOL_GPL(ufshcd_mcq_queue_cfg_addr);
> > +
> >   /**
> >    * ufshcd_mcq_decide_queue_depth - decide the queue depth
> >    * @hba: per adapter instance
> > diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> > index df68fb1d4f3f..9e0581115b34 100644
> > --- a/include/ufs/ufshcd.h
> > +++ b/include/ufs/ufshcd.h
> > @@ -1278,6 +1278,7 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val);
> >   void ufshcd_hba_stop(struct ufs_hba *hba);
> >   void ufshcd_schedule_eh_work(struct ufs_hba *hba);
> >   void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
> > +unsigned int ufshcd_mcq_queue_cfg_addr(struct ufs_hba *hba);
> >   u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i);
> >   void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
> >   unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
> 
> New functions should not be introduced as a separate patch but instead should
> be introduced in the first patch that adds a caller to the new function.

I will squash it to the second one and udpate it in the next version.

> 
> Thanks,
> 
> Bart.
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 2/3] ufs: pci: Add support MCQ for QEMU-based UFS

[Minwoo Im]

Recently, ufs-mcq feature has been introduced to QEMU hw/ufs device [1].
This patch adds MCQ support for upstream QEMU UFS PCI controller.  This
patch provides mandatory vops callbacks to make UFS controller work
properly on MCQ mode.  Operation and Runtime Config register stride is
fixed to 48bytes which is implemented by qemu.

[1] https://lore.kernel.org/qemu-devel/cover.1716876237.git.jeuk20.kim@samsung.com/

Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
---
 drivers/ufs/host/ufshcd-pci.c | 48 ++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index 0aca666d2199..d4d64a29390e 100644
--- a/drivers/ufs/host/ufshcd-pci.c
+++ b/drivers/ufs/host/ufshcd-pci.c
@@ -20,6 +20,8 @@
 #include <linux/acpi.h>
 #include <linux/gpio/consumer.h>
 
+#define MAX_SUPP_MAC 64
+
 struct ufs_host {
 	void (*late_init)(struct ufs_hba *hba);
 };
@@ -446,6 +448,49 @@ static int ufs_intel_mtl_init(struct ufs_hba *hba)
 	return ufs_intel_common_init(hba);
 }
 
+static int ufs_redhat_get_hba_mac(struct ufs_hba *hba)
+{
+	return MAX_SUPP_MAC;
+}
+
+static int ufs_redhat_mcq_config_resource(struct ufs_hba *hba)
+{
+	hba->mcq_base = hba->mmio_base + ufshcd_mcq_queue_cfg_addr(hba);
+
+	return 0;
+}
+
+static int ufs_redhat_op_runtime_config(struct ufs_hba *hba)
+{
+	struct ufshcd_mcq_opr_info_t *opr;
+	int i;
+
+	u32 sqdao = ufsmcq_readl(hba, ufshcd_mcq_cfg_offset(REG_SQDAO, 0));
+	u32 sqisao = ufsmcq_readl(hba, ufshcd_mcq_cfg_offset(REG_SQISAO, 0));
+	u32 cqdao = ufsmcq_readl(hba, ufshcd_mcq_cfg_offset(REG_CQDAO, 0));
+	u32 cqisao = ufsmcq_readl(hba, ufshcd_mcq_cfg_offset(REG_CQISAO, 0));
+
+	hba->mcq_opr[OPR_SQD].offset = sqdao;
+	hba->mcq_opr[OPR_SQIS].offset = sqisao;
+	hba->mcq_opr[OPR_CQD].offset = cqdao;
+	hba->mcq_opr[OPR_CQIS].offset = cqisao;
+
+	for (i = 0; i < OPR_MAX; i++) {
+		opr = &hba->mcq_opr[i];
+		opr->stride = 48;
+		opr->base = hba->mmio_base + opr->offset;
+	}
+
+	return 0;
+}
+
+static struct ufs_hba_variant_ops ufs_redhat_hba_vops = {
+	.name                   = "redhat-pci",
+	.get_hba_mac		= ufs_redhat_get_hba_mac,
+	.mcq_config_resource	= ufs_redhat_mcq_config_resource,
+	.op_runtime_config	= ufs_redhat_op_runtime_config,
+};
+
 static struct ufs_hba_variant_ops ufs_intel_cnl_hba_vops = {
 	.name                   = "intel-pci",
 	.init			= ufs_intel_common_init,
@@ -591,7 +636,8 @@ static const struct dev_pm_ops ufshcd_pci_pm_ops = {
 };
 
 static const struct pci_device_id ufshcd_pci_tbl[] = {
-	{ PCI_VENDOR_ID_REDHAT, 0x0013, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+	{ PCI_VENDOR_ID_REDHAT, 0x0013, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+		(kernel_ulong_t)&ufs_redhat_hba_vops },
 	{ PCI_VENDOR_ID_SAMSUNG, 0xC00C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
 	{ PCI_VDEVICE(INTEL, 0x9DFA), (kernel_ulong_t)&ufs_intel_cnl_hba_vops },
 	{ PCI_VDEVICE(INTEL, 0x4B41), (kernel_ulong_t)&ufs_intel_ehl_hba_vops },
-- 
2.34.1

Re: [PATCH 2/3] ufs: pci: Add support MCQ for QEMU-based UFS

[Bart Van Assche]

On 5/31/24 03:38, Minwoo Im wrote:
> +static int ufs_redhat_get_hba_mac(struct ufs_hba *hba)
> +{
> +	return MAX_SUPP_MAC;
> +}

Why the prefix "ufs_redhat" instead of "ufs_qemu"?

Thanks,

Bart.

Re: [PATCH 2/3] ufs: pci: Add support MCQ for QEMU-based UFS

[Minwoo Im]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

On 24-05-31 13:16:40, Bart Van Assche wrote:
> On 5/31/24 03:38, Minwoo Im wrote:
> > +static int ufs_redhat_get_hba_mac(struct ufs_hba *hba)
> > +{
> > +	return MAX_SUPP_MAC;
> > +}
> 
> Why the prefix "ufs_redhat" instead of "ufs_qemu"?

I thouogh I had to use the same name pattern as ufs_intel_* for the name of
vendor ID of the PCI device.  I will update it in the next version.

Thanks for the review.

> 
> Thanks,
> 
> Bart.
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 3/3] ufs: mcq: Prevent no I/O queue case for MCQ

[Minwoo Im]

If hba_maxq equals poll_queues, which means there are no I/O queues
(HCTX_TYPE_DEFAULT, HCTX_TYPE_READ), the very first hw queue will be
allocated as HCTX_TYPE_POLL and it will be used as the dev_cmd_queue.
In this case, device commands such as QUERY cannot be properly handled.

This patch prevents the initialization of MCQ when the number of I/O
queues is not set and only the number of POLL queues is set.

Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
---
 drivers/ufs/core/ufs-mcq.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 46faa54aea94..4bcae410c268 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -179,6 +179,15 @@ static int ufshcd_mcq_config_nr_queues(struct ufs_hba *hba)
 		return -EOPNOTSUPP;
 	}
 
+	/*
+	 * Device should support at least one I/O queue to handle device
+	 * commands via hba->dev_cmd_queue.
+	 */
+	if (hba_maxq == poll_queues) {
+		dev_err(hba->dev, "At least one non-poll queue required\n");
+		return -EOPNOTSUPP;
+	}
+
 	rem = hba_maxq;
 
 	if (rw_queues) {
-- 
2.34.1

Re: [PATCH 3/3] ufs: mcq: Prevent no I/O queue case for MCQ

[Bart Van Assche]

On 5/31/24 03:38, Minwoo Im wrote:
> If hba_maxq equals poll_queues, which means there are no I/O queues
> (HCTX_TYPE_DEFAULT, HCTX_TYPE_READ), the very first hw queue will be
> allocated as HCTX_TYPE_POLL and it will be used as the dev_cmd_queue.
> In this case, device commands such as QUERY cannot be properly handled.
> 
> This patch prevents the initialization of MCQ when the number of I/O
> queues is not set and only the number of POLL queues is set.
> 
> Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
> ---
>   drivers/ufs/core/ufs-mcq.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 46faa54aea94..4bcae410c268 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -179,6 +179,15 @@ static int ufshcd_mcq_config_nr_queues(struct ufs_hba *hba)
>   		return -EOPNOTSUPP;
>   	}
>   
> +	/*
> +	 * Device should support at least one I/O queue to handle device
> +	 * commands via hba->dev_cmd_queue.
> +	 */
> +	if (hba_maxq == poll_queues) {
> +		dev_err(hba->dev, "At least one non-poll queue required\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>   	rem = hba_maxq;
>   
>   	if (rw_queues) {

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