Re: [PATCH v7 17/23] scsi: ufs: mediatek: Rework ufs_mtk_wait_idle_state

[AngeloGioacchino Del Regno]

Il 25/02/26 11:32, Peter Wang (王信友) ha scritto:
> On Mon, 2026-02-16 at 14:37 +0100, Nicolas Frattaroli wrote:
> > While ufs_mtk_wait_idle state has some code smells for me (the
> > VS_HCE_BASE early exit seems racey at best), it can still benefit
> > from
> > some general cleanup to make the code flow less convoluted.
> >
> > Use the iopoll helpers, for one, and specifically the one that sleeps
> > and does not busy delay, as it's being done for up to 5ms.
> >
> > The register read is split out to a helper function that branches
> > between new and old style flow.
> >
> > Every called uses the same 5ms timeout value, so there is no point in
> > making this a parameter. Just assume a 5ms timeout in the function.
> >
> > Reviewed-by: AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@xxxxxxxxxxxxx>
>
> Hi Nicolas,
>
> The logic is quite different.
>
> Previously, the check was:
> If (sm >= VS_HIB_ENTER) and (sm <= VS_HIB_EXIT), then wait for sm to
> reach VS_HCE_BASE.
> If not ((val < VS_HIB_ENTER) and (val > VS_HIB_EXIT)), then exit the
> loop directly.
>
> Now, the logic is:
> If sm == VS_HCE_BASE, return 0.
> If (sm >= VS_HIB_ENTER) and (sm <= VS_HIB_EXIT), then wait for sm to
> transition to any other state and exit the poll.

The logic is practically the same, except it's proper now.

There's no need to wait for `sm = VS_HCE_BASE` if `sm == VS_HCE_BASE`.

In other words, just read the comment that Nicolas has put in the code:
/* If the device is already in the base state after 10us, don't wait. */

...because there's no need to wait for the device to get to BASE state,
if the device is *already* in BASE state. It's pretty obvious stuff here.

Regards,
Angelo

> Thanks
> Peter