From: ChiYuan Huang <u0084500@gmail.com>
To: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Lee Jones <lee.jones@linaro.org>,
dmitry.torokhov@gmail.com, Liam Girdwood <lgirdwood@gmail.com>,
cy_huang <cy_huang@richtek.com>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>,
linux-input@vger.kernel.org
Subject: Re: [PATCH 3/4] regulator: rt5120: Add PMIC regulator support
Date: Thu, 9 Jun 2022 14:35:07 +0800 [thread overview]
Message-ID: <CADiBU390XRXZ2yx5CT2NxhN3aROHXcxs7w2d-xhB6+EYn+uTfA@mail.gmail.com> (raw)
In-Reply-To: <YqB19O/HYvEAxdiM@sirena.org.uk>
Mark Brown <broonie@kernel.org> 於 2022年6月8日 週三 下午6:12寫道:
>
> On Wed, Jun 08, 2022 at 11:15:56AM +0800, ChiYuan Huang wrote:
> > Mark Brown <broonie@kernel.org> 於 2022年6月8日 週三 上午3:00寫道:
> > > On Tue, Jun 07, 2022 at 01:52:40PM +0800, cy_huang wrote:
>
> > > > + static const char * const name[] = { "buck1", "buck2", "buck3", "buck4",
> > > > + "ldo", "exten" };
> > > > + static const char * const sname[] = { "vin1", "vin2", "vin3", "vin4",
> > > > + "vinldo", NULL };
>
> > > It would be easier and clearer to just make this a static table like
> > > other drivers do, there's no need to generate anything dynamically as
> > > far as I can see.
>
> > My excuse. let me explain it.
> > buck1 voltage range from 600mV to 1393.75mV.
> > buck2~4/ldo/exten is the fixed regulator.
> > buck3 and buck4 is fixed by the IC efuse default.
> > buck2 and ldo is fixed by the external resistor chosen.
> > exten is designed to connected to the external power.
>
> > That's why I cannot directly declared it as the static regulator_desc.
>
> So buck 2-4 need some dynamic handling then but the rest can be static -
> that would be a lot clearer. You could also have a template for the
> ones with some dynamic values and just override the few fields that need
> it.
>
Not just buck2/3, buck2/3/4/ldo/exten all need the dynamic handling.
> > > > + if (init_data->constraints.min_uV != init_data->constraints.max_uV) {
> > > > + dev_err(priv->dev, "Variable voltage for fixed regulator\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + desc->fixed_uV = init_data->constraints.min_uV;
> > > > + init_data->constraints.apply_uV = 0;
>
> > > Drivers should never override constraints passed in by machine drivers,
> > > if there's validation needed let the core do it. The same probably
> > > applies to providing a voltage range for a fixed regulator though that's
> > > not modifying everything so not such a problem.
>
> > Please check the above explanation about each power rails.
>
> I'm not sure what you're referencing here?
>
Sorry. Let me explain it.
You mean 'of_parse_cb' must not override constraint.
But if the regulator is fixed and dynamic, after
'of_get_regulation_constraint', apply_uV will be true.
The is referring to 'fixed.c'
> > > > + for (i = 0; i < RT5120_MAX_REGULATOR; i++) {
> > > > + ret = rt5120_of_parse_cb(priv, i, rt5120_regu_match + i);
> > > > + if (ret) {
> > > > + dev_err(priv->dev, "Failed in [%d] of_passe_cb\n", i);
> > > > + return ret;
> > > > + }
> > > > + }
> > >
> > > This is all open coding stuff that's in the core - just provde an
> > > of_parse_cb() operation and let the core take care of calling it.
>
> > Ditto
>
> Or here.
If I put 'of_parce_cb' to make core handling it, the input parameter
'init_data' is declared as const.
I cannot override the 'apply_uV'.
Right?
next prev parent reply other threads:[~2022-06-09 6:35 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-07 5:52 [PATCH 0/4] Add Richtek RT5120 PMIC support cy_huang
2022-06-07 5:52 ` [PATCH 1/4] dt-binding: mfd: " cy_huang
2022-06-07 11:52 ` Krzysztof Kozlowski
2022-06-08 2:52 ` ChiYuan Huang
2022-06-08 7:01 ` Krzysztof Kozlowski
2022-06-08 7:25 ` ChiYuan Huang
2022-06-08 7:44 ` Krzysztof Kozlowski
2022-06-07 19:02 ` Mark Brown
2022-06-08 3:04 ` ChiYuan Huang
2022-06-07 5:52 ` [PATCH 2/4] mfd: rt5120: Add Richtek " cy_huang
2022-06-07 5:52 ` [PATCH 3/4] regulator: rt5120: Add PMIC regulator support cy_huang
2022-06-07 19:00 ` Mark Brown
2022-06-08 3:15 ` ChiYuan Huang
2022-06-08 10:12 ` Mark Brown
2022-06-09 6:35 ` ChiYuan Huang [this message]
2022-06-10 11:03 ` Mark Brown
2022-06-13 1:49 ` ChiYuan Huang
2022-06-13 11:10 ` Mark Brown
2022-06-07 5:52 ` [PATCH 4/4] input: misc: rt5120: Add power key support cy_huang
2022-06-08 7:03 ` Krzysztof Kozlowski
2022-06-08 7:05 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CADiBU390XRXZ2yx5CT2NxhN3aROHXcxs7w2d-xhB6+EYn+uTfA@mail.gmail.com \
--to=u0084500@gmail.com \
--cc=broonie@kernel.org \
--cc=cy_huang@richtek.com \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lee.jones@linaro.org \
--cc=lgirdwood@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).