| Message ID | 271ed77a0ff46390c90fdcde71890d8cec83b8c9.1665017636.git.quic_asutoshd@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Multi Circular Queue Support | expand |
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); I think res->base has already NULL value. Thanks, Daejun
Hi Eddie, On 10/14/2022 5:08 PM, Eddie Huang wrote: > Hi Asutosh, > > On Wed, 2022-10-05 at 18:06 -0700, Asutosh Das wrote: >> 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 */ >> + if (res->base) >> + goto out; >> + >> + /* Manually allocate MCQ resource from ufs_mem */ >> + res_mcq = res->resource; >> + 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); >> + return ret; >> + } >> + > Mediatek UFS hardware put MCQ SQ head/tail and SQ IS/IE together (SQ0 > head, SQ0 tail, SQ0 IS, SQ0 IE, CQ0 head, CQ0 tail....), which means > mcq_sqd register range interleave with mcq_sqis. I suggest let vendor > customize config mcq resource function to fit vendor's register > assignment In your case, which is similar to ours, you can just provide the res of SQD in DT, then use the ufshcd_mcq_vops_op_runtime_config() introduced in Patch #8 to configure each operation and runtime pointers like we are doing in ufs_qcom_op_runtime_config(). Please let us know if it is not enough for your case. Regards, Can Guo. > > Regards, > Eddie Huang > >
On Tue, Oct 18 2022 at 19:57 -0700, Eddie Huang wrote: >Hi Asutosh, > [...] >> >> How about we add a vops to ufshcd_mcq_config_resource(). >> SoC vendors should make sure that the vops populates the mcq_base. >> > >It is good to add vops to ufshcd_mcq_config_resource() let SoC vendors >populate mcq_base > While adding the vops it occurred to me that it'd remain empty because ufs-qcom doesn't need it. And I'm not sure how MTK register space is laid out. So please can you help add a vops in the next version? I can address the other comment and push the series. Please let me know. > >Thanks, >Eddie Huang > -asd >
On 10/19/22 12:50, Asutosh Das wrote: > While adding the vops it occurred to me that it'd remain empty because > ufs-qcom > doesn't need it. And I'm not sure how MTK register space is laid out. > So please can you help add a vops in the next version? > I can address the other comment and push the series. Please do not introduce new vops without adding at least one implementation of the vop in the same patch series. How about letting MediaTek add more vops as necessary in a separate patch series and focusing in this patch series on UFSHCI 4.0 compliance? Thanks, Bart.
On Wed, Oct 19 2022 at 14:07 -0700, Bart Van Assche wrote: >On 10/19/22 12:50, Asutosh Das wrote: >>While adding the vops it occurred to me that it'd remain empty >>because ufs-qcom >>doesn't need it. And I'm not sure how MTK register space is laid out. >>So please can you help add a vops in the next version? >>I can address the other comment and push the series. > >Please do not introduce new vops without adding at least one >implementation of the vop in the same patch series. How about letting >MediaTek add more vops as necessary in a separate patch series and >focusing in this patch series on UFSHCI 4.0 compliance? > Yep, I agree, I'll push the version shortly without the vops. -asd >Thanks, > >Bart.
On Wed, 2022-10-19 at 14:06 -0700, Bart Van Assche wrote: > On 10/19/22 12:50, Asutosh Das wrote: > > While adding the vops it occurred to me that it'd remain empty > > because > > ufs-qcom > > doesn't need it. And I'm not sure how MTK register space is laid > > out. > > So please can you help add a vops in the next version? > > I can address the other comment and push the series. > > Please do not introduce new vops without adding at least one > implementation of the vop in the same patch series. How about > letting > MediaTek add more vops as necessary in a separate patch series and > focusing in this patch series on UFSHCI 4.0 compliance? > I am not sure whether other SoC vendor's register definition follow this patch's arrangement or not. As I explain Mediatek treat UFS as a single IP and map whole UFS register address space one time. To speed up landing this series, please bypass this vops. I will send a separate patch to add this vops Eddie
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 */ + if (res->base) + goto out; + + /* Manually allocate MCQ resource from ufs_mem */ + res_mcq = res->resource; + 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); + 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; + return ret; + } + +out: + hba->mcq_base = res->base; + return 0; +} + static int ufshcd_mcq_config_nr_queues(struct ufs_hba *hba) { int i; @@ -107,7 +203,10 @@ int ufshcd_mcq_init(struct ufs_hba *hba) int ret; ret = ufshcd_mcq_config_nr_queues(hba); + if (ret) + return ret; + ret = ufshcd_mcq_config_resource(hba); return ret; } diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 298e103..5a5132d 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -720,6 +720,30 @@ struct ufs_hba_monitor { }; /** + * struct ufshcd_res_info_t - MCQ related resource regions + * + * @name: resource name + * @resource: pointer to resource region + * @base: register base address + */ +struct ufshcd_res_info { + const char *name; + struct resource *resource; + void __iomem *base; +}; + +enum ufshcd_res { + RES_UFS, + RES_MCQ, + RES_MCQ_SQD, + RES_MCQ_SQIS, + RES_MCQ_CQD, + RES_MCQ_CQIS, + RES_MCQ_VS, + RES_MAX, +}; + +/** * struct ufs_hba - per adapter private structure * @mmio_base: UFSHCI base register address * @ucdl_base_addr: UFS Command Descriptor base address @@ -829,6 +853,8 @@ struct ufs_hba_monitor { * @mcq_sup: is mcq supported by UFSHC * @nr_hw_queues: number of hardware queues configured * @nr_queues: number of Queues of different queue types + * @res: array of resource info of MCQ registers + * @mcq_base: Multi circular queue registers base address */ struct ufs_hba { void __iomem *mmio_base; @@ -981,6 +1007,8 @@ struct ufs_hba { bool mcq_sup; unsigned int nr_hw_queues; unsigned int nr_queues[HCTX_MAX_TYPES]; + struct ufshcd_res_info res[RES_MAX]; + void __iomem *mcq_base; }; static inline bool is_mcq_supported(struct ufs_hba *hba)