From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bjorn Andersson Subject: Re: [PATCH v2 2/3] scsi: ufs: Allow resetting the UFS device Date: Thu, 6 Jun 2019 00:18:28 -0700 Message-ID: <20190606071828.GS22737@tuxbook-pro> References: <20190606010249.3538-1-bjorn.andersson@linaro.org> <20190606010249.3538-3-bjorn.andersson@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Avri Altman Cc: Rob Herring , Mark Rutland , Alim Akhtar , Pedro Sousa , "James E.J. Bottomley" , "Martin K. Petersen" , Andy Gross , Linus Walleij , Evan Green , "linux-arm-msm@vger.kernel.org" , "linux-gpio@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-scsi@vger.kernel.org" List-Id: devicetree@vger.kernel.org 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