Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 22 Aug 2022 15:04:33 -0500
From:   Andrew Halaney <ahalaney@redhat.com>
To:     Douglas Anderson <dianders@chromium.org>
Cc:     Mark Brown <broonie@kernel.org>, Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: qcom-rpmh: Implement get_optimum_mode(), not
 set_load()
Message-ID: <20220822200433.k5ximhpufcxom2kv@halaneylaptop>
References: <20220726102024.1.Icc838fe7bf0ef54a014ab2fee8af311654f5342a@changeid>
 <20220822193153.zn2oxljmd76awqot@halaneylaptop>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220822193153.zn2oxljmd76awqot@halaneylaptop>
Precedence: bulk

On Mon, Aug 22, 2022 at 02:31:53PM -0500, Andrew Halaney wrote:
> Hey Douglas,
> 
> On Tue, Jul 26, 2022 at 10:20:29AM -0700, Douglas Anderson wrote:
> > Since we don't actually pass the load to the firmware, switch to using
> > get_optimum_mode() instead of open-coding it.
> > 
> > This is intended to have no effect other than cleanup.
> 
> I hate to post something without looking into it very deeply, but with
> this statement about no effect I figured I'd report what I'm noticing
> before diving deeper.
> 
> I'm currently playing with the dts patches in [0], and without this
> patch (and a hack patch applied that is unrelated to this) the board boots
> fine. With this patch applied I get the messages reported in [1] (which I
> think is still a clean up that should be applied) and the board fails to
> boot due to regulator_enable() failing at the first invocation I see.

Sorry for more spam, but just to be clear, in the above I'm saying that
if I revert this patch the error message below goes away and the
regulator_enable() call succeeds allowing the board to boot. Re-reading
what I wrote it sounds like a jumbled mess.

> 
> Is that something you expect?
> 
> Here's the ultimate failure message for regulator_enable():
> 
>     [    1.213419] ufshcd-qcom 1d84000.ufs: Adding to iommu group 0
>     [    1.219492] ufshcd-qcom 1d84000.ufs: ufshcd_populate_vreg: Unable to find vdd-hba-supply regulator, assuming enabled
>     [    1.230287] ufshcd-qcom 1d84000.ufs: ufshcd_populate_vreg: Unable to find vccq2-supply regulator, assuming enabled
>     [    1.241079] vreg_l17c: invalid input voltage found
>     [    1.246002] ufshcd-qcom 1d84000.ufs: ufshcd_enable_vreg: vcc enable failed, err=-22
>     [    1.253952] ufshcd-qcom 1d84000.ufs: Initialization failed
>     [    1.259622] ufshcd-qcom 1d84000.ufs: ufshcd_pltfrm_init() failed -22
>     [    1.266151] ufshcd-qcom: probe of 1d84000.ufs failed with error -22
> 
> [0] https://lore.kernel.org/all/20220722143711.17563-1-quic_ppareek@quicinc.com/
> [1] https://lore.kernel.org/all/166118500944.215002.10320899094541954077.b4-ty@kernel.org/
> 
> Thanks,
> Andrew
> 
> > 
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > 
> >  drivers/regulator/qcom-rpmh-regulator.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
> > index 561de6b2e6e3..b2debde79361 100644
> > --- a/drivers/regulator/qcom-rpmh-regulator.c
> > +++ b/drivers/regulator/qcom-rpmh-regulator.c
> > @@ -306,9 +306,10 @@ static unsigned int rpmh_regulator_vrm_get_mode(struct regulator_dev *rdev)
> >  }
> >  
> >  /**
> > - * rpmh_regulator_vrm_set_load() - set the regulator mode based upon the load
> > - *		current requested
> > + * rpmh_regulator_vrm_get_optimum_mode() - get the mode based on the  load
> >   * @rdev:		Regulator device pointer for the rpmh-regulator
> > + * @input_uV:		Input voltage
> > + * @output_uV:		Output voltage
> >   * @load_uA:		Aggregated load current in microamps
> >   *
> >   * This function is used in the regulator_ops for VRM type RPMh regulator
> > @@ -316,17 +317,15 @@ static unsigned int rpmh_regulator_vrm_get_mode(struct regulator_dev *rdev)
> >   *
> >   * Return: 0 on success, errno on failure
> >   */
> > -static int rpmh_regulator_vrm_set_load(struct regulator_dev *rdev, int load_uA)
> > +static unsigned int rpmh_regulator_vrm_get_optimum_mode(
> > +	struct regulator_dev *rdev, int input_uV, int output_uV, int load_uA)
> >  {
> >  	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
> > -	unsigned int mode;
> >  
> >  	if (load_uA >= vreg->hw_data->hpm_min_load_uA)
> > -		mode = REGULATOR_MODE_NORMAL;
> > +		return REGULATOR_MODE_NORMAL;
> >  	else
> > -		mode = REGULATOR_MODE_IDLE;
> > -
> > -	return rpmh_regulator_vrm_set_mode(rdev, mode);
> > +		return REGULATOR_MODE_IDLE;
> >  }
> >  
> >  static int rpmh_regulator_vrm_set_bypass(struct regulator_dev *rdev,
> > @@ -375,7 +374,7 @@ static const struct regulator_ops rpmh_regulator_vrm_drms_ops = {
> >  	.list_voltage		= regulator_list_voltage_linear_range,
> >  	.set_mode		= rpmh_regulator_vrm_set_mode,
> >  	.get_mode		= rpmh_regulator_vrm_get_mode,
> > -	.set_load		= rpmh_regulator_vrm_set_load,
> > +	.get_optimum_mode	= rpmh_regulator_vrm_get_optimum_mode,
> >  };
> >  
> >  static const struct regulator_ops rpmh_regulator_vrm_bypass_ops = {
> > -- 
> > 2.37.1.359.gd136c6c3e2-goog
> >