Received-SPF: pass (google.com: domain of linux-kernel+bounces-178762-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
To: <johan@kernel.org>
CC: <andersson@kernel.org>, <andy.shevchenko@gmail.com>, <broonie@kernel.org>,
        <conor+dt@kernel.org>, <devicetree@vger.kernel.org>,
        <johan+linaro@kernel.org>, <konrad.dybcio@linaro.org>,
        <krzk+dt@kernel.org>, <lee@kernel.org>, <lgirdwood@gmail.com>,
        <linus.walleij@linaro.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-gpio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <quic_c_skakit@quicinc.com>, <quic_gurus@quicinc.com>,
        <robh@kernel.org>, <swboyd@chromium.org>
Subject: Re: [PATCH 12/13] regulator: add pm8008 pmic regulator driver
Date: Tue, 14 May 2024 19:13:17 +0530
Message-ID: <20240514134317.691887-1-quic_skakitap@quicinc.com>
In-Reply-To: <ZjpMeVk_HiixZUEu@hovoldconsulting.com>
References: <ZjpMeVk_HiixZUEu@hovoldconsulting.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain

> On Tue, May 07, 2024 at 01:48:30PM +0200, Konrad Dybcio wrote:=0D
> > On 5/6/24 17:08, Johan Hovold wrote:=0D
> > > From: Satya Priya <quic_c_skakit@quicinc.com>=0D
> > > =0D
> > > Qualcomm Technologies, Inc. PM8008 is an I2C-controlled PMIC containi=
ng=0D
> > > seven LDO regulators. Add a PM8008 regulator driver to support PMIC=0D
> > > regulator management via the regulator framework.=0D
> > > =0D
> > > Note that this driver, originally submitted by Satya Priya [1], has b=
een=0D
> > > reworked to match the new devicetree binding which no longer describe=
s=0D
> > > each regulator as a separate device.=0D
> > > =0D
> > > This avoids describing internal details like register offsets in the=
=0D
> > > devicetree and allows for extending the implementation with features=
=0D
> > > like over-current protection without having to update the binding.=0D
> > > =0D
> > > Specifically note that the regulator interrupts are shared between al=
l=0D
> > > regulators.=0D
> > > =0D
> > > Note that the secondary regmap is looked up by name and that if the=0D
> > > driver ever needs to be generalised to support regulators provided by=
=0D
> > > the primary regmap (I2C address) such information could be added to a=
=0D
> > > driver lookup table matching on the parent compatible.=0D
> > > =0D
> > > This also fixes the original implementation, which looked up regulato=
rs=0D
> > > by 'regulator-name' property rather than devicetree node name and whi=
ch=0D
> > > prevented the regulators from being named to match board schematics.=
=0D
> > > =0D
> > > [1] https://lore.kernel.org/r/1655200111-18357-8-git-send-email-quic_=
c_skakit@quicinc.com=0D
> > > =0D
> > > Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>=0D
=0D
This is my old email which is discontinued, please use <quic_skakitap@quici=
nc.com>=0D
=0D
> > > Cc: Stephen Boyd <swboyd@chromium.org>=0D
> > > [ johan: rework probe to match new binding, amend commit message and=
=0D
> > >           Kconfig entry]=0D
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>=0D
> > > ---=0D
> > =0D
> > I'm a bit lukewarm on calling this qcom-pm8008-regulator.. But then=0D
> > qcom-i2c-regulator or qpnp-i2c-regulator may bite due to being overly=0D
> > generic.. Would you know whether this code will also be used for e.g.=0D
> > PM8010?=0D
> =0D
> Yes, for any sufficiently similar PMICs, including SPMI ones. So=0D
> 'qpnp-regulator' would be a generic name, but only Qualcomm knows what=0D
> PMICs they have and how they are related -- the rest of us is left doing=
=0D
> tedious code forensics to try to make some sense of this.=0D
> =0D
> So just like for compatible strings, letting the first supported PMIC=0D
> name the driver makes sense as we don't know when we'll want to add a=0D
> second one for another set of devices (and we don't want to call that=0D
> one 'qpnp-regulator-2'). On the other hand, these names are now mostly=0D
> internal and can more easily be renamed later.=0D
=0D
There is a PMIC called PM8010 which also is implemented over I2C, which cou=
ld use the same pm8008 regulator driver.=0D
Hence it is better to use device_id instead of a MODULE_ALIAS().=