All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Avri Altman <Avri.Altman@wdc.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alim Akhtar <alim.akhtar@samsung.com>,
	Pedro Sousa <pedrom.sousa@synopsys.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Andy Gross <agross@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Evan Green <evgreen@chromium.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH v2 2/3] scsi: ufs: Allow resetting the UFS device
Date: Thu, 6 Jun 2019 00:18:28 -0700	[thread overview]
Message-ID: <20190606071828.GS22737@tuxbook-pro> (raw)
In-Reply-To: <SN6PR04MB4925FC3F1001326AA218DF21FC170@SN6PR04MB4925.namprd04.prod.outlook.com>

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

  reply	other threads:[~2019-06-06  7:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-06  1:02 [PATCH v2 0/3] (Qualcomm) UFS device reset support Bjorn Andersson
2019-06-06  1:02 ` Bjorn Andersson
2019-06-06  1:02 ` [PATCH v2 1/3] pinctrl: qcom: sdm845: Expose ufs_reset as gpio Bjorn Andersson
2019-06-06  1:02 ` [PATCH v2 2/3] scsi: ufs: Allow resetting the UFS device Bjorn Andersson
2019-06-06  6:36   ` Avri Altman
2019-06-06  7:18     ` Bjorn Andersson [this message]
2019-07-09  1:48   ` Rob Herring
2019-07-28 22:09   ` Linus Walleij
2019-06-06  1:02 ` [PATCH v2 3/3] arm64: dts: qcom: sdm845-mtp: Specify UFS device-reset GPIO Bjorn Andersson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190606071828.GS22737@tuxbook-pro \
    --to=bjorn.andersson@linaro.org \
    --cc=Avri.Altman@wdc.com \
    --cc=agross@kernel.org \
    --cc=alim.akhtar@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=evgreen@chromium.org \
    --cc=jejb@linux.ibm.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.petersen@oracle.com \
    --cc=pedrom.sousa@synopsys.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.