| Message ID | 20230106215800.2249344-4-bvanassche@acm.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable DMA clustering in the UFS driver | expand |
> All UFS host controllers support DMA clustering. Hence enable DMA clustering. > > Note: without patch "Exynos: Fix the maximum segment size", this patch breaks > support for the Exynos controller. > > 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 > be18edf4ef7f..fe83fdda8d23 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -8429,7 +8429,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, Isn't DBC <= 256KB implies that for a boundary? Thanks, Avri > .rpm_autosuspend_delay = RPM_AUTOSUSPEND_DELAY_MS, };
On 1/8/23 00:20, Avri Altman wrote: >> All UFS host controllers support DMA clustering. Hence enable DMA clustering. >> >> Note: without patch "Exynos: Fix the maximum segment size", this patch breaks >> support for the Exynos controller. >> >> 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 >> be18edf4ef7f..fe83fdda8d23 100644 >> --- a/drivers/ufs/core/ufshcd.c >> +++ b/drivers/ufs/core/ufshcd.c >> @@ -8429,7 +8429,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, > > Isn't DBC <= 256KB implies that for a boundary? Hi Avri, I don't think so. I think that the 256 KiB limit for the PRDT corresponds to the max_segment_size parameter. 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 for UFS controllers other than the 256 KiB limit for a single PRDT and a limitation to 32-bits addresses for controllers that only support 32-bits DMA. The latter restriction is already handled by ufshcd_set_dma_mask(). Bart.
On 6/01/23 23:58, Bart Van Assche wrote: > All UFS host controllers support DMA clustering. Hence enable DMA > clustering. The patch history of ufshcd.c seems to suggest the dma_boundary setting was never intentional, but was inherited by default. However, I guess it is not impossible that removing the setting exposes issues for existing controllers. I suggest perhaps expanding upon the commit message and the cc list so more people will be aware of the change. Possibly worth including the explanation in your reply to Avri concerning PRDT 256K DBC limit > > Note: without patch "Exynos: Fix the maximum segment size", this patch > breaks support for the Exynos controller. > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Otherwise: Acked-by: Adrian Hunter <adrian.hunter@intel.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 be18edf4ef7f..fe83fdda8d23 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -8429,7 +8429,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, > }; >
On 1/9/23 23:56, Adrian Hunter wrote: > On 6/01/23 23:58, Bart Van Assche wrote: >> All UFS host controllers support DMA clustering. Hence enable DMA >> clustering. > > The patch history of ufshcd.c seems to suggest the dma_boundary > setting was never intentional, but was inherited by default. > > However, I guess it is not impossible that removing the setting > exposes issues for existing controllers. > > I suggest perhaps expanding upon the commit message and the > cc list so more people will be aware of the change. > > Possibly worth including the explanation in your reply to > Avri concerning PRDT 256K DBC limit > >> >> Note: without patch "Exynos: Fix the maximum segment size", this patch >> breaks support for the Exynos controller. >> >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> > > Otherwise: > > Acked-by: Adrian Hunter <adrian.hunter@intel.com> Hi Adrian, I will make the requested changes. Thanks for the review! Bart.
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index be18edf4ef7f..fe83fdda8d23 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -8429,7 +8429,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, };
All UFS host controllers support DMA clustering. Hence enable DMA clustering. Note: without patch "Exynos: Fix the maximum segment size", this patch breaks support for the Exynos controller. Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/ufs/core/ufshcd.c | 1 - 1 file changed, 1 deletion(-)