diff mbox series

[v6,04/10] scsi: ufs: introduce UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk

Message ID 20200417175944.47189-5-alim.akhtar@samsung.com (mailing list archive)
State Superseded
Headers show
Series exynos-ufs: Add support for UFS HCI | expand

Commit Message

Alim Akhtar April 17, 2020, 5:59 p.m. UTC 
Some UFS host controllers may think granularities of PRDT length and
offset as bytes, not double words.

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 30 +++++++++++++++++++++++-------
 drivers/scsi/ufs/ufshcd.h |  6 ++++++
 2 files changed, 29 insertions(+), 7 deletions(-)

Comments

Avri Altman April 20, 2020, 8:13 a.m. UTC | #1 
> 
> Some UFS host controllers may think granularities of PRDT length and
> offset as bytes, not double words.
> 
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Christoph Hellwig April 22, 2020, 6:54 a.m. UTC | #2 
On Fri, Apr 17, 2020 at 11:29:38PM +0530, Alim Akhtar wrote:
> Some UFS host controllers may think granularities of PRDT length and
> offset as bytes, not double words.
> 
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 30 +++++++++++++++++++++++-------
>  drivers/scsi/ufs/ufshcd.h |  6 ++++++
>  2 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index ee30ed6cc805..b32fcedcdcb9 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -2151,8 +2151,14 @@ static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>  		return sg_segments;
>  
>  	if (sg_segments) {
> -		lrbp->utr_descriptor_ptr->prd_table_length =
> -			cpu_to_le16((u16)sg_segments);
> +
> +		if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
> +			lrbp->utr_descriptor_ptr->prd_table_length =
> +				cpu_to_le16((u16)(sg_segments *
> +					sizeof(struct ufshcd_sg_entry)));
> +		else
> +			lrbp->utr_descriptor_ptr->prd_table_length =
> +				cpu_to_le16((u16) (sg_segments));

No double words here.  "Normal" UFS uses the actual segment count,
while Samsumg uses bytes.  Also no need fo the u16 count in
either the old or new version.
Alim Akhtar April 26, 2020, 3:56 p.m. UTC | #3 
Hi Christoph,

> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: 22 April 2020 12:24
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: robh@kernel.org; devicetree@vger.kernel.org;
linux-scsi@vger.kernel.org;
> krzk@kernel.org; avri.altman@wdc.com; martin.petersen@oracle.com;
> kwmad.kim@samsung.com; stanley.chu@mediatek.com;
> cang@codeaurora.org; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v6 04/10] scsi: ufs: introduce
> UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk
> 
> On Fri, Apr 17, 2020 at 11:29:38PM +0530, Alim Akhtar wrote:
> > Some UFS host controllers may think granularities of PRDT length and
> > offset as bytes, not double words.
> >
> > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 30 +++++++++++++++++++++++-------
> > drivers/scsi/ufs/ufshcd.h |  6 ++++++
> >  2 files changed, 29 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index ee30ed6cc805..b32fcedcdcb9 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -2151,8 +2151,14 @@ static int ufshcd_map_sg(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
> >  		return sg_segments;
> >
> >  	if (sg_segments) {
> > -		lrbp->utr_descriptor_ptr->prd_table_length =
> > -			cpu_to_le16((u16)sg_segments);
> > +
> > +		if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
> > +			lrbp->utr_descriptor_ptr->prd_table_length =
> > +				cpu_to_le16((u16)(sg_segments *
> > +					sizeof(struct ufshcd_sg_entry)));
> > +		else
> > +			lrbp->utr_descriptor_ptr->prd_table_length =
> > +				cpu_to_le16((u16) (sg_segments));
> 
> No double words here.  "Normal" UFS uses the actual segment count, while
> Samsumg uses bytes.  Also no need fo the u16 count in either the old or
new
> version.
Ok, will update the commit message and take your suggestion in the next
version.
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ee30ed6cc805..b32fcedcdcb9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2151,8 +2151,14 @@  static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 		return sg_segments;
 
 	if (sg_segments) {
-		lrbp->utr_descriptor_ptr->prd_table_length =
-			cpu_to_le16((u16)sg_segments);
+
+		if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
+			lrbp->utr_descriptor_ptr->prd_table_length =
+				cpu_to_le16((u16)(sg_segments *
+					sizeof(struct ufshcd_sg_entry)));
+		else
+			lrbp->utr_descriptor_ptr->prd_table_length =
+				cpu_to_le16((u16) (sg_segments));
 
 		prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr;
 
@@ -3500,11 +3506,21 @@  static void ufshcd_host_memory_configure(struct ufs_hba *hba)
 				cpu_to_le32(upper_32_bits(cmd_desc_element_addr));
 
 		/* Response upiu and prdt offset should be in double words */
-		utrdlp[i].response_upiu_offset =
-			cpu_to_le16(response_offset >> 2);
-		utrdlp[i].prd_table_offset = cpu_to_le16(prdt_offset >> 2);
-		utrdlp[i].response_upiu_length =
-			cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
+		if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN) {
+			utrdlp[i].response_upiu_offset =
+				cpu_to_le16(response_offset);
+			utrdlp[i].prd_table_offset =
+				cpu_to_le16(prdt_offset);
+			utrdlp[i].response_upiu_length =
+				cpu_to_le16(ALIGNED_UPIU_SIZE);
+		} else {
+			utrdlp[i].response_upiu_offset =
+				cpu_to_le16(response_offset >> 2);
+			utrdlp[i].prd_table_offset =
+				cpu_to_le16(prdt_offset >> 2);
+			utrdlp[i].response_upiu_length =
+				cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
+		}
 
 		ufshcd_init_lrb(hba, &hba->lrb[i], i);
 	}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index f8d08cb9caf7..a9b9ace9fc72 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -535,6 +535,12 @@  enum ufshcd_quirks {
 	 * enabled via HCE register.
 	 */
 	UFSHCI_QUIRK_BROKEN_HCE				= 1 << 8,
+
+	/*
+	 * This quirk needs to be enabled if the host controller regards
+	 * resolution of the values of PRDTO and PRDTL in UTRD as byte.
+	 */
+	UFSHCD_QUIRK_PRDT_BYTE_GRAN			= 1 << 9,
 };
 
 enum ufshcd_caps {