| Message ID | 20200417175944.47189-5-alim.akhtar@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6,01/10] scsi: ufs: add quirk to fix mishandling utrlclr/utmrlclr | expand |
> > 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>
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.
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 --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 {