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

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

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

[Fang Hongjie(方洪杰)]

> From: Bean Huo [mailto:beanhuo@iokpp.de]
> Sent: Wednesday, May 6, 2026 8:44 PM
> To: Bart Van Assche <bvanassche@acm.org>; Fang Hongjie(方洪杰)
> <hongjiefang@asrmicro.com>; alim.akhtar@samsung.com;
> avri.altman@wdc.com; James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
> can.guo@oss.qualcomm.com
> Subject: Re: [PATCH v5] scsi: ufs: core: call hibern8 notify when hibern8 cmd
> failed
> 
> 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.
> 

We have discovered that on our new platform, the UIC error interrupt must be
temporarily disabled when entering or exiting hibern8; otherwise,
it will interfere with the execution of the hibern8 command itself.
Of course, this is merely a specific corner-case limitation in the controller
design and has no impact on data transmission. Due to this limitation, 
the UIC error must be re-enabled regardless of whether the hibern8 command
executes successfully, as failure to do so will affect the monitoring of the
actual UIC errors during subsequent data transmission.


> Kind Regards,
> Bean


Best.