Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221011021653.27277-1-gaoyankaigeren@gmail.com>
 <DM6PR04MB6575AE77585D584652179089FC239@DM6PR04MB6575.namprd04.prod.outlook.com>
 <CAFN68yVajOkV++gCp-y1+SD5VOKLgUeBhfgskrJZOX5dfGi07A@mail.gmail.com>
 <DM6PR04MB65756E9549E9DCDB4BCF8FF6FC229@DM6PR04MB6575.namprd04.prod.outlook.com>
 <DM6PR04MB657509954BB9AA1C22739928FC229@DM6PR04MB6575.namprd04.prod.outlook.com>
In-Reply-To: <DM6PR04MB657509954BB9AA1C22739928FC229@DM6PR04MB6575.namprd04.prod.outlook.com>
From:   =?UTF-8?B?6auY5Lil5Yev?= <gaoyankaigeren@gmail.com>
Date:   Wed, 12 Oct 2022 16:49:15 +0800
Message-ID: <CAFN68yW=r+vZn0wBDrkHeoNXajcknkQOm+5tZ=dwkg+3eLqGEg@mail.gmail.com>
Subject: Re: [PATCH] ufs: core: Disable auto h8 before ssu
To:     Avri Altman <Avri.Altman@wdc.com>
Cc:     Alim Akhtar <alim.akhtar@samsung.com>,
        Bart Van Assche <bvanassche@acm.org>,
        "James E . J . Bottomley" <jejb@linux.ibm.com>,
        Bean Huo <beanhuo@micron.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Dear Avri

TKS for your suggestion. and adjust timimg in own vop can solve this
problem surely.
but I think it's more of a common patch for all platforms,
Patch can adapt different vendors,and align to
ufshcd_auto_hibern8_enable in __ufshcd_wl_resume.
Not distinguishing pm_op can refer to ufshcd_auto_hibern8_enable in
__ufshcd_wl_resume.

Avri Altman <Avri.Altman@wdc.com> =E4=BA=8E2022=E5=B9=B410=E6=9C=8812=E6=97=
=A5=E5=91=A8=E4=B8=89 16:13=E5=86=99=E9=81=93=EF=BC=9A
>
> > >
> > > Dear Avri
> > >   Unisoc reports resume fail on UFS(micron FS164) during suspend/resu=
me
> > > test.
> > >   We check host inserts auto H8 enter/exit event between SSU sleep
> > > command and H8 enter command in runtime suspend .
> > >   Asfollows: SSU Sleep command --> auto H8 enter --> auto H8 exit -->
> > > H8 enter --> idle 2ms --> VCC off.
> > >   However device AQL FW can=E2=80=99t enter LPM within 2ms after seco=
nd H8
> > enter
> > > command.
> > >   FW already enter LPM after receive auto H8 enter command , Next aut=
o
> > > H8 exit command will trigger FW exit from LPM, it need take over 10ms=
,
> > and
> > > FW can=E2=80=99t enter
> > >   LPM again after second H8 enter command until device complete exit
> > from
> > > LPM. So disable auto h8 before ssu is a reasonable solution to solve =
it.
> > >   Hynix also has similar request.
> > Is this something common to all platforms?
> > If not, and you need your platform to disable h8 before ssu,
> > You can implement it in your own vop - see e.g.
> > commit 9561f58442e4 (scsi: ufs: mediatek: Support vops pre suspend to
> > disable auto-hibern8)
> Maybe to further clarify, I am not saying that your suggestion doesn't ma=
ke sense.
> It's just that you need, IMHO, to make it part of ufshcd_vops_suspend, wh=
ich I think
> Its ufshcd_system_suspend for every platform.
> And you need to get an ack on that from the other platform owners (maybe =
except
> Stanley who is doing it already).
>
> Btw, you are not checking the pm_op so your are disabling auto-h8 for run=
time-suspend
> As well.
>
> And maybe elaborate the commit log with your explanation above - making i=
t more clear.
>
> Thanks,
> Avri
> >
> > Thanks,
> > Avri
> >
> > >
> > > Avri Altman <Avri.Altman@wdc.com> =E4=BA=8E2022=E5=B9=B410=E6=9C=8811=
=E6=97=A5=E5=91=A8=E4=BA=8C 15:07=E5=86=99=E9=81=93
> > =EF=BC=9A
> > > >
> > > > > From: Ten Gao <ten.gao@unisoc.com>
> > > > >
> > > > > Ensure auto h8 will not hit dme h8,and there won't be two h8 in a
> > > > > row after ssu.
> > > > I don't think the hw should do that.
> > > > Can you please share on which platform/host controller did you obse=
rve
> > > this?
> > > >
> > > > Thanks,
> > > > Avri
> > > > >
> > > > > Signed-off-by: Ten Gao <ten.gao@unisoc.com>
> > > > > ---
> > > > >  drivers/ufs/core/ufshcd.c | 10 ++++++++++
> > > > >  1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.=
c
> > > > > index
> > > > > a202d7d5240d..42f93648d796 100644
> > > > > --- a/drivers/ufs/core/ufshcd.c
> > > > > +++ b/drivers/ufs/core/ufshcd.c
> > > > > @@ -4256,6 +4256,14 @@ void ufshcd_auto_hibern8_update(struct
> > > > > ufs_hba *hba, u32 ahit)  }
> > > > > EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
> > > > >
> > > > > +void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) {
> > > > > +       if (!ufshcd_is_auto_hibern8_supported(hba))
> > > > > +               return;
> > > > > +
> > > > > +       ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER); }
> > > > > +
> > > > >  void ufshcd_auto_hibern8_enable(struct ufs_hba *hba)  {
> > > > >         if (!ufshcd_is_auto_hibern8_supported(hba))
> > > > > @@ -9036,6 +9044,8 @@ static int __ufshcd_wl_suspend(struct
> > ufs_hba
> > > > > *hba, enum ufs_pm_op pm_op)
> > > > >         if (ret)
> > > > >                 goto enable_scaling;
> > > > >
> > > > > +       ufshcd_auto_hibern8_disable(hba);
> > > > > +
> > > > >         if (req_dev_pwr_mode !=3D hba->curr_dev_pwr_mode) {
> > > > >                 if (pm_op !=3D UFS_RUNTIME_PM)
> > > > >                         /* ensure that bkops is disabled */
> > > > > --
> > > > > 2.17.1
> > > >