[PATCH v2 0/3] Enable DMA clustering in the UFS driver

[PATCH v2 0/3] Enable DMA clustering in the UFS driver

[Bart Van Assche]

Hi Martin,

The third patch in this series enables DMA clustering in the UFS driver since
UFS host controllers support DMA clustering. The first two patches fix bugs in
the Exynos host controller driver.

Please consider this patch series for the next merge window.

Thanks,

Bart.

Changes compared to v1:
- Expanded the description of patch 3/3.
- Reworked patch 2/3 such that setting host->max_segment_size no longer races with
  LUN scanning.

Bart Van Assche (3):
  scsi: ufs: Exynos: Fix DMA alignment for PAGE_SIZE != 4096
  scsi: ufs: Exynos: Fix the maximum segment size
  scsi: ufs: Enable DMA clustering

 drivers/ufs/core/ufshcd.c     |  5 ++---
 drivers/ufs/host/ufs-exynos.c | 10 +++++++++-
 include/ufs/ufshcd.h          |  4 ++--
 3 files changed, 13 insertions(+), 6 deletions(-)

[PATCH v2 1/3] scsi: ufs: Exynos: Fix DMA alignment for PAGE_SIZE != 4096

[Bart Van Assche]

The Exynos UFS controller only supports scatter/gather list elements
that are aligned on a 4 KiB boundary. Fix DMA alignment in case
PAGE_SIZE != 4096. Rename UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE into
UFSHCD_QUIRK_4KB_DMA_ALIGNMENT.

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
Tested-by: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Kiwoong Kim <kwmad.kim@samsung.com>
Fixes: 2b2bfc8aa519 ("scsi: ufs: Introduce a quirk to allow only page-aligned sg entries")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c     | 4 ++--
 drivers/ufs/host/ufs-exynos.c | 2 +-
 include/ufs/ufshcd.h          | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0514669e03be..5fdbc983ce2e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5029,8 +5029,8 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
 	ufshcd_hpb_configure(hba, sdev);
 
 	blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
-	if (hba->quirks & UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE)
-		blk_queue_update_dma_alignment(q, PAGE_SIZE - 1);
+	if (hba->quirks & UFSHCD_QUIRK_4KB_DMA_ALIGNMENT)
+		blk_queue_update_dma_alignment(q, 4096 - 1);
 	/*
 	 * Block runtime-pm until all consumers are added.
 	 * Refer ufshcd_setup_links().
diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
index c3628a8645a5..3cdac89a28b8 100644
--- a/drivers/ufs/host/ufs-exynos.c
+++ b/drivers/ufs/host/ufs-exynos.c
@@ -1673,7 +1673,7 @@ static const struct exynos_ufs_drv_data exynos_ufs_drvs = {
 				  UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR |
 				  UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL |
 				  UFSHCD_QUIRK_SKIP_DEF_UNIPRO_TIMEOUT_SETTING |
-				  UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE,
+				  UFSHCD_QUIRK_4KB_DMA_ALIGNMENT,
 	.opts			= EXYNOS_UFS_OPT_HAS_APB_CLK_CTRL |
 				  EXYNOS_UFS_OPT_BROKEN_AUTO_CLK_CTRL |
 				  EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX |
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index fc7373a1a15e..ca32df0ce6ba 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -570,9 +570,9 @@ enum ufshcd_quirks {
 	UFSHCD_QUIRK_SKIP_DEF_UNIPRO_TIMEOUT_SETTING = 1 << 13,
 
 	/*
-	 * This quirk allows only sg entries aligned with page size.
+	 * Align DMA SG entries on a 4 KiB boundary.
 	 */
-	UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE		= 1 << 14,
+	UFSHCD_QUIRK_4KB_DMA_ALIGNMENT			= 1 << 14,
 
 	/*
 	 * This quirk needs to be enabled if the host controller does not

[PATCH v2 2/3] scsi: ufs: Exynos: Fix the maximum segment size

[Bart Van Assche]

Prepare for enabling DMA clustering and also for supporting
PAGE_SIZE != 4096 by declaring explicitly that the maximum segment
size is 4096 bytes for Exynos UFS host controllers. Add this code
in exynos_ufs_hce_enable_notify() such that it happens after
scsi_host_alloc() and before __scsi_init_queue() is called by the
LUN scanning code.

Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Kiwoong Kim <kwmad.kim@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/host/ufs-exynos.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
index 3cdac89a28b8..7c985fc38db1 100644
--- a/drivers/ufs/host/ufs-exynos.c
+++ b/drivers/ufs/host/ufs-exynos.c
@@ -1300,6 +1300,14 @@ static int exynos_ufs_hce_enable_notify(struct ufs_hba *hba,
 
 	switch (status) {
 	case PRE_CHANGE:
+		/*
+		 * The maximum segment size must be set after scsi_host_alloc()
+		 * has been called and before LUN scanning starts
+		 * (ufshcd_async_scan()). Note: this callback may also be called
+		 * from other functions than ufshcd_init().
+		 */
+		hba->host->max_segment_size = 4096;
+
 		if (ufs->drv_data->pre_hce_enable) {
 			ret = ufs->drv_data->pre_hce_enable(ufs);
 			if (ret)

[PATCH v2 3/3] scsi: ufs: Enable DMA clustering

[Bart Van Assche]

All UFS host controllers support DMA clustering. Hence enable DMA
clustering.

Notes:
- The max_segment_size parameter implements the 256 KiB limit for the
  PRDT. The dma_boundary parameter represents a boundary that must not
  be crossed by DMA scatter/gather lists. I'm not aware of any
  restrictions on DMA scatter/gather lists in the UFSHCI specification
  other than the 256 KiB limit for the PRDT and the 32-bit address
  restriction for controllers that only support 32-bits DMA. The latter
  restriction is already handled by ufshcd_set_dma_mask().
- Without patch "Exynos: Fix the maximum segment size", this patch
  breaks support for the Exynos controller.

The history of the dma_boundary parameter in the UFS driver is as
follows:
* The initial UFS driver did not set the dma_boundary parameter.
* Commit 4dd4130a722f ("scsi: make sure all drivers set the
  use_clustering flag") set the .use_clustering flag.
* Commit 4af14d113bcf ("scsi: remove the use_clustering flag") removed
  the use_clustering flag and set the dma_boundary parameter instead.

Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Kiwoong Kim <kwmad.kim@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 5fdbc983ce2e..d28b44a1ffcf 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8460,7 +8460,6 @@ static struct scsi_host_template ufshcd_driver_template = {
 	.max_host_blocked	= 1,
 	.track_queue_depth	= 1,
 	.sdev_groups		= ufshcd_driver_groups,
-	.dma_boundary		= PAGE_SIZE - 1,
 	.rpm_autosuspend_delay	= RPM_AUTOSUSPEND_DELAY_MS,
 };
 

Re: [PATCH v2 2/3] scsi: ufs: Exynos: Fix the maximum segment size

[Martin K. Petersen]

Alim,

> Prepare for enabling DMA clustering and also for supporting
> PAGE_SIZE != 4096 by declaring explicitly that the maximum segment
> size is 4096 bytes for Exynos UFS host controllers. Add this code
> in exynos_ufs_hce_enable_notify() such that it happens after
> scsi_host_alloc() and before __scsi_init_queue() is called by the
> LUN scanning code.

Now that you're Exynos maintainer it falls upon you to review Bart's
patch.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH v2 2/3] scsi: ufs: Exynos: Fix the maximum segment size

[Alim Akhtar]

>-----Original Message-----
>From: Bart Van Assche [mailto:bvanassche@acm.org]
>Sent: Friday, January 13, 2023 5:12 AM
>To: Martin K . Petersen <martin.petersen@oracle.com>
>Cc: Jaegeuk Kim <jaegeuk@kernel.org>; linux-scsi@vger.kernel.org; Adrian
>Hunter <adrian.hunter@intel.com>; Alim Akhtar
><alim.akhtar@samsung.com>; Bart Van Assche <bvanassche@acm.org>;
>Kiwoong Kim <kwmad.kim@samsung.com>; James E.J. Bottomley
><jejb@linux.ibm.com>; Krzysztof Kozlowski
><krzysztof.kozlowski@linaro.org>; Chanho Park
><chanho61.park@samsung.com>; Bean Huo <beanhuo@micron.com>
>Subject: [PATCH v2 2/3] scsi: ufs: Exynos: Fix the maximum segment size
>
>Prepare for enabling DMA clustering and also for supporting PAGE_SIZE !=
>4096 by declaring explicitly that the maximum segment size is 4096 bytes
for
>Exynos UFS host controllers. Add this code in
>exynos_ufs_hce_enable_notify() such that it happens after
>scsi_host_alloc() and before __scsi_init_queue() is called by the LUN
scanning
>code.
>
>Cc: Alim Akhtar <alim.akhtar@samsung.com>
>Cc: Kiwoong Kim <kwmad.kim@samsung.com>
>Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

Tested basic read/write on platform containing Exynos UFS HCI, so 

Tested-by: Alim Akhtar <alim.akhtar@samsung.com>

> drivers/ufs/host/ufs-exynos.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
>index 3cdac89a28b8..7c985fc38db1 100644
>--- a/drivers/ufs/host/ufs-exynos.c
>+++ b/drivers/ufs/host/ufs-exynos.c
>@@ -1300,6 +1300,14 @@ static int exynos_ufs_hce_enable_notify(struct
>ufs_hba *hba,
>
> 	switch (status) {
> 	case PRE_CHANGE:
>+		/*
>+		 * The maximum segment size must be set after
>scsi_host_alloc()
>+		 * has been called and before LUN scanning starts
>+		 * (ufshcd_async_scan()). Note: this callback may also be
>called
>+		 * from other functions than ufshcd_init().
>+		 */
>+		hba->host->max_segment_size = 4096;
>+
> 		if (ufs->drv_data->pre_hce_enable) {
> 			ret = ufs->drv_data->pre_hce_enable(ufs);
> 			if (ret)

RE: [PATCH v2 3/3] scsi: ufs: Enable DMA clustering

[Alim Akhtar]

>-----Original Message-----
>From: Bart Van Assche [mailto:bvanassche@acm.org]
>Sent: Friday, January 13, 2023 5:12 AM
>To: Martin K . Petersen <martin.petersen@oracle.com>
>Cc: Jaegeuk Kim <jaegeuk@kernel.org>; linux-scsi@vger.kernel.org; Adrian
>Hunter <adrian.hunter@intel.com>; Alim Akhtar
><alim.akhtar@samsung.com>; Bart Van Assche <bvanassche@acm.org>; Avri
>Altman <avri.altman@wdc.com>; Kiwoong Kim <kwmad.kim@samsung.com>;
>James E.J. Bottomley <jejb@linux.ibm.com>; Bean Huo
><beanhuo@micron.com>; Stanley Chu <stanley.chu@mediatek.com>;
>Jinyoung Choi <j-young.choi@samsung.com>
>Subject: [PATCH v2 3/3] scsi: ufs: Enable DMA clustering
>
>All UFS host controllers support DMA clustering. Hence enable DMA
>clustering.
>
>Notes:
>- The max_segment_size parameter implements the 256 KiB limit for the
>  PRDT. The dma_boundary parameter represents a boundary that must not
>  be crossed by DMA scatter/gather lists. I'm not aware of any
>  restrictions on DMA scatter/gather lists in the UFSHCI specification
>  other than the 256 KiB limit for the PRDT and the 32-bit address
>  restriction for controllers that only support 32-bits DMA. The latter
>  restriction is already handled by ufshcd_set_dma_mask().
>- Without patch "Exynos: Fix the maximum segment size", this patch
>  breaks support for the Exynos controller.
>
>The history of the dma_boundary parameter in the UFS driver is as
>follows:
>* The initial UFS driver did not set the dma_boundary parameter.
>* Commit 4dd4130a722f ("scsi: make sure all drivers set the
>  use_clustering flag") set the .use_clustering flag.
>* Commit 4af14d113bcf ("scsi: remove the use_clustering flag") removed
>  the use_clustering flag and set the dma_boundary parameter instead.
>
>Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>Cc: Avri Altman <avri.altman@wdc.com>
>Cc: Alim Akhtar <alim.akhtar@samsung.com>
>Cc: Kiwoong Kim <kwmad.kim@samsung.com>
>Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>


> drivers/ufs/core/ufshcd.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
>5fdbc983ce2e..d28b44a1ffcf 100644
>--- a/drivers/ufs/core/ufshcd.c
>+++ b/drivers/ufs/core/ufshcd.c
>@@ -8460,7 +8460,6 @@ static struct scsi_host_template
>ufshcd_driver_template = {
> 	.max_host_blocked	= 1,
> 	.track_queue_depth	= 1,
> 	.sdev_groups		= ufshcd_driver_groups,
>-	.dma_boundary		= PAGE_SIZE - 1,
> 	.rpm_autosuspend_delay	= RPM_AUTOSUSPEND_DELAY_MS,
> };
>

Re: [PATCH v2 0/3] Enable DMA clustering in the UFS driver

[Martin K. Petersen]

Bart,

> The third patch in this series enables DMA clustering in the UFS
> driver since UFS host controllers support DMA clustering. The first
> two patches fix bugs in the Exynos host controller driver.

Applied to 6.3/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering