Received-SPF: pass (google.com: domain of linux-kernel+bounces-171723-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 7 May 2024 17:52:15 +0200
From: Johan Hovold <johan@kernel.org>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Johan Hovold <johan+linaro@kernel.org>, Lee Jones <lee@kernel.org>,
	Mark Brown <broonie@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Das Srinagesh <quic_gurus@quicinc.com>,
	Satya Priya <quic_c_skakit@quicinc.com>,
	Stephen Boyd <swboyd@chromium.org>, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org
Subject: Re: [PATCH 12/13] regulator: add pm8008 pmic regulator driver
Message-ID: <ZjpOLzG8vSwuDk5k@hovoldconsulting.com>
References: <20240506150830.23709-1-johan+linaro@kernel.org>
 <20240506150830.23709-13-johan+linaro@kernel.org>
 <6dc632b0-792c-49c8-9f66-43f7a14789cc@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6dc632b0-792c-49c8-9f66-43f7a14789cc@linaro.org>

On Tue, May 07, 2024 at 01:48:30PM +0200, Konrad Dybcio wrote:
> On 5/6/24 17:08, Johan Hovold wrote:
> > From: Satya Priya <quic_c_skakit@quicinc.com>
> > 
> > Qualcomm Technologies, Inc. PM8008 is an I2C-controlled PMIC containing
> > seven LDO regulators. Add a PM8008 regulator driver to support PMIC
> > regulator management via the regulator framework.
> > 
> > Note that this driver, originally submitted by Satya Priya [1], has been
> > reworked to match the new devicetree binding which no longer describes
> > each regulator as a separate device.
> > 
> > This avoids describing internal details like register offsets in the
> > devicetree and allows for extending the implementation with features
> > like over-current protection without having to update the binding.
> > 
> > Specifically note that the regulator interrupts are shared between all
> > regulators.
> > 
> > Note that the secondary regmap is looked up by name and that if the
> > driver ever needs to be generalised to support regulators provided by
> > the primary regmap (I2C address) such information could be added to a
> > driver lookup table matching on the parent compatible.
> > 
> > This also fixes the original implementation, which looked up regulators
> > by 'regulator-name' property rather than devicetree node name and which
> > prevented the regulators from being named to match board schematics.
> > 
> > [1] https://lore.kernel.org/r/1655200111-18357-8-git-send-email-quic_c_skakit@quicinc.com
> > 
> > Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> > Cc: Stephen Boyd <swboyd@chromium.org>
> > [ johan: rework probe to match new binding, amend commit message and
> >           Kconfig entry]
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> 
> I'm a bit lukewarm on calling this qcom-pm8008-regulator.. But then
> qcom-i2c-regulator or qpnp-i2c-regulator may bite due to being overly
> generic.. Would you know whether this code will also be used for e.g.
> PM8010?

Yes, for any sufficiently similar PMICs, including SPMI ones. So
'qpnp-regulator' would be a generic name, but only Qualcomm knows what
PMICs they have and how they are related -- the rest of us is left doing
tedious code forensics to try to make some sense of this.

So just like for compatible strings, letting the first supported PMIC
name the driver makes sense as we don't know when we'll want to add a
second one for another set of devices (and we don't want to call that
one 'qpnp-regulator-2'). On the other hand, these names are now mostly
internal and can more easily be renamed later.

Johan