Received-SPF: pass (google.com: domain of linux-kernel+bounces-9137-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20231221211222.1380658-1-martin.blumenstingl@googlemail.com> <0c99b575-5cf2-4bd6-8cfd-af19f5fd58da@sirena.org.uk>
In-Reply-To: <0c99b575-5cf2-4bd6-8cfd-af19f5fd58da@sirena.org.uk>
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date: Thu, 21 Dec 2023 23:42:29 +0100
Message-ID: <CAFBinCDJnVzE2sMwu52MQGTKW7dtCuUoj63ZZHhJPJO0+dZDkg@mail.gmail.com>
Subject: Re: [RFC PATCH v1] regulator: pwm-regulator: Fix continuous
 get_voltage for disabled PWM
To: Mark Brown <broonie@kernel.org>
Cc: lgirdwood@gmail.com, =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= <u.kleine-koenig@pengutronix.de>, 
	linux-pwm@vger.kernel.org, linux-amlogic@lists.infradead.org, 
	linux-kernel@vger.kernel.org, Thierry Reding <thierry.reding@gmail.com>, 
	Heiner Kallweit <hkallweit1@gmail.com>, Dmitry Rokosov <ddrokosov@sberdevices.ru>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Mark,

On Thu, Dec 21, 2023 at 10:45=E2=80=AFPM Mark Brown <broonie@kernel.org> wr=
ote:
>
> On Thu, Dec 21, 2023 at 10:12:22PM +0100, Martin Blumenstingl wrote:
>
> > It turns out that at least some bootloader versions are keeping the PWM
> > output disabled. This is not a problem due to the specific design of th=
e
> > regulator: when the PWM output is disabled the output pin is pulled LOW=
,
> > effectively achieving a 0% duty cycle (which in return means that VDDEE
> > voltage is at 1140mV).
>
> Hrm.  Perhaps the regulator should figure out that it's on with a
> minimum voltage of 1.14V in this case - AIUI that broadly corresponds to
> your change except for the fact that it doesn't recognise that there's
> actually an output in this case since it assumes that disabling the PWM
> disables the output which isn't the case with this hardware.  We'd need
> to know more about the PWM in that case though I think.
If you have any specific questions then feel free to ask.
Generally it's a very simple PWM controller:
- when disabled the output is LOW
- when enabled the output matches the requested period and duty cycle
as best as possible (depending on the available input clocks)

> > The problem comes when the pwm-regulator driver tries to initialize the
> > PWM output. To do so it reads the current state from the hardware, whic=
h
> > is:
> >   period: 3666ns
> >   duty cycle: 3333ns (=3D ~91%)
> >   enabled: false
> > Then those values are translated using the continuous voltage range to
> > 860mV.
>
> > Later, when the regulator is being enabled (either by the regulator cor=
e
> > due to the always-on flag or first consumer - in this case the lima
> > driver for the Mali-450 GPU) the pwm-regulator driver tries to keep the
> > voltage (at 860mV) and just enable the PWM output. This is when things
> > start to go wrong as the typical voltage used for VDDEE is 1100mV.
>
> So, the constraints say that the 860mV voltage is within range.  Where
> does the requirement for 1.1V come from in this situation?  Is it just
> that lima hasn't started yet and requires the 1.1V for hardware init
> (and presumably power on) even if it can use a lower voltage at runtime?
The vendor BSP includes a custom u-boot with lots of relevant
information for which there's seemingly no documentation.
It seems that 1.1V is what should be used during normal operation.
0.86V is what can be used during system suspend (when power to the
Cortex-A5 cores is turned off and an integrated ARC core is taking
over for wakeup purposes).
Hence the supported voltage range of 0.86..1.1V


Best regards,
Martin