Re: [PATCH V2 2/2] scsi: ufs: introduce quirk to extend PA_HIBERN8TIME for UFS devices
From: MANISH PANDEY
Date: Tue Apr 08 2025 - 13:29:09 EST
On 4/8/2025 10:01 PM, Manivannan Sadhasivam wrote:
On Tue, Apr 08, 2025 at 01:14:50PM +0530, MANISH PANDEY wrote:
On 4/8/2025 12:53 PM, Manivannan Sadhasivam wrote:
On Tue, Apr 08, 2025 at 11:07:58AM +0530, MANISH PANDEY wrote:
On 4/7/2025 12:05 AM, Manivannan Sadhasivam wrote:
On Fri, Apr 04, 2025 at 11:15:39PM +0530, Manish Pandey wrote:
Some UFS devices need additional time in hibern8 mode before exiting,
beyond the negotiated handshaking phase between the host and device.
Introduce a quirk to increase the PA_HIBERN8TIME parameter by 100 µs
to ensure proper hibernation process.
This commit message didn't mention the UFS device for which this quirk is being
applied.
Since it's a quirk and may be applicable to other vendors also in future, so
i thought to keep it general.
You cannot make commit message generic. It should precisely describe the change.
Will update in next patch set if required.
>> Signed-off-by: Manish Pandey <quic_mapa@xxxxxxxxxxx>
---
drivers/ufs/core/ufshcd.c | 31 +++++++++++++++++++++++++++++++
include/ufs/ufs_quirks.h | 6 ++++++
2 files changed, 37 insertions(+)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 464f13da259a..2b8203fe7b8c 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -278,6 +278,7 @@ static const struct ufs_dev_quirk ufs_fixups[] = {
.model = UFS_ANY_MODEL,
.quirk = UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |
UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE |
+ UFS_DEVICE_QUIRK_PA_HIBER8TIME |
UFS_DEVICE_QUIRK_RECOVERY_FROM_DL_NAC_ERRORS },
{ .wmanufacturerid = UFS_VENDOR_SKHYNIX,
.model = UFS_ANY_MODEL,
@@ -8384,6 +8385,33 @@ static int ufshcd_quirk_tune_host_pa_tactivate(struct ufs_hba *hba)
return ret;
}
+/**
+ * ufshcd_quirk_override_pa_h8time - Ensures proper adjustment of PA_HIBERN8TIME.
+ * @hba: per-adapter instance
+ *
+ * Some UFS devices require specific adjustments to the PA_HIBERN8TIME parameter
+ * to ensure proper hibernation timing. This function retrieves the current
+ * PA_HIBERN8TIME value and increments it by 100us.
+ */
+static void ufshcd_quirk_override_pa_h8time(struct ufs_hba *hba)
+{
+ u32 pa_h8time = 0;
Why do you need to initialize it?
Agree.. Not needed, will update.>> + int ret;
+
+ ret = ufshcd_dme_get(hba, UIC_ARG_MIB(PA_HIBERN8TIME),
+ &pa_h8time);
+ if (ret) {
+ dev_err(hba->dev, "Failed to get PA_HIBERN8TIME: %d\n", ret);
+ return;
+ }
+
+ /* Increment by 1 to increase hibernation time by 100 µs */
From where the value of 100us adjustment is coming from?
- Mani
These values are derived from experiments on Qualcomm SoCs.
However this is also matching with ufs-exynos.c
Okay. In that case, you should mention that the 100us value is derived from
experiments on Qcom and Samsung SoCs. Otherwise, it gives an assumption that
this value is universal.
- Mani
<< Otherwise, it gives an assumption that this value is universal. >>
So with this, should i add this quirk for Qcom only, or should add in
ufshcd.c and make it common for all SoC vendors?
You can add the quirk for both Qcom and Samsung. My comment was about clarifying
it in the kernel doc comments.
- Mani
Just for conclusion, why i moved this quirk from ufs-qcom to ufshcd.c is
as per Bart's suggestion in patchset
https://lore.kernel.org/lkml/c0691392-1523-4863-a722-d4f4640e4e28@xxxxxxx/
<< Which of these quirks are required for all host controllers and which
of these quirks are only required for Qualcomm host controllers?
> Should we consider moving the QUIRK_PA_HIBER8TIME quirk to the ufshcd
> driver? Please advise.
That would be appreciated. >>
Just to brief, QUIRK_PA_HIBER8TIME is required for Samsung UFS devices
and may be applicable to all SoC vendors with Samsung ufs device.
If you suggest to move it to ufs-qcom.c, i don't have any concern.
BTW Samsung UFS driver already has this implemented in their driver,
So i need not have to add this quirk to Samsung driver (ufs-exynos.c).
Regards
Manish