| Message ID | 1440149281-26336-3-git-send-email-alim.akhtar@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Few minor comments below, On Fri, Aug 21, 2015 at 2:57 PM, Alim Akhtar <alim.akhtar@samsung.com> wrote: > From: Seungwon Jeon <tgih.jun@samsung.com> > > UTRD(UTP Transfer Request Descriptor)'s field such as offset/length, > especially response's has DWORD expression. This quirk can be specified > for host controller not to conform standard. > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> > --- > drivers/scsi/ufs/ufshcd.c | 28 +++++++++++++++++++++------- > drivers/scsi/ufs/ufshcd.h | 7 +++++++ > 2 files changed, 28 insertions(+), 7 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index b0ade73..f882bf0 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -1009,7 +1009,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) > * > * Returns 0 in case of success, non-zero value in case of failure > */ > -static int ufshcd_map_sg(struct ufshcd_lrb *lrbp) > +static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) > { > struct ufshcd_sg_entry *prd_table; > struct scatterlist *sg; > @@ -1023,8 +1023,13 @@ static int ufshcd_map_sg(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 & UFSHCI_QUIRK_BROKEN_UTRD) > + 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; > > @@ -1347,7 +1352,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) > > /* form UPIU before issuing the command */ > ufshcd_compose_upiu(hba, lrbp); > - err = ufshcd_map_sg(lrbp); > + err = ufshcd_map_sg(hba, lrbp); > if (err) { > lrbp->cmd = NULL; > clear_bit_unlock(tag, &hba->lrb_in_use); > @@ -2035,12 +2040,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 */ This comment can be moved below for the else case. > - utrdlp[i].response_upiu_offset = > + if (hba->quirks & UFSHCI_QUIRK_BROKEN_UTRD) { > + 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 = > + utrdlp[i].prd_table_offset = > cpu_to_le16((prdt_offset >> 2)); > - utrdlp[i].response_upiu_length = > + utrdlp[i].response_upiu_length = > cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); > + } > > hba->lrb[i].utr_descriptor_ptr = (utrdlp + i); > hba->lrb[i].ucd_req_ptr = > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index c40a0e7..1fa5ac1 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -459,6 +459,13 @@ struct ufs_hba { > */ > #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) > > + /* > + * This quirk needs to be enabled if host controller doesn't conform > + * with UTRD. Some fields such as offset/length might not be in double word, > + * but in byte. > + */ > + #define UFSHCI_QUIRK_BROKEN_UTRD UFS_BIT(6) This macro name may be given more meaningful name such as UFSHCI_QUIRK_BYTE_ALIGN_UTRD or something similar. > + > unsigned int quirks; /* Deviations from standard UFSHCI spec. */ > > wait_queue_head_t tm_wq; > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Amit, Thanks for your review comments. On 08/26/2015 11:43 AM, amit daniel kachhap wrote: > Few minor comments below, > > On Fri, Aug 21, 2015 at 2:57 PM, Alim Akhtar <alim.akhtar@samsung.com> wrote: >> From: Seungwon Jeon <tgih.jun@samsung.com> >> >> UTRD(UTP Transfer Request Descriptor)'s field such as offset/length, >> especially response's has DWORD expression. This quirk can be specified >> for host controller not to conform standard. >> >> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >> --- >> drivers/scsi/ufs/ufshcd.c | 28 +++++++++++++++++++++------- >> drivers/scsi/ufs/ufshcd.h | 7 +++++++ >> 2 files changed, 28 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index b0ade73..f882bf0 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -1009,7 +1009,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) >> * >> * Returns 0 in case of success, non-zero value in case of failure >> */ >> -static int ufshcd_map_sg(struct ufshcd_lrb *lrbp) >> +static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) >> { >> struct ufshcd_sg_entry *prd_table; >> struct scatterlist *sg; >> @@ -1023,8 +1023,13 @@ static int ufshcd_map_sg(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 & UFSHCI_QUIRK_BROKEN_UTRD) >> + 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; >> >> @@ -1347,7 +1352,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) >> >> /* form UPIU before issuing the command */ >> ufshcd_compose_upiu(hba, lrbp); >> - err = ufshcd_map_sg(lrbp); >> + err = ufshcd_map_sg(hba, lrbp); >> if (err) { >> lrbp->cmd = NULL; >> clear_bit_unlock(tag, &hba->lrb_in_use); >> @@ -2035,12 +2040,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 */ > This comment can be moved below for the else case. ok >> - utrdlp[i].response_upiu_offset = >> + if (hba->quirks & UFSHCI_QUIRK_BROKEN_UTRD) { >> + 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 = >> + utrdlp[i].prd_table_offset = >> cpu_to_le16((prdt_offset >> 2)); >> - utrdlp[i].response_upiu_length = >> + utrdlp[i].response_upiu_length = >> cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); >> + } >> >> hba->lrb[i].utr_descriptor_ptr = (utrdlp + i); >> hba->lrb[i].ucd_req_ptr = >> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h >> index c40a0e7..1fa5ac1 100644 >> --- a/drivers/scsi/ufs/ufshcd.h >> +++ b/drivers/scsi/ufs/ufshcd.h >> @@ -459,6 +459,13 @@ struct ufs_hba { >> */ >> #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) >> >> + /* >> + * This quirk needs to be enabled if host controller doesn't conform >> + * with UTRD. Some fields such as offset/length might not be in double word, >> + * but in byte. >> + */ >> + #define UFSHCI_QUIRK_BROKEN_UTRD UFS_BIT(6) > This macro name may be given more meaningful name such as > UFSHCI_QUIRK_BYTE_ALIGN_UTRD or something similar. ok, will change >> + >> unsigned int quirks; /* Deviations from standard UFSHCI spec. */ >> >> wait_queue_head_t tm_wq; >> -- >> 1.7.10.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index b0ade73..f882bf0 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1009,7 +1009,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) * * Returns 0 in case of success, non-zero value in case of failure */ -static int ufshcd_map_sg(struct ufshcd_lrb *lrbp) +static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) { struct ufshcd_sg_entry *prd_table; struct scatterlist *sg; @@ -1023,8 +1023,13 @@ static int ufshcd_map_sg(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 & UFSHCI_QUIRK_BROKEN_UTRD) + 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; @@ -1347,7 +1352,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) /* form UPIU before issuing the command */ ufshcd_compose_upiu(hba, lrbp); - err = ufshcd_map_sg(lrbp); + err = ufshcd_map_sg(hba, lrbp); if (err) { lrbp->cmd = NULL; clear_bit_unlock(tag, &hba->lrb_in_use); @@ -2035,12 +2040,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 = + if (hba->quirks & UFSHCI_QUIRK_BROKEN_UTRD) { + 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 = + utrdlp[i].prd_table_offset = cpu_to_le16((prdt_offset >> 2)); - utrdlp[i].response_upiu_length = + utrdlp[i].response_upiu_length = cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); + } hba->lrb[i].utr_descriptor_ptr = (utrdlp + i); hba->lrb[i].ucd_req_ptr = diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index c40a0e7..1fa5ac1 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -459,6 +459,13 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) + /* + * This quirk needs to be enabled if host controller doesn't conform + * with UTRD. Some fields such as offset/length might not be in double word, + * but in byte. + */ + #define UFSHCI_QUIRK_BROKEN_UTRD UFS_BIT(6) + unsigned int quirks; /* Deviations from standard UFSHCI spec. */ wait_queue_head_t tm_wq;