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;
MIME-Version: 1.0
References: <20220823131629.RFT.1.I137e6bef4f6d517be7b081be926059321102fd3d@changeid>
 <20220823221429.3bte2tgtyniur4wb@halaneylaptop>
In-Reply-To: <20220823221429.3bte2tgtyniur4wb@halaneylaptop>
From:   Doug Anderson <dianders@chromium.org>
Date:   Tue, 23 Aug 2022 15:32:44 -0700
Message-ID: <CAD=FV=WSSrObW2PthmUL+t=VYJ+mN6Z763pozCQDbOdPtAXyQA@mail.gmail.com>
Subject: Re: [RFT PATCH] regulator: core: Require regulator drivers to check
 uV for get_optimum_mode()
To:     Andrew Halaney <ahalaney@redhat.com>
Cc:     Mark Brown <broonie@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

On Tue, Aug 23, 2022 at 3:14 PM Andrew Halaney <ahalaney@redhat.com> wrote:
>
> I think this makes sense, but unfortunately it doesn't entirely fix my
> issue introduced by efb0cb50c427 ("regulator: qcom-rpmh: Implement get_optimum_mode(), not set_load()"),
> I'm seeing this now:
>
>     [    1.240757] vreg_l17c: mode operation not allowed
>     [    1.245586] vreg_l17c: failed to get optimum mode @ 800000 uA 0 -> 2504000 uV: -EPERM
>     [    1.253631] ufshcd-qcom 1d84000.ufs: ufshcd_enable_vreg: vcc enable failed, err=-1
>
> which if I understand correctly is because my devicetree isn't setting
> regulator-allowed-modes. It appears most of the qcom one's don't set
> that (and a good number set regulator-allow-set-load, which I think is
> necessary for the RPMH regulator to get this far), so I think others
> will be in the same boat as me.
>
> Just for clarity, I'm running with this dtb right now:
>
>     https://lore.kernel.org/all/20220812165453.11608-4-quic_ppareek@quicinc.com/

Wonderful. I guess the old mechanism that RPMH was using totally
bypassed regulator_mode_constrain(). Seems crazy that nobody has
noticed that till now...

I guess maybe we could add the "initial mode" as an implicitly allowed
mode? I guess the other option is to implicitly disable
regulator-allow-set-load if no allowed modes are listed? Both of these
two will still change behavior for your board, of course. Since I
think previously we were setting modes completely ignoring
"regulator-allowed-modes" both of those two options will effectively
disable mode switching.

I guess the other option would be to say that if
"regulator-allow-set-load" is set but not "regulator-allowed-modes"
then we allow all modes? Somehow that doesn't seem like the "safest"
option, though.

...or, of course, we could say that these device trees are broken and
we're not going to care about supporting them, then we could fix the
device trees...

-Doug