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

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

 



Hello Manivannan,

Thanks for taking a look.

Sorry for the late reponse, was a bit caught up.

On 8/12/2022 2:10 AM, Manivannan Sadhasivam wrote:
On Thu, Aug 11, 2022 at 03:33:03AM -0700, Can Guo wrote:
From: Asutosh Das <quic_asutoshd@xxxxxxxxxxx>

Adds MCQ support to UFS.

The design uses shared tags across all the hw queues.
The queue-depth is chosen within range supported by controller &
device. It also takes EXT_IID into account while choosing the queue
depth.

It supports default, read, poll and a dev cmd queue.
It enables MCQ after determining the queue-depth that the ufs
device supports.

Co-developed-by: Can Guo <quic_cang@xxxxxxxxxxx>

As per the Documentation, "Co-developed-by" should be followed by
"Signed-off-by" of the author. So you can just move this "Co-developed-by"
below Asutosh's S-o-b and that should be fine.

Ok.
Signed-off-by: Asutosh Das <quic_asutoshd@xxxxxxxxxxx>
Signed-off-by: Can Guo <quic_cang@xxxxxxxxxxx>
---
  drivers/ufs/core/Makefile  |   2 +-
  drivers/ufs/core/ufs-mcq.c | 524 +++++++++++++++++++++++++++++++++++++++++++++
  drivers/ufs/core/ufshcd.c  | 394 ++++++++++++++++++++++++++--------
  include/ufs/ufs.h          |   5 +
  include/ufs/ufshcd.h       | 223 ++++++++++++++++++-
  include/ufs/ufshci.h       |  77 +++++++
  6 files changed, 1135 insertions(+), 90 deletions(-)

This patch is too big to review. Could you please split it into multiple
patches?

The current changes make up a single functional base MCQ driver.
Usually splitting into multiple changes are per feature based.
Since this is a single feature, it makes sense to be separated out into core and host only. Refer 7a3e97b0d - the base ufs driver.



s/qd/depth

Here queue depth is abbreviated to qd which I think is inline with what the function does. So I prefer to keep it as qd.

+
+	val = ufshcd_readl(hba, REG_UFS_MCQ_CFG);


+struct ufs_hw_queue {
+	void *sqe_base_addr;

s/sqe_base_addr/sqe_base
 >> +	dma_addr_t sqe_dma_addr;

s/sqe_dma_addr/sqe_dma

+	struct cq_entry *cqe_base_addr;

s/cqe_base_addr/cqe_base

+	dma_addr_t cqe_dma_addr;

s/cqe_dma_addr/cqe_dma

Existing ufs driver dma addresses have the suffix addr, Refer struct ufshcd_lrb. So it is in line with the current naming convention.

+	u32 max_entries;
+	u32 id;
+
+	void __iomem *mcq_sq_hp;
+	void __iomem *mcq_sq_tp;
+	void __iomem *mcq_cq_hp;
+	void __iomem *mcq_cq_tp;
[...]

+			      MCQ_CFG_n(REG_SQATTR, i));
If you are writing to the same memory region, like in this case "mcq_base",
then you should be able to use _relaxed variants as the writes to the same
device memory are guaranteed to be in-order.

Not sure if I understand this correctly. Let me check this internally and get back.

This also removes the overhead associated with __iowmb included in writel.

Please audit this change throught the driver and use _relaxed variants where
applicable.


-asd

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.






[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