Re: [PATCH 1/2] scsi: ufs: Add Multi-Circular Queue support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/24/2022 2:54 PM, Bean Huo wrote:

Hi Can/Asutosh

A few questions about MCQ configuration:



Hello Bean,
Thanks for the review.

On Tue, 2022-07-19 at 00:01 -0700, Can Guo wrote:
From: Asutosh Das <quic_asutoshd@xxxxxxxxxxx>

Adds MCQ support to UFS driver.

Co-developed-by: Can Guo <quic_cang@xxxxxxxxxxx>
Signed-off-by: Asutosh Das <quic_asutoshd@xxxxxxxxxxx>
Signed-off-by: Can Guo <quic_cang@xxxxxxxxxxx>
---

+void ufshcd_mcq_config_mac(struct ufs_hba *hba)
+{
+       u32 val = ufshcd_readl(hba, REG_UFS_MCQ_CFG);
+
+       val &= ~MCQ_CFG_MAC_MASK;
+       val |= hba->dev_info.bqueuedepth << MCQ_CFG_MAC_OFFSET;
+       ufshcd_writel(hba, val, REG_UFS_MCQ_CFG);

Here you set MaxActiveCommand to dev_info.bqueuedepth (this limit comes
from UFS devices). I see in the qsize configuration that you want to
set the queue depth in each HW queue to be hba->nutrs (this limit comes
from UFSHCI),  should not it be min(device limit, ufshci limit)?

Yes, looks like it should be. Let me relook this logic.
+}
+EXPORT_SYMBOL_GPL(ufshcd_mcq_config_mac);
+

...
+
+       for_each_hw_queue(hba, i) {
+               hwq = &hba->uhq[i];
+               hwq->id = i;
+               qsize = hwq->max_entries * MCQ_ENTRY_SIZE_IN_DWORD -
1;

qsize is hba->nutrs, 32*8-1 = 255 =256DW,  per draft spec , should not
be 8DW in 4.0?

+
+               /* SQLBA */
+               ufsmcq_writel(hba, lower_32_bits(hwq->sqe_dma_addr),
+                             MCQ_CFG_n(REG_SQLBA, i));
+               /* SQUBA */
+               ufsmcq_writel(hba, upper_32_bits(hwq->sqe_dma_addr),
+                             MCQ_CFG_n(REG_SQUBA, i));
+               /* SQDAO */
+               ufsmcq_writel(hba, MCQ_ROP_OFFSET_n(ROP_SQD, i),
+                             MCQ_CFG_n(REG_SQDAO, i));


...

        }
+
+out:
+       hba->mcq_base = res->base;
+       return 0;
+
+out_err:
+       ufshcd_mcq_release_resource(hba);
+       return ret;
+}
+
+int ufshcd_mcq_init(struct ufs_hba *hba)
+{
+       struct Scsi_Host *host = hba->host;
+       struct ufs_hw_queue *hwq;
+       int i, ret = 0;
+
+       if (!is_mcq_supported(hba))
+               return 0;
+
+       ret = ufshcd_mcq_config_resource(hba);
+       if (ret) {
+               dev_err(hba->dev, "Failed to config MCQ resource\n");
+               return ret;
+       }
+
+       ret = ufshcd_vops_config_mcq_rop(hba);
+       if (ret) {
+               dev_err(hba->dev, "MCQ Runtime Operation Pointers not
configured\n");
+               goto out_err;
+       }
+
+       hba->nr_queues[HCTX_TYPE_DEFAULT] = num_possible_cpus();

4.0 supports maximum number of queues is 32. for cpus < 32, cpu to
queue will be 1x1, how about cpus > 32?

Good point. Will check and fix it.
+       hba->nr_queues[HCTX_TYPE_READ] = 0;
+       hba->nr_queues[HCTX_TYPE_POLL] = 1;
+
+       for (i = 0; i < HCTX_MAX_TYPES; i++)
+               host->nr_hw_queues += hba->nr_queues[i];
+
+       host->can_queue = hba->nutrs;

Also here, can_queue is inlined with ufshci limitation, not the UFS
device limit.

Yes, will take a look.
+       host->cmd_per_lun = hba->nutrs;
+
+       /* One more reserved for dev_cmd_queue */
+       hba->nr_hw_queues = host->nr_hw_queues + 1;
+
+       hba->uhq = devm_kmalloc(hba->dev,
...
        ufshcd_tune_unipro_params(hba);
@@ -9641,6 +9775,10 @@ int ufshcd_init(struct ufs_hba *hba, void
__iomem *mmio_base, unsigned int irq)
                 goto out_disable;
         }
+       err = ufshcd_mcq_init(hba);

The driver will force the customer to use MCQ, how about adding a
configuration option for the customer to choose (like eMMC CMDQ does)?

Let me check what eMMC does and see if that can be adopted here.

Kind regards,
Bean



-asd



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux