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

From: Mark Brown <broonie@kernel.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: lgirdwood@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 2/4] regulator: Add driver for MT6331 PMIC regulators
Date: Mon, 23 May 2022 14:00:33 +0100	[thread overview]
Message-ID: <YouFcSapkVC7ZfuP@sirena.org.uk> (raw)
In-Reply-To: <6cc68be9-e509-eae4-801d-997fdc01dcf2@collabora.com>

[-- Attachment #1.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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel