[PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd failed

[PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd failed

[Hongjie Fang]

The vendor hibern8 notify callback always can be executed in the
PRE_CHANGE phase of hibern8 enter/exit. But it cannot be executed
in the POST_CHANGE phase if the hibern8 cmd fails.

When the hibern8 cmd fails, the vendor hibern8 notify callback
should still have the opportunity to execute.

Add a third enum ROLLBACK_CHANGE for the ufshcd_vops_hibern8_notify(),
pass the ROLLBACK_CHANGE when the hibern8 command returns a failure and
use the POST_CHANGE otherwise.

Signed-off-by: Hongjie Fang <hongjiefang@asrmicro.com>
---
 drivers/ufs/core/ufshcd.c     | 11 ++++++-----
 drivers/ufs/host/ufs-exynos.c |  6 ++++++
 drivers/ufs/host/ufs-qcom.c   |  4 +++-
 include/ufs/ufshcd.h          |  1 +
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 9ceb6d6d479d..1d4939bb7ec0 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4500,9 +4500,9 @@ int ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
 	if (ret)
 		dev_err(hba->dev, "%s: hibern8 enter failed. ret = %d\n",
 			__func__, ret);
-	else
-		ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER,
-								POST_CHANGE);
+
+	ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER,
+					ret ? ROLLBACK_CHANGE : POST_CHANGE);
 
 	return ret;
 }
@@ -4526,12 +4526,13 @@ int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
 		dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n",
 			__func__, ret);
 	} else {
-		ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT,
-								POST_CHANGE);
 		hba->ufs_stats.last_hibern8_exit_tstamp = local_clock();
 		hba->ufs_stats.hibern8_exit_cnt++;
 	}
 
+	ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT,
+					ret ? ROLLBACK_CHANGE : POST_CHANGE);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(ufshcd_uic_hibern8_exit);
diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
index 76fee3a79c77..7ada4e96f236 100644
--- a/drivers/ufs/host/ufs-exynos.c
+++ b/drivers/ufs/host/ufs-exynos.c
@@ -1654,6 +1654,8 @@ static int exynos_ufs_hce_enable_notify(struct ufs_hba *hba,
 		if (ufs->drv_data->post_hce_enable)
 			ret = ufs->drv_data->post_hce_enable(ufs);
 
+		break;
+	default:
 		break;
 	}
 
@@ -1672,6 +1674,8 @@ static int exynos_ufs_link_startup_notify(struct ufs_hba *hba,
 	case POST_CHANGE:
 		ret = exynos_ufs_post_link(hba);
 		break;
+	default:
+		break;
 	}
 
 	return ret;
@@ -1692,6 +1696,8 @@ static int exynos_ufs_pwr_change_notify(struct ufs_hba *hba,
 	case POST_CHANGE:
 		ret = exynos_ufs_post_pwr_mode(hba, dev_req_params);
 		break;
+	default:
+		break;
 	}
 
 	return ret;
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 375fd24ba458..4f1caf8dabbf 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1234,7 +1234,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
 	struct phy *phy;
-	int err;
+	int err = 0;
 
 	/*
 	 * In case ufs_qcom_init() is not yet done, simply ignore.
@@ -1289,6 +1289,8 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 					    ufs_qcom_bw_table[MODE_MIN][0][0].cfg_bw);
 		}
 		break;
+	default:
+		break;
 	}
 
 	return 0;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 8563b6648976..4f7c619db324 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -270,6 +270,7 @@ struct ufs_clk_info {
 enum ufs_notify_change_status {
 	PRE_CHANGE,
 	POST_CHANGE,
+	ROLLBACK_CHANGE,
 };
 
 struct ufs_pa_layer_attr {
-- 
2.25.1

Re: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd failed

[Bart Van Assche]

On 5/2/26 4:30 PM, Hongjie Fang wrote:
> The vendor hibern8 notify callback always can be executed in the
> PRE_CHANGE phase of hibern8 enter/exit. But it cannot be executed
> in the POST_CHANGE phase if the hibern8 cmd fails.
> 
> When the hibern8 cmd fails, the vendor hibern8 notify callback
> should still have the opportunity to execute.
> 
> Add a third enum ROLLBACK_CHANGE for the ufshcd_vops_hibern8_notify(),
> pass the ROLLBACK_CHANGE when the hibern8 command returns a failure and
> use the POST_CHANGE otherwise.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd failed

[Bean Huo]

On Sat, 2026-05-02 at 22:30 +0800, Hongjie Fang wrote:
> +       default:
> +               break;
>         }
>  
>         return 0;
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 8563b6648976..4f7c619db324 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -270,6 +270,7 @@ struct ufs_clk_info {
>  enum ufs_notify_change_status {
>         PRE_CHANGE,
>         POST_CHANGE,
> +       ROLLBACK_CHANGE,
>  };
>  
>  struct ufs_pa_layer_attr {
> --
> 2.25.1

Could you include the platform driver that actually handles ROLLBACK_CHANGE in
this series? Adding  this new rollback_change without an usage makes it hard to
verify the design is correct. 

Kind regards,
Bean

RE: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd failed

[Fang Hongjie(方洪杰)]

> From: Bean Huo [mailto:beanhuo@iokpp.de]
> Sent: Tuesday, May 5, 2026 2:41 PM
> To: Fang Hongjie <hongjiefang@asrmicro.com>;
> alim.akhtar@samsung.com; avri.altman@wdc.com; bvanassche@acm.org;
> James.Bottomley@HansenPartnership.com; martin.petersen@oracle.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd
> failed
> 
> On Sat, 2026-05-02 at 22:30 +0800, Hongjie Fang wrote:
> > +       default:
> > +               break;
> >         }
> >
> >         return 0;
> > diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> > index 8563b6648976..4f7c619db324 100644
> > --- a/include/ufs/ufshcd.h
> > +++ b/include/ufs/ufshcd.h
> > @@ -270,6 +270,7 @@ struct ufs_clk_info {
> >  enum ufs_notify_change_status {
> >         PRE_CHANGE,
> >         POST_CHANGE,
> > +       ROLLBACK_CHANGE,
> >  };
> >
> >  struct ufs_pa_layer_attr {
> > --
> > 2.25.1
> 
> Could you include the platform driver that actually handles
> ROLLBACK_CHANGE in
> this series? Adding  this new rollback_change without an usage makes it hard
> to  verify the design is correct.
> 

The platform driver code is still under development, and we plan to submit it
in the future. The purpose of this patch is to first provide a mechanism that
allows vendor callbacks perform relevant rollback when hibern8 fails.

> Kind regards,
> Bean

Best.

Re: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd failed

[Bean Huo]

On Wed, 2026-05-06 at 03:29 +0000, Fang Hongjie(方洪杰) wrote:
> 
> > From: Bean Huo [mailto:beanhuo@iokpp.de]
> > Sent: Tuesday, May 5, 2026 2:41 PM
> > To: Fang Hongjie <hongjiefang@asrmicro.com>;
> > alim.akhtar@samsung.com; avri.altman@wdc.com; bvanassche@acm.org;
> > James.Bottomley@HansenPartnership.com; martin.petersen@oracle.com
> > Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8
> > cmd
> > failed
> > 
> > On Sat, 2026-05-02 at 22:30 +0800, Hongjie Fang wrote:
> > > +       default:
> > > +               break;
> > >         }
> > > 
> > >         return 0;
> > > diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> > > index 8563b6648976..4f7c619db324 100644
> > > --- a/include/ufs/ufshcd.h
> > > +++ b/include/ufs/ufshcd.h
> > > @@ -270,6 +270,7 @@ struct ufs_clk_info {
> > >  enum ufs_notify_change_status {
> > >         PRE_CHANGE,
> > >         POST_CHANGE,
> > > +       ROLLBACK_CHANGE,
> > >  };
> > > 
> > >  struct ufs_pa_layer_attr {
> > > --
> > > 2.25.1
> > 
> > Could you include the platform driver that actually handles
> > ROLLBACK_CHANGE in
> > this series? Adding  this new rollback_change without an usage makes it hard
> > to  verify the design is correct.
> > 
> 
> The platform driver code is still under development, and we plan to submit it
> in the future. The purpose of this patch is to first provide a mechanism that
> allows vendor callbacks perform relevant rollback when hibern8 fails.
> 
> > Kind regards,
> > Bean
> 
> Best.
> 

Thanks for the explanation. However, the kernel development practice is to not
merge infrastructure without at least one in-tree user. Please resubmit this
patch together with your platform driver (or at least the hibern8_notify
callback that handles ROLLBACK_CHANGE) so reviewers can verify the design is
correct and actually works as intended.

@Bart, any idea?

Kind Regards,
Bean

Re: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd failed

[Bart Van Assche]

On 5/6/26 10:47 AM, Bean Huo wrote:
> Thanks for the explanation. However, the kernel development practice is to not
> merge infrastructure without at least one in-tree user. Please resubmit this
> patch together with your platform driver (or at least the hibern8_notify
> callback that handles ROLLBACK_CHANGE) so reviewers can verify the design is
> correct and actually works as intended.
> 
> @Bart, any idea?

Everyone who works on Android smartphones has to deal with the following:
- The upstream-first policy.
- Not disclosing any aspect of the phone under development until it has
   been announced publicly.

Typically two years elapse between the start of testing kernel code for
a new phone and public announcement. Another 1 - 4 years elapse after
a phone has been announced until all kernel code for a smartphone is
upstream. Insisting on not merging any code upstream until a user for
the code is upstream makes the job of smartphone kernel developers
harder than necessary.

This is why I'm fine with deviating from the rule explained in your
email for small changes.

Thanks,

Bart.

Re: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd failed

[Bean Huo]

On Wed, 2026-05-06 at 11:16 +0200, Bart Van Assche wrote:
> On 5/6/26 10:47 AM, Bean Huo wrote:
> > Thanks for the explanation. However, the kernel development practice is to
> > not
> > merge infrastructure without at least one in-tree user. Please resubmit this
> > patch together with your platform driver (or at least the hibern8_notify
> > callback that handles ROLLBACK_CHANGE) so reviewers can verify the design is
> > correct and actually works as intended.
> > 
> > @Bart, any idea?
> 
> Everyone who works on Android smartphones has to deal with the following:
> - The upstream-first policy.
> - Not disclosing any aspect of the phone under development until it has
>    been announced publicly.
> 
> Typically two years elapse between the start of testing kernel code for
> a new phone and public announcement. Another 1 - 4 years elapse after
> a phone has been announced until all kernel code for a smartphone is
> upstream. Insisting on not merging any code upstream until a user for
> the code is upstream makes the job of smartphone kernel developers
> harder than necessary.
> 
> This is why I'm fine with deviating from the rule explained in your
> email for small changes.
> 
> Thanks,
> 
> Bart.


Thanks Bart.


@Fang Hongjie, 

I respect the practical constraints Bart described, but my concern is about
design validation, not disclosure. Could you at least show a minimal
hibern8_notify handler that uses ROLLBACK_CHANGE? 

It doesn't need to be the full platform driver, just enough to demonstrate what
"rollback" means concretely (e.g., undo clock gating, restore PHY state, etc.).
Without that, we're merging an API whose semantics are unverifiable.

Especially, other vendors (SS, Qcom, MTK, etc.) should be able to see the
direction and understand whether they also need to handle ROLLBACK_CHANGE in
their own hibern8_notify callbacks.

Otherwise, please include this patch as part of your platform driver patch
series when you submit it, so reviewers can evaluate the infrastructure and its
consumer together.

Kind Regards,
Bean