From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alim Akhtar Subject: Re: [PATCH 02/10] scsi: ufs: add quirk to contain unconformable utrd field Date: Fri, 28 Aug 2015 15:03:33 +0530 Message-ID: <55E02AED.8060209@samsung.com> References: <1440149281-26336-1-git-send-email-alim.akhtar@samsung.com> <1440149281-26336-3-git-send-email-alim.akhtar@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-reply-to: Sender: linux-kernel-owner@vger.kernel.org To: amit daniel kachhap Cc: linux-scsi@vger.kernel.org, "linux-kernel@vger.kernel.org" , vinholikatti@gmail.com, JBottomley@odin.com, Seungwon Jeon List-Id: linux-scsi@vger.kernel.org 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 wrote: >> From: Seungwon Jeon >> >> 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 >> Signed-off-by: Alim Akhtar >> --- >> 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/ >