Re: [PATCH v5 14/16] ufs: mcq: Add completion support of a cqe

From: Manivannan Sadhasivam
Date: Mon Nov 28 2022 - 12:00:46 EST

On Tue, Nov 22, 2022 at 08:10:27PM -0800, Asutosh Das wrote:
> Add support for completing requests from Completion Queue.
> Some host controllers support vendor specific registers
> that provide a bitmap of all CQ's which have at least one
> completed CQE. Add this support.
> The MCQ specification doesn't provide the Task Tag or its
> equivalent in the Completion Queue Entry.
> So use an indirect method to find the Task Tag from the
> Completion Queue Entry.
>
> Co-developed-by: Can Guo <quic_cang@xxxxxxxxxxx>
> Signed-off-by: Can Guo <quic_cang@xxxxxxxxxxx>
> Signed-off-by: Asutosh Das <quic_asutoshd@xxxxxxxxxxx>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>

What is this reported by for?

> Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> drivers/ufs/core/ufs-mcq.c | 57 ++++++++++++++++++++++++++++++++++++++++++
> drivers/ufs/core/ufshcd-priv.h | 43 +++++++++++++++++++++++++++++++
> drivers/ufs/core/ufshcd.c | 37 +++++++++++++++++++++++++++
> drivers/ufs/host/ufs-qcom.c | 16 ++++++++++++
> drivers/ufs/host/ufs-qcom.h | 4 +++
> include/ufs/ufshcd.h | 7 ++++++
> include/ufs/ufshci.h | 3 +++
> 7 files changed, 167 insertions(+)
>
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 10a0d0d7..365ad98 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -28,6 +28,7 @@
> ((((c) >> 16) & MCQ_QCFGPTR_MASK) * MCQ_QCFGPTR_UNIT)
> #define MCQ_QCFG_SIZE 0x40
> #define MCQ_ENTRY_SIZE_IN_DWORD 8
> +#define CQE_UCD_BA GENMASK_ULL(63, 7)
>
> static int rw_queue_count_set(const char *val, const struct kernel_param *kp)
> {
> @@ -333,6 +334,59 @@ static void __iomem *mcq_opr_base(struct ufs_hba *hba,
> return opr->base + opr->stride * i;
> }
>
> +u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i)
> +{
> + return readl(mcq_opr_base(hba, OPR_CQIS, i) + REG_CQIS);
> +}
> +
> +void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i)
> +{
> + writel(val, mcq_opr_base(hba, OPR_CQIS, i) + REG_CQIS);
> +}
> +

It'd be good to add a comment about tag extraction as you did in description.

> +static int ufshcd_mcq_get_tag(struct ufs_hba *hba,
> + struct ufs_hw_queue *hwq,
> + struct cq_entry *cqe)
> +{
> + dma_addr_t dma_addr;
> +
> + /* sizeof(struct utp_transfer_cmd_desc) must be a multiple of 128 */
> + BUILD_BUG_ON(sizeof(struct utp_transfer_cmd_desc) & GENMASK(6, 0));
> +
> + /* Bits 63:7 UCD base address, 6:5 are reserved, 4:0 is SQ ID */
> + dma_addr = le64_to_cpu(cqe->command_desc_base_addr) & CQE_UCD_BA;
> +
> + return (dma_addr - hba->ucdl_dma_addr) /
> + sizeof(struct utp_transfer_cmd_desc);
> +}
> +

[...]

> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index dd53e85..89d29a1 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -1454,6 +1454,21 @@ static int ufs_qcom_get_hba_mac(struct ufs_hba *hba)
> return MAX_SUPP_MAC;
> }
>
> +static int ufs_qcom_get_outstanding_cqs(struct ufs_hba *hba,
> + unsigned long *ocqs)
> +{
> + u32 cqis_vs;
> + struct ufshcd_res_info *mcq_vs_res = &hba->res[RES_MCQ_VS];
> +
> + if (!mcq_vs_res->base)
> + return -EINVAL;
> +
> + cqis_vs = readl(mcq_vs_res->base + UFS_MEM_CQIS_VS);
> + *ocqs = cqis_vs;

Why can't you assign readl to *ocqs?

Thanks,
Mani
> +
> + return 0;
> +}
> +
> /*
> * struct ufs_hba_qcom_vops - UFS QCOM specific variant operations
> *
> @@ -1479,6 +1494,7 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
> .program_key = ufs_qcom_ice_program_key,
> .get_hba_mac = ufs_qcom_get_hba_mac,
> .op_runtime_config = ufs_qcom_op_runtime_config,
> + .get_outstanding_cqs = ufs_qcom_get_outstanding_cqs,
> };
>
> /**
> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
> index 44466a3..7769f03 100644
> --- a/drivers/ufs/host/ufs-qcom.h
> +++ b/drivers/ufs/host/ufs-qcom.h
> @@ -72,6 +72,10 @@ enum {
> UFS_UFS_DBG_RD_EDTL_RAM = 0x1900,
> };
>
> +enum {
> + UFS_MEM_CQIS_VS = 0x8,
> +};
> +
> #define UFS_CNTLR_2_x_x_VEN_REGS_OFFSET(x) (0x000 + x)
> #define UFS_CNTLR_3_x_x_VEN_REGS_OFFSET(x) (0x400 + x)
>
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 853c525..d5fde64 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -300,6 +300,7 @@ struct ufs_pwr_mode_info {
> * @event_notify: called to notify important events
> * @get_hba_mac: called to get vendor specific mac value, mandatory for mcq mode
> * @op_runtime_config: called to config Operation and runtime regs Pointers
> + * @get_outstanding_cqs: called to get outstanding completion queues
> */
> struct ufs_hba_variant_ops {
> const char *name;
> @@ -340,6 +341,8 @@ struct ufs_hba_variant_ops {
> enum ufs_event_type evt, void *data);
> int (*get_hba_mac)(struct ufs_hba *hba);
> int (*op_runtime_config)(struct ufs_hba *hba);
> + int (*get_outstanding_cqs)(struct ufs_hba *hba,
> + unsigned long *ocqs);
> };
>
> /* clock gating state */
> @@ -1064,6 +1067,8 @@ struct ufs_hba {
> * @id: hardware queue ID
> * @sq_tp_slot: current slot to which SQ tail pointer is pointing
> * @sq_lock: serialize submission queue access
> + * @cq_tail_slot: current slot to which CQ tail pointer is pointing
> + * @cq_head_slot: current slot to which CQ head pointer is pointing
> */
> struct ufs_hw_queue {
> void __iomem *mcq_sq_head;
> @@ -1079,6 +1084,8 @@ struct ufs_hw_queue {
> u32 id;
> u32 sq_tail_slot;
> spinlock_t sq_lock;
> + u32 cq_tail_slot;
> + u32 cq_head_slot;
> };
>
> static inline bool is_mcq_enabled(struct ufs_hba *hba)
> diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
> index c85bdf2..1e557ba 100644
> --- a/include/ufs/ufshci.h
> +++ b/include/ufs/ufshci.h
> @@ -262,6 +262,9 @@ enum {
> /* UTMRLRSR - UTP Task Management Request Run-Stop Register 80h */
> #define UTP_TASK_REQ_LIST_RUN_STOP_BIT 0x1
>
> +/* CQISy - CQ y Interrupt Status Register */
> +#define UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS 0x1
> +
> /* UICCMD - UIC Command */
> #define COMMAND_OPCODE_MASK 0xFF
> #define GEN_SELECTOR_INDEX_MASK 0xFFFF
> --
> 2.7.4
>

--
மணிவண்ணன் சதாசிவம்