Re: [PATCH 2/4] regulator: Add driver for MT6331 PMIC regulators

[Mark Brown <broonie@kernel.org>]

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

On Mon, May 23, 2022 at 02:49:19PM +0200, AngeloGioacchino Del Regno wrote:
> Il 20/05/22 16:45, Mark Brown ha scritto:
> > On Fri, May 20, 2022 at 03:33:03PM +0200, AngeloGioacchino Del Regno wrote:

> > > +static const unsigned int ldo_volt_table10[] = {
> > > +	1200000, 1300000, 1500000, 1800000,
> > > +	1200000, 1300000, 1500000, 1800000,
> > > +	1200000, 1300000, 1500000, 1800000,
> > > +	1200000, 1300000, 1500000, 1800000,
> > > +};

> > So the top bits of the voltate selection field just get ignored?  Might
> > be easier to just write the code to not include the top bits.
> > 

> No, they're all valid values for real... but I guess that I can simplify
> this voltage table by simply modifying the bitmask that we use for the
> regulators that are using this table....

Right, my point here is that it looks awfully like the documentation
(this came from documentation I guess?) is including some extra bits
that get ignored in the voltage selection field here.  That seems like a
weird choice somewhere along the line.

> > > +	if (info->qi > 0) {
> > > +		reg = info->desc.enable_reg;
> > > +		en_mask = info->qi;

> > If the regulator doesn't have status readback it shouldn't provide a
> > get_status() operation.

> What I've understood is that when there's no "QI" flag, the enable register
> will provide the regulator status (EN/DIS) acting like QI, that's why I've
> added that if branch...

> Anyway, I'll recheck this part before sending the next version!

That would be fairly unusual, often a regulator won't even detect when
it's gone out of regulation.

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