Replace manual offset calculations for response_upiu and prd_table in ufshcd_init_lrb() with pre-calculated offsets already stored in the utp_transfer_req_desc structure. The pre-calculated offsets are set differently in ufshcd_host_memory_configure() based on the UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk, ensuring correct alignment and access.
Fixes: 26f968d7de82 ("scsi: ufs: Introduce UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk") Cc: stable@vger.kernel.org Signed-off-by: Avri Altman avri.altman@wdc.com
--- Changes in v2: - add Fixes: and Cc: stable tags - fix kernel test robot warning about type mismatch by using le16_to_cpu --- drivers/ufs/core/ufshcd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 8ea5a82503a9..85251c176ef7 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -2919,9 +2919,8 @@ static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i) struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr; dma_addr_t cmd_desc_element_addr = hba->ucdl_dma_addr + i * ufshcd_get_ucd_size(hba); - u16 response_offset = offsetof(struct utp_transfer_cmd_desc, - response_upiu); - u16 prdt_offset = offsetof(struct utp_transfer_cmd_desc, prd_table); + u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset); + u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset);
lrb->utr_descriptor_ptr = utrdlp + i; lrb->utrd_dma_addr = hba->utrdl_dma_addr +
On 9/9/24 9:45 PM, Avri Altman wrote:
Replace manual offset calculations for response_upiu and prd_table in ufshcd_init_lrb() with pre-calculated offsets already stored in the utp_transfer_req_desc structure. The pre-calculated offsets are set differently in ufshcd_host_memory_configure() based on the UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk, ensuring correct alignment and access.
With which host controllers has this patch been tested?
Thanks,
Bart.
On 9/9/24 9:45 PM, Avri Altman wrote:
Replace manual offset calculations for response_upiu and prd_table in ufshcd_init_lrb() with pre-calculated offsets already stored in the utp_transfer_req_desc structure. The pre-calculated offsets are set differently in ufshcd_host_memory_configure() based on the UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk, ensuring correct alignment and access.
With which host controllers has this patch been tested?
Qualcomm RB5 platform. I guess I'll be needing help with testing it on an exynos platforms?
Thanks, Avri
Thanks,
Bart.
+Alim
On 9/9/24 9:45 PM, Avri Altman wrote:
Replace manual offset calculations for response_upiu and prd_table in ufshcd_init_lrb() with pre-calculated offsets already stored in the utp_transfer_req_desc structure. The pre-calculated offsets are set differently in ufshcd_host_memory_configure() based on the UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk, ensuring correct alignment and access.
With which host controllers has this patch been tested?
Qualcomm RB5 platform. I guess I'll be needing help with testing it on an exynos platforms?
Would appreciate any help testing it on Exynos.
Thanks, Avri
Thanks, Avri
Thanks,
Bart.
On 9/9/24 9:45 PM, Avri Altman wrote:
Replace manual offset calculations for response_upiu and prd_table in ufshcd_init_lrb() with pre-calculated offsets already stored in the utp_transfer_req_desc structure. The pre-calculated offsets are set differently in ufshcd_host_memory_configure() based on the UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk, ensuring correct alignment and access.
Fixes: 26f968d7de82 ("scsi: ufs: Introduce UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk") Cc: stable@vger.kernel.org Signed-off-by: Avri Altman avri.altman@wdc.com
Changes in v2:
- add Fixes: and Cc: stable tags
- fix kernel test robot warning about type mismatch by using le16_to_cpu
drivers/ufs/core/ufshcd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 8ea5a82503a9..85251c176ef7 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -2919,9 +2919,8 @@ static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i) struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr; dma_addr_t cmd_desc_element_addr = hba->ucdl_dma_addr + i * ufshcd_get_ucd_size(hba);
- u16 response_offset = offsetof(struct utp_transfer_cmd_desc,
response_upiu);- u16 prdt_offset = offsetof(struct utp_transfer_cmd_desc, prd_table);
- u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset);
- u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset);
lrb->utr_descriptor_ptr = utrdlp + i; lrb->utrd_dma_addr = hba->utrdl_dma_addr +
Please always Cc the author of the original patch when posting a candidate fix.
Alim, since the upstream kernel code seems to work fine with Exynos UFS host controllers, is the description of UFSHCD_QUIRK_PRDT_BYTE_GRAN perhaps wrong? I'm referring to the following description:
/* * 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,
Thanks,
Bart.
+Daejun
Thanks, Avri
On 9/9/24 9:45 PM, Avri Altman wrote:
Replace manual offset calculations for response_upiu and prd_table in ufshcd_init_lrb() with pre-calculated offsets already stored in the utp_transfer_req_desc structure. The pre-calculated offsets are set differently in ufshcd_host_memory_configure() based on the UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk, ensuring correct alignment and access.
Fixes: 26f968d7de82 ("scsi: ufs: Introduce
UFSHCD_QUIRK_PRDT_BYTE_GRAN
quirk") Cc: stable@vger.kernel.org Signed-off-by: Avri Altman avri.altman@wdc.com
Changes in v2:
- add Fixes: and Cc: stable tags
- fix kernel test robot warning about type mismatch by using
le16_to_cpu
drivers/ufs/core/ufshcd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 8ea5a82503a9..85251c176ef7 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -2919,9 +2919,8 @@ static void ufshcd_init_lrb(struct ufs_hba *hba,
struct ufshcd_lrb *lrb, int i)
struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr; dma_addr_t cmd_desc_element_addr = hba->ucdl_dma_addr + i * ufshcd_get_ucd_size(hba);
u16 response_offset = offsetof(struct utp_transfer_cmd_desc,response_upiu);u16 prdt_offset = offsetof(struct utp_transfer_cmd_desc, prd_table);
u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset);u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset); lrb->utr_descriptor_ptr = utrdlp + i; lrb->utrd_dma_addr = hba->utrdl_dma_addr +Please always Cc the author of the original patch when posting a candidate fix.
Alim, since the upstream kernel code seems to work fine with Exynos UFS host controllers, is the description of UFSHCD_QUIRK_PRDT_BYTE_GRAN perhaps wrong? I'm referring to the following description:
/* * 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,Thanks,
Bart.
+Daejun
Thanks, Avri
Bart - How do you want to proceed with this fix?
Thanks, Avri
On 9/9/24 9:45 PM, Avri Altman wrote:
Replace manual offset calculations for response_upiu and prd_table in ufshcd_init_lrb() with pre-calculated offsets already stored in the utp_transfer_req_desc structure. The pre-calculated offsets are set differently in ufshcd_host_memory_configure() based on the UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk, ensuring correct alignment and access.
Fixes: 26f968d7de82 ("scsi: ufs: Introduce
UFSHCD_QUIRK_PRDT_BYTE_GRAN
quirk") Cc: stable@vger.kernel.org Signed-off-by: Avri Altman avri.altman@wdc.com
Changes in v2:
- add Fixes: and Cc: stable tags
- fix kernel test robot warning about type mismatch by using
le16_to_cpu
drivers/ufs/core/ufshcd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 8ea5a82503a9..85251c176ef7 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -2919,9 +2919,8 @@ static void ufshcd_init_lrb(struct ufs_hba *hba,
struct ufshcd_lrb *lrb, int i)
struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr; dma_addr_t cmd_desc_element_addr = hba->ucdl_dma_addr + i * ufshcd_get_ucd_size(hba);
u16 response_offset = offsetof(struct utp_transfer_cmd_desc,response_upiu);u16 prdt_offset = offsetof(struct utp_transfer_cmd_desc, prd_table);
u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset);u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset); lrb->utr_descriptor_ptr = utrdlp + i; lrb->utrd_dma_addr = hba->utrdl_dma_addr +Please always Cc the author of the original patch when posting a candidate
fix.
Alim, since the upstream kernel code seems to work fine with Exynos UFS host controllers, is the description of UFSHCD_QUIRK_PRDT_BYTE_GRAN perhaps wrong? I'm referring to the
following description:
/* * 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,Thanks,
Bart.
On 10/1/24 12:19 AM, Avri Altman wrote:
Bart - How do you want to proceed with this fix?
Since this patch affects the Exynos UFSHCI, I think we need feedback from the maintainers of the Exynos UFSHCI code.
Personally I'm hesitant to proceed with this patch. As you may know the android-mainline kernel tracks the upstream kernel very closely. Currently that kernel branch is only one week behind Linus' master branch. The android-mainline kernel boots fine on Pixel 6 devices. These devices have an Exynos UFS host controller. Since this patch modifies the behavior for the Exynos UFS host controller there is a potential that it will break the support for that controller.
Thanks,
Bart.
On 10/1/24 12:19 AM, Avri Altman wrote:
Bart - How do you want to proceed with this fix?
As one can see here [1], if this patch is applied on top of the android-mainline kernel branch (close to v6.12-rc1) then all presubmit tests pass. This includes booting the kernel on a Pixel 6 device that has an Exynos UFS host controller. So I'm fine with this patch.
[1] https://android-review.googlesource.com/c/kernel/common/+/3291741
Thanks,
Bart.
On Tue, 10 Sep 2024 07:45:43 +0300, Avri Altman wrote:
Replace manual offset calculations for response_upiu and prd_table in ufshcd_init_lrb() with pre-calculated offsets already stored in the utp_transfer_req_desc structure. The pre-calculated offsets are set differently in ufshcd_host_memory_configure() based on the UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk, ensuring correct alignment and access.
[...]
Applied to 6.12/scsi-fixes, thanks!
[1/1] scsi: ufs: Use pre-calculated offsets in ufshcd_init_lrb https://git.kernel.org/mkp/scsi/c/d5130c5a0932
linux-stable-mirror@lists.linaro.org
-
Avri Altman -
Avri Altman
-
Bart Van Assche -
Martin K. Petersen