Re: [PATCH 3/4] regulator: rt5120: Add PMIC regulator support

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On Mon, Jun 13, 2022 at 09:49:31AM +0800, ChiYuan Huang wrote:
> Mark Brown <broonie@kernel.org> 於 2022年6月10日 週五 下午7:03寫道：

> > > Not just buck2/3, buck2/3/4/ldo/exten all need the dynamic handling.

> > Why do the others need it?

> Sometimes, for this kind of general purpose PMIC, it need to provide
> the flexibility.
> Cause buck2 and ldo can already be fixed by the external resistor,
> buck3 and buck4 seems to be fixed by IC default.
> So there may be the same part number and use the postfix to be
> different like as 5120'A'/5120'B', etc...
> And use it to define the voltage for the different IC default for
> buck3 and buck4, and exten behavior.
> That's due to the different application use the different power on
> sequence and default voltages.l

Variants should have separate compatibles, and if the code is doing
something fixed then it shouldn't make any difference if that fixed
thing is written in code or as a data table.

> > > 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?

> > Yes, that's by design.

> I have traced the code for 'of_get_regulator_init_data' and
> 'set_machine_constraints' in regulator register.
> If I cannot overwrite apply_uV variable, it will cause the
> regulator_register return -EINVAL.

We have a very large number of fixed voltage regulators in use on
various systems which manage perfectly fine without this.  If the
constraints are trying to set something invalid then they're what 
should be fixed.

> Is the below flow that you suggested?
> 1. of_parse_cb to check min_uV and max_uV, and fill in the fixed_uV in
> regulator_desc

No, the device should just know what voltage the fixed voltage
regulators in the device have.

> 2. provide the duummy set/get voltage  to make set_machine_constraints
> not return '-EINVAL'.

No, people just shouldn't be trying to set the voltage for fixed voltage
regulators.  If the regulators are configurable in hardware then provide
DT properties for whatever is configured (eg, resistors).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]