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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YqcbDfceSusNea6v@sirena.org.uk>
Date:   Mon, 13 Jun 2022 12:10:05 +0100
From:   Mark Brown <broonie@...nel.org>
To:     ChiYuan Huang <u0084500@...il.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Lee Jones <lee.jones@...aro.org>, dmitry.torokhov@...il.com,
        Liam Girdwood <lgirdwood@...il.com>,
        cy_huang <cy_huang@...htek.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, lkml <linux-kernel@...r.kernel.org>,
        linux-input@...r.kernel.org
Subject: Re: [PATCH 3/4] regulator: rt5120: Add PMIC regulator support

On Mon, Jun 13, 2022 at 09:49:31AM +0800, ChiYuan Huang wrote:
> Mark Brown <broonie@...nel.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).

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)