[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <69a111a2-219e-e3d4-8b89-3400facc02e3@quicinc.com>
Date: Thu, 2 Oct 2025 11:48:51 -0700
From: "Bao D. Nguyen" <quic_nguyenb@...cinc.com>
To: Peter Wang (王信友) <peter.wang@...iatek.com>,
"avri.altman@....com" <avri.altman@....com>,
"quic_cang@...cinc.com"
<quic_cang@...cinc.com>,
"quic_nitirawa@...cinc.com"
<quic_nitirawa@...cinc.com>,
"manivannan.sadhasivam@...aro.org"
<manivannan.sadhasivam@...aro.org>,
"bvanassche@....org"
<bvanassche@....org>,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>,
"martin.petersen@...cle.com" <martin.petersen@...cle.com>
CC: "beanhuo@...ron.com" <beanhuo@...ron.com>,
"chu.stanley@...il.com"
<chu.stanley@...il.com>,
"quic_mapa@...cinc.com" <quic_mapa@...cinc.com>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"AngeloGioacchino
Del Regno" <angelogioacchino.delregno@...labora.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"matthias.bgg@...il.com"
<matthias.bgg@...il.com>,
"James.Bottomley@...senPartnership.com"
<James.Bottomley@...senPartnership.com>,
"mani@...nel.org" <mani@...nel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH v1 1/2] scsi: ufs: core: Remove
UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk
On 10/2/2025 12:57 AM, Peter Wang (王信友) wrote:
> On Wed, 2025-10-01 at 13:57 -0700, Bao D. Nguyen wrote:
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 2e1fa8c..45e509b 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -9738,10 +9738,9 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba
>> *hba)
>> }
>>
>> /*
>> - * Some UFS devices require delay after VCC power rail is
>> turned-off.
>> + * All UFS devices require delay after VCC power rail is
>> turned-off.
>> */
>> - if (vcc_off && hba->vreg_info.vcc &&
>> - hba->dev_quirks & UFS_DEVICE_QUIRK_DELAY_AFTER_LPM)
>> + if (vcc_off && hba->vreg_info.vcc)
>> usleep_range(5000, 5100);
>> }
>>
>> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
>> mediatek.c
>> index f902ce0..5c204d1 100644
>> --- a/drivers/ufs/host/ufs-mediatek.c
>> +++ b/drivers/ufs/host/ufs-mediatek.c
>> @@ -40,8 +40,7 @@ static int ufs_mtk_config_mcq(struct ufs_hba *hba,
>> bool irq);
>> static const struct ufs_dev_quirk ufs_mtk_dev_fixups[] = {
>> { .wmanufacturerid = UFS_ANY_VENDOR,
>> .model = UFS_ANY_MODEL,
>> - .quirk = UFS_DEVICE_QUIRK_DELAY_AFTER_LPM |
>> - UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
>> + .quirk = UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
>> { .wmanufacturerid = UFS_VENDOR_SKHYNIX,
>> .model = "H9HQ21AFAMZDAR",
>> .quirk = UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES },
>> @@ -1713,15 +1712,13 @@ static void ufs_mtk_fixup_dev_quirks(struct
>> ufs_hba *hba)
>> {
>> ufshcd_fixup_dev_quirks(hba, ufs_mtk_dev_fixups);
>>
>> - if (ufs_mtk_is_broken_vcc(hba) && hba->vreg_info.vcc &&
>> - (hba->dev_quirks & UFS_DEVICE_QUIRK_DELAY_AFTER_LPM)) {
>> + if (ufs_mtk_is_broken_vcc(hba) && hba->vreg_info.vcc) {
>>
>
> Hi Bao,
>
> Adding a delay is not reasonable if we have decided to
> keep VCC always on.
Hi Peter,
The current Mediatek platform driver applies this quirk to all ufs
vendors which is consistent with what we would like to do in the
Qualcomm platform driver per the vendor's requests.
I do see that, about 5 years ago, Mediatek merged a patch to keep the
device vcc always on, probably to workaround some HW issues. Since this
is a very old patch and the impact of this change on a broken hardware
is minimal, I would like weight the benefit of cleaning up the ufs core
driver by removing the unnecessary quirk
UFS_DEVICE_QUIRK_DELAY_AFTER_LPM vs the inconvenience of a 5ms
(potentially reduce to 2ms) delay impact it may cause on an old broken
HW in the suspend/shutdown path.
I believe removing the UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk in the ufs
core driver as well as all the platform drivers yields positive net
benefits in this case.
Thanks, Bao
>
> Thanks
> Peter
>
>
>> hba->vreg_info.vcc->always_on = true;
>> /*
>> * VCC will be kept always-on thus we don't
>> - * need any delay during regulator operations
>> + * need any delay before putting device's VCC in LPM
>> mode.
>> */
>> - hba->dev_quirks &=
>> ~(UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |
>> - UFS_DEVICE_QUIRK_DELAY_AFTER_LPM);
>> + hba->dev_quirks &=
>> ~UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM;
>>
>
>
>
>
>
Powered by blists - more mailing lists