Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200602100924.26256-1-sumit.semwal@linaro.org>
 <20200602100924.26256-5-sumit.semwal@linaro.org> <20200602113241.GE5684@sirena.org.uk>
 <CAO_48GGgNUGosN2PiL=U5JkR3Bh5wNK3N4xYYML1UwmdfDPRww@mail.gmail.com>
 <20200602122554.GG5684@sirena.org.uk> <CAO_48GFwEHBGmz0QvN+pXFSyHC9+7=0aoJLHF4uupGSx2TcSvA@mail.gmail.com>
 <20200617114721.GD4613@sirena.org.uk>
In-Reply-To: <20200617114721.GD4613@sirena.org.uk>
From:   Sumit Semwal <sumit.semwal@linaro.org>
Date:   Wed, 17 Jun 2020 17:27:12 +0530
Message-ID: <CAO_48GF9pKZCCof170TvB0ubOkecDzcGhtUUuY_Td78L1J338A@mail.gmail.com>
Subject: Re: [PATCH v4 4/5] regulator: qcom: Add labibb driver
To:     Mark Brown <broonie@kernel.org>
Cc:     agross@kernel.org, Bjorn Andersson <bjorn.andersson@linaro.org>,
        lgirdwood@gmail.com, robh+dt@kernel.org,
        Nisha Kumari <nishakumari@codeaurora.org>,
        linux-arm-msm@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
        devicetree@vger.kernel.org, kgunda@codeaurora.org,
        Rajendra Nayak <rnayak@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 17 Jun 2020 at 17:17, Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Jun 17, 2020 at 05:12:35PM +0530, Sumit Semwal wrote:
>
> > I understand from a pure regulators' correctness point of view,
> > ENABLE_CTL should be the one checked there, so I can change the patch
> > as you suggested, but there seems to be some performance penalty
> > there.
>
> I thought the goal was to have the performance penalty to ensure that
> the regulator had actually started?
IMHO, with the poll_enabled_time mechanism added, we would not need to
wait for the full enabled_time time for the regulator to get enabled,
but we could poll (and potentially know earlier) if the regulator is
enabled.
The performance penalty I was talking, is about how should we check if
the regulator is really enabled or not - via reading the STATUS1
register, which seems to tell the status a bit faster, or via reading
the ENABLE_CTL register which we also use to enable/disable the
regulator, but which seems to be slower in updating the status.

>
> > > > The WARN_ON? This was suggested by Bjorn to catch the case where the
> > > > DT binding for a PMIC instantiates only one of the regulators.
>
> > > No, this whole loop - why this whole match and get child stuff?
>
> > This loop mechanism is what I saw in the other qcom regulators
> > upstream, so thought it was an acceptable way.
> > For the two children nodes, do you recommend another mechanism to get
> > and validate both nodes?
>
> I don't understand what you mean by "two children nodes" here?
The two 'lab' and 'ibb' regulator nodes that are part of the labibb node.

Best,
Sumit.