RE:(2) [PATCH v2 06/17] ufs: core: mcq: Configure resource regions

From: Daejun Park <daejun7.park@samsung.com>
To: Daejun Park <daejun7.park@samsung.com>,
	Asutosh Das <quic_asutoshd@quicinc.com>,
	"quic_cang@quicinc.com" <quic_cang@quicinc.com>,
	"quic_nitirawa@quicinc.com" <quic_nitirawa@quicinc.com>,
	"quic_rampraka@quicinc.com" <quic_rampraka@quicinc.com>,
	"quic_bhaskarv@quicinc.com" <quic_bhaskarv@quicinc.com>,
	"quic_richardp@quicinc.com" <quic_richardp@quicinc.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Cc: "linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	"quic_nguyenb@quicinc.com" <quic_nguyenb@quicinc.com>,
	"quic_xiaosenh@quicinc.com" <quic_xiaosenh@quicinc.com>,
	"bvanassche@acm.org" <bvanassche@acm.org>,
	"avri.altman@wdc.com" <avri.altman@wdc.com>,
	"mani@kernel.org" <mani@kernel.org>,
	"beanhuo@micron.com" <beanhuo@micron.com>
Subject: RE:(2) [PATCH v2 06/17] ufs: core: mcq: Configure resource regions
Date: Fri, 07 Oct 2022 12:44:19 +0900	[thread overview]
Message-ID: <20221007034419epcms2p84f51fef929459cef230c34c1dbfbff1d@epcms2p8> (raw)
In-Reply-To: <20221007024138epcms2p729595abf03da8402618c4803b20a4d13@epcms2p7>

Hi Asutosh Das,

>Define the mcq resources and add support to ioremap
>the resource regions.
>
>Co-developed-by: Can Guo <quic_cang@quicinc.com>
>Signed-off-by: Can Guo <quic_cang@quicinc.com>
>Signed-off-by: Asutosh Das <quic_asutoshd@quicinc.com>
>---
> drivers/ufs/core/ufs-mcq.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++
> include/ufs/ufshcd.h       | 28 +++++++++++++
> 2 files changed, 127 insertions(+)
>
>diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
>index 659398d..7d0a37a 100644
>--- a/drivers/ufs/core/ufs-mcq.c
>+++ b/drivers/ufs/core/ufs-mcq.c
>@@ -18,6 +18,11 @@
> #define UFS_MCQ_NUM_DEV_CMD_QUEUES 1
> #define UFS_MCQ_MIN_POLL_QUEUES 0
> 
>+#define MCQ_QCFGPTR_MASK        GENMASK(7, 0)
>+#define MCQ_QCFGPTR_UNIT        0x200
>+#define MCQ_SQATTR_OFFSET(c) \
>+        ((((c) >> 16) & MCQ_QCFGPTR_MASK) * MCQ_QCFGPTR_UNIT)
>+#define MCQ_QCFG_SIZE        0x40
> 
> static int rw_queue_count_set(const char *val, const struct kernel_param *kp)
> {
>@@ -67,6 +72,97 @@ module_param_cb(poll_queues, &poll_queue_count_ops, &poll_queues, 0644);
> MODULE_PARM_DESC(poll_queues,
>                  "Number of poll queues used for r/w. Default value is 1");
> 
>+/* Resources */
>+static const struct ufshcd_res_info ufs_res_info[RES_MAX] = {
>+        {.name = "ufs_mem",},
>+        {.name = "mcq",},
>+        /* Submission Queue DAO */
>+        {.name = "mcq_sqd",},
>+        /* Submission Queue Interrupt Status */
>+        {.name = "mcq_sqis",},
>+        /* Completion Queue DAO */
>+        {.name = "mcq_cqd",},
>+        /* Completion Queue Interrupt Status */
>+        {.name = "mcq_cqis",},
>+        /* MCQ vendor specific */
>+        {.name = "mcq_vs",},
>+};
>+
>+static int ufshcd_mcq_config_resource(struct ufs_hba *hba)
>+{
>+        struct platform_device *pdev = to_platform_device(hba->dev);
>+        struct ufshcd_res_info *res;
>+        struct resource *res_mem, *res_mcq;
>+        int i, ret = 0;
>+
>+        memcpy(hba->res, ufs_res_info, sizeof(ufs_res_info));
>+
>+        for (i = 0; i < RES_MAX; i++) {
>+                res = &hba->res[i];
>+                res->resource = platform_get_resource_byname(pdev,
>+                                                             IORESOURCE_MEM,
>+                                                             res->name);
>+                if (!res->resource) {
>+                        dev_info(hba->dev, "Resource %s not provided\n", res->name);
>+                        if (i == RES_UFS)
>+                                return -ENOMEM;
>+                        continue;
>+                } else if (i == RES_UFS) {
>+                        res_mem = res->resource;
>+                        res->base = hba->mmio_base;
>+                        continue;
>+                }
>+
>+                res->base = devm_ioremap_resource(hba->dev, res->resource);
>+                if (IS_ERR(res->base)) {
>+                        dev_err(hba->dev, "Failed to map res %s, err=%d\n",
>+                                         res->name, (int)PTR_ERR(res->base));
>+                        res->base = NULL;
>+                        ret = PTR_ERR(res->base);
>+                        return ret;
>+                }
>+        }
>+
>+        /* MCQ resource provided in DT */
>+        res = &hba->res[RES_MCQ];
>+        /* Bail if NCQ resource is provided */
Maybe MCQ?

>+        if (res->base)
>+                goto out;
>+
>+        /* Manually allocate MCQ resource from ufs_mem */
>+        res_mcq = res->resource;
Why we assign the value to res_mcq?

>+        res_mcq = devm_kzalloc(hba->dev, sizeof(*res_mcq), GFP_KERNEL);
>+        if (!res_mcq) {
>+                dev_err(hba->dev, "Failed to allocate MCQ resource\n");
>+                return ret;
>+        }
>+
>+        res_mcq->start = res_mem->start +
>+                         MCQ_SQATTR_OFFSET(hba->mcq_capabilities);
>+        res_mcq->end = res_mcq->start + hba->nr_hw_queues * MCQ_QCFG_SIZE - 1;
>+        res_mcq->flags = res_mem->flags;
>+        res_mcq->name = "mcq";
>+
>+        ret = insert_resource(&iomem_resource, res_mcq);
>+        if (ret) {
>+                dev_err(hba->dev, "Failed to insert MCQ resource, err=%d\n", ret);
Should we free the 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));
Should we call remove_resource and free the res_mcq?

Thanks,
Daejun