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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YqB19O/HYvEAxdiM@sirena.org.uk>
Date:   Wed, 8 Jun 2022 11:12:04 +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 Wed, Jun 08, 2022 at 11:15:56AM +0800, ChiYuan Huang wrote:
> Mark Brown <broonie@...nel.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.

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

> > > +     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.

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