On Wed 05 Jun 23:36 PDT 2019, Avri Altman wrote:
>
> > static int ufshcd_hba_init(struct ufs_hba *hba)
> > {
> > int err;
> > @@ -7425,9 +7460,15 @@ static int ufshcd_hba_init(struct ufs_hba *hba)
> > if (err)
> > goto out_disable_vreg;
> >
> > + err = ufshcd_init_device_reset(hba);
> > + if (err)
> > + goto out_disable_variant;
> > +
> > hba->is_powered = true;
> > goto out;
> >
> > +out_disable_variant:
> > + ufshcd_vops_setup_regulators(hba, false);
> Is this necessary?
> ufshcd_vops_setup_regulators() was just called as part of
> ufshcd_variant_hba_init
>
Yes, so my attempt here is to reverse the enablement of the vops
regulators (hence passing false). But looking at it again I see that we
should also do ufshcd_vops_exit(), so the right thing to call here is
ufshcd_variant_hba_exit().
PS. This initialization sequence should really be rewritten to first
acquire all resources and then turn them on. This mixes init/setup
sequence is really hard to reason about.
Regards,
Bjorn
