Re: [PATCH 12/13] regulator: add pm8008 pmic regulator driver

[Johan Hovold]

On Tue, May 14, 2024 at 07:13:17PM +0530, Satya Priya Kakitapalli wrote:
> > 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@xxxxxxxxxxx>
> > > > 
> > > > 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.

> > > > [1] https://lore.kernel.org/r/1655200111-18357-8-git-send-email-quic_c_skakit@xxxxxxxxxxx
> > > > 
> > > > Signed-off-by: Satya Priya <quic_c_skakit@xxxxxxxxxxx>
> 
> This is my old email which is discontinued, please use <quic_skakitap@xxxxxxxxxxx>

I've cleaned up and reworked the driver for v2 and changed the
authorship to myself in the process, but I'll make sure to CC your new
address when submitting.

You should add an alias as Konrad suggested as you apparently have
commits that use your old address.

> > > > Cc: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> > > > [ johan: rework probe to match new binding, amend commit message and
> > > >           Kconfig entry]
> > > > Signed-off-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
> > > > ---
> > > 
> > > 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.
> 
> There is a PMIC called PM8010 which also is implemented over I2C,
> which could use the same pm8008 regulator driver.
> Hence it is better to use device_id instead of a MODULE_ALIAS().

Right, I've had PM8010 in mind all along, but it's hard to tell whether
it will be using the same (sub)driver until code is submitted since you
guys (Qualcomm) don't publish any documentation.

I've changed the regulator (and GPIO) drivers to use platform device id
matching for now either way.

Johan