[PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

[PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

[Mark Brown]

The approach that's being taken by the mmc-twl4030 driver to disabling
regulators is a normal and supported one so there is no need to print
messages on the console warning about this - their system is functioning
normally.

Signed-off-by: Mark Brown <broonie@sirena.org.uk>
---
 arch/arm/mach-omap2/mmc-twl4030.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/mmc-twl4030.c b/arch/arm/mach-omap2/mmc-twl4030.c
index 8db1f29..b76a31a 100644
--- a/arch/arm/mach-omap2/mmc-twl4030.c
+++ b/arch/arm/mach-omap2/mmc-twl4030.c
@@ -136,14 +136,11 @@ static int twl_mmc_late_init(struct device *dev)
 			 * (which is safe for MMC, but not in general).
 			 */
 			if (regulator_is_enabled(hsmmc[i].vcc) > 0) {
-				dev_warn(dev, "APPLY REGULATOR HACK for vmmc\n");
 				regulator_enable(hsmmc[i].vcc);
 				regulator_disable(hsmmc[i].vcc);
 			}
 			if (hsmmc[i].vcc_aux) {
 				if (regulator_is_enabled(reg) > 0) {
-					dev_warn(dev, "APPLY REGULATOR HACK "
-						"for vmmc_aux\n");
 					regulator_enable(reg);
 					regulator_disable(reg);
 				}
-- 
1.6.2.1

Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

[David Brownell]

On Monday 30 March 2009, Mark Brown wrote:
> The approach that's being taken by the mmc-twl4030 driver to disabling
> regulators is a normal and supported one 

So when are you going to fix the regulator docs to report that:

	ALL regulator consumers must start by enabling and
	then disabling the regulator.

The regulator framework is still broken in this respect.
(Not that you want to hear that, but it's still true.)

Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

[Mark Brown]

On Mon, Mar 30, 2009 at 01:53:43PM -0700, David Brownell wrote:

> So when are you going to fix the regulator docs to report that:

> 	ALL regulator consumers must start by enabling and
> 	then disabling the regulator.

The documention should not be changed to say that since only consumers
that need the regulator to be off at startup should do this, and then
probably only if they find that it is already enabled.

Other consumers do not need to do this.  Consumers that want to enable a
regulator at startup can do so directly.  Consumers that don't need a
specific state (for example, because they are able to share the
regulator and don't need it enabling) should just leave it alone.

[APPLIED] [PATCH] OMAP: Don't warn user about expected behaviour in

[Tony Lindgren]

This patch has been applied to the linux-omap
by youw fwiendly patch wobot.

Initial commit ID (Likely to change): e8acebfbfd355bc52a6a18c173c400f15cb8e4a9

PatchWorks
http://patchwork.kernel.org/patch/15244/

Git
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit;h=e8acebfbfd355bc52a6a18c173c400f15cb8e4a9

Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

[Tony Lindgren]

* Mark Brown <broonie@sirena.org.uk> [090331 14:00]:
> On Mon, Mar 30, 2009 at 01:53:43PM -0700, David Brownell wrote:
> 
> > So when are you going to fix the regulator docs to report that:
> 
> > 	ALL regulator consumers must start by enabling and
> > 	then disabling the regulator.
> 
> The documention should not be changed to say that since only consumers
> that need the regulator to be off at startup should do this, and then
> probably only if they find that it is already enabled.
> 
> Other consumers do not need to do this.  Consumers that want to enable a
> regulator at startup can do so directly.  Consumers that don't need a
> specific state (for example, because they are able to share the
> regulator and don't need it enabling) should just leave it alone.

Maybe you guys have gone back and forth on this option too.. But what
if we have something in regulator_init_data that would tell the
regulator to reset the regulator on init? That could be then be
then disabled with some cmdline option if needed for debugging or
while booting from other operating systems etc.

I've applied Mark's patch to l-o, I guess now I need to start sorting
out what we need to mainline for twl4030-mmc.c for this -rc :)

Regards,

Tony

Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

[Mark Brown]

On Fri, Apr 03, 2009 at 05:26:52PM -0700, Tony Lindgren wrote:

> Maybe you guys have gone back and forth on this option too.. But what
> if we have something in regulator_init_data that would tell the
> regulator to reset the regulator on init? That could be then be
> then disabled with some cmdline option if needed for debugging or
> while booting from other operating systems etc.

Yup, that works perfectly well - in fact, Jonathan Cameron has a patch
adding a new constraint doing exactly that which managed ought to have
been merged but ended up falling through the cracks for 2.6.30.  I'm
expecting him to do the required refresh early on in this cycle.  Since
it's an explicit constraint we can just go ahead and apply it without
needing an ability to override it.

There's also support for powering off any unused regulators in a
late_initcall() which did get merged but that doesn't help with things
like MMC since they'll be probing before late_initcall().

Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

[David Brownell]

On Tuesday 31 March 2009, Mark Brown wrote:
> On Mon, Mar 30, 2009 at 01:53:43PM -0700, David Brownell wrote:
> 
> > So when are you going to fix the regulator docs to report that:
> 
> > 	ALL regulator consumers must start by enabling and
> > 	then disabling the regulator.
> 
> The documention should not be changed to say that since only consumers
> that need the regulator to be off at startup should do this, and then
> probably only if they find that it is already enabled.

"Probably" shows you still don't have a good answer...

I see that mainline now has ca7255614e0861e36480103f4a402a115803d7b5
which is a variant of the first late-fixup patch I sent.  The isssue
with that approach is that it leaves a huge window between regulator
init and its late fixup ... during which driver probe() calls suffer
the bad effects of the current self-inconsistent initialization.  So
it doesn't really address the MMC cases.


On Friday 03 April 2009, Tony Lindgren wrote:
> 			... what
> if we have something in regulator_init_data that would tell the
> regulator to reset the regulator on init?

That would address the MMC scenario, where the stack always wants to
start with regulator "off" if it's possible.

And in fact, that's exactly what I think folk should be doing with
the recent additions to twl4030-power supporting explicit init of
all the power resources ... as done with e.g. Beagle.  The benefit
of doing it that way is that it can cover the regulators which are
not actually used on the board (like VAUX3 on Beagle, which u-boot
enables but is not wired to anything) plus non-regulator resources
like CLKREQ etc.

So there's an OMAP-specific workaround now in place, albeit not
yet headed towards mainline.

- Dave

Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

[Mark Brown]

On Fri, Apr 03, 2009 at 06:03:33PM -0700, David Brownell wrote:
> On Tuesday 31 March 2009, Mark Brown wrote:

> > The documention should not be changed to say that since only consumers
> > that need the regulator to be off at startup should do this, and then
> > probably only if they find that it is already enabled.

> "Probably" shows you still don't have a good answer...

No, it means that I can't think of any reason why they'd want to bounce
the supply but I'm open to the possibility that there could be one.

> I see that mainline now has ca7255614e0861e36480103f4a402a115803d7b5
> which is a variant of the first late-fixup patch I sent.  The isssue
> with that approach is that it leaves a huge window between regulator
> init and its late fixup ... during which driver probe() calls suffer
> the bad effects of the current self-inconsistent initialization.  So
> it doesn't really address the MMC cases.

That change was not intended to do anything for MMC, it's there for
other users like shared regulators and situations where no consumer
driver instantiates.

It is really important that you engage with the feedback you are
getting.  At this point we are all quite familiar with your views and we
do understand them, restating them is not what's needed.  Changes have
to be made with an understanding of the existing interface and how it
supports the use cases the API has, including users other than MMC.
Changes also need to be made with consideration given to merge issues.

> And in fact, that's exactly what I think folk should be doing with
> the recent additions to twl4030-power supporting explicit init of
> all the power resources ... as done with e.g. Beagle.  The benefit

...

> So there's an OMAP-specific workaround now in place, albeit not
> yet headed towards mainline.

With Jonathan's patch there will be support in the regulator API for
doing this with all regulators.