* [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators
@ 2021-05-19 22:12 Dmitry Baryshkov
2021-05-19 22:12 ` [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators Dmitry Baryshkov
2021-05-20 21:08 ` (subset) [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators Mark Brown
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2021-05-19 22:12 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: Michał Mirosław, linux-kernel
For the boot-on/always-on regulators the set_machine_constrainst() is
called before resolving rdev->supply. Thus the code would try to enable
rdev before enabling supplying regulator. Enforce resolving supply
regulator before enabling rdev.
Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/regulator/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f192bf19492e..e20e77e4c159 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1425,6 +1425,12 @@ static int set_machine_constraints(struct regulator_dev *rdev)
* and we have control then make sure it is enabled.
*/
if (rdev->constraints->always_on || rdev->constraints->boot_on) {
+ /* If we want to enable this regulator, make sure that we know
+ * the supplying regulator.
+ */
+ if (rdev->supply_name && !rdev->supply)
+ return -EPROBE_DEFER;
+
if (rdev->supply) {
ret = regulator_enable(rdev->supply);
if (ret < 0) {
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators
2021-05-19 22:12 [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators Dmitry Baryshkov
@ 2021-05-19 22:12 ` Dmitry Baryshkov
2021-05-20 11:57 ` Mark Brown
2021-05-20 21:08 ` (subset) [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators Mark Brown
1 sibling, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2021-05-19 22:12 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: Michał Mirosław, linux-kernel
Some regulators (e.g. fixed) do not have .enable callback per se, but
use supply regulator and enable_delay. Do not return early from
_regulator_do_enable in such cases, so that enable_delay is properly
handled.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/regulator/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e20e77e4c159..66c465bd00ca 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2516,7 +2516,8 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
ret = rdev->desc->ops->enable(rdev);
if (ret < 0)
return ret;
- } else {
+ } else if (!rdev->supply) {
+ /* Still handle delay if the regulator was just turned on using the supply */
return -EINVAL;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators
2021-05-19 22:12 ` [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators Dmitry Baryshkov
@ 2021-05-20 11:57 ` Mark Brown
2021-06-02 13:14 ` Dmitry Baryshkov
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2021-05-20 11:57 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: Liam Girdwood, Michał Mirosław, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 465 bytes --]
On Thu, May 20, 2021 at 01:12:24AM +0300, Dmitry Baryshkov wrote:
> Some regulators (e.g. fixed) do not have .enable callback per se, but
> use supply regulator and enable_delay. Do not return early from
> _regulator_do_enable in such cases, so that enable_delay is properly
> handled.
This doesn't seem like the right fix - if we didn't actually do anything
then we don't need to add a delay. We should only be doing this if some
parent regulator changed state.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: (subset) [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators
2021-05-19 22:12 [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators Dmitry Baryshkov
2021-05-19 22:12 ` [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators Dmitry Baryshkov
@ 2021-05-20 21:08 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2021-05-20 21:08 UTC (permalink / raw)
To: Liam Girdwood, Dmitry Baryshkov
Cc: Mark Brown, linux-kernel, Michał Mirosław
On Thu, 20 May 2021 01:12:23 +0300, Dmitry Baryshkov wrote:
> For the boot-on/always-on regulators the set_machine_constrainst() is
> called before resolving rdev->supply. Thus the code would try to enable
> rdev before enabling supplying regulator. Enforce resolving supply
> regulator before enabling rdev.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/2] regulator: core: resolve supply for boot-on/always-on regulators
commit: 98e48cd9283dbac0e1445ee780889f10b3d1db6a
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators
2021-05-20 11:57 ` Mark Brown
@ 2021-06-02 13:14 ` Dmitry Baryshkov
2021-06-03 10:27 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2021-06-02 13:14 UTC (permalink / raw)
To: Mark Brown; +Cc: Liam Girdwood, Michał Mirosław, open list
On Thu, 20 May 2021 at 14:58, Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, May 20, 2021 at 01:12:24AM +0300, Dmitry Baryshkov wrote:
> > Some regulators (e.g. fixed) do not have .enable callback per se, but
> > use supply regulator and enable_delay. Do not return early from
> > _regulator_do_enable in such cases, so that enable_delay is properly
> > handled.
>
> This doesn't seem like the right fix - if we didn't actually do anything
> then we don't need to add a delay. We should only be doing this if some
> parent regulator changed state.
I have implemented this, but then it becomes too fragile. If the
parent gets enabled for whatever reason just few us ago, the whole
delay would be skipped.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators
2021-06-02 13:14 ` Dmitry Baryshkov
@ 2021-06-03 10:27 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2021-06-03 10:27 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: Liam Girdwood, Michał Mirosław, open list
[-- Attachment #1: Type: text/plain, Size: 922 bytes --]
On Wed, Jun 02, 2021 at 04:14:35PM +0300, Dmitry Baryshkov wrote:
> On Thu, 20 May 2021 at 14:58, Mark Brown <broonie@kernel.org> wrote:
> > On Thu, May 20, 2021 at 01:12:24AM +0300, Dmitry Baryshkov wrote:
> > > Some regulators (e.g. fixed) do not have .enable callback per se, but
> > > use supply regulator and enable_delay. Do not return early from
> > > _regulator_do_enable in such cases, so that enable_delay is properly
> > > handled.
> > This doesn't seem like the right fix - if we didn't actually do anything
> > then we don't need to add a delay. We should only be doing this if some
> > parent regulator changed state.
> I have implemented this, but then it becomes too fragile. If the
> parent gets enabled for whatever reason just few us ago, the whole
> delay would be skipped.
This seems like a solvable problem, we can track what parents are doing
or teach parents about their children for example.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-03 10:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 22:12 [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators Dmitry Baryshkov
2021-05-19 22:12 ` [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators Dmitry Baryshkov
2021-05-20 11:57 ` Mark Brown
2021-06-02 13:14 ` Dmitry Baryshkov
2021-06-03 10:27 ` Mark Brown
2021-05-20 21:08 ` (subset) [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).