From patchwork Wed Mar 11 20:34:59 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Brownell X-Patchwork-Id: 11200 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n2BKZB0X025029 for ; Wed, 11 Mar 2009 20:35:11 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750728AbZCKUfE (ORCPT ); Wed, 11 Mar 2009 16:35:04 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750807AbZCKUfE (ORCPT ); Wed, 11 Mar 2009 16:35:04 -0400 Received: from smtp127.sbc.mail.sp1.yahoo.com ([69.147.65.186]:42336 "HELO smtp127.sbc.mail.sp1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750728AbZCKUfC (ORCPT ); Wed, 11 Mar 2009 16:35:02 -0400 Received: (qmail 49668 invoked from network); 11 Mar 2009 20:35:00 -0000 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=s1024; d=pacbell.net; h=Received:X-YMail-OSG:X-Yahoo-Newman-Property:From:To:Subject:Date:User-Agent:Cc:References:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding:Content-Disposition:Message-Id; b=xfOTk2TiS2Zlm8oAbT4ItzCj0qx7XFEvSSezA7aPZ8qXLmgh8NPUURhv65zZ1ZIjXkZDA0jMjITS6osxcqJTZHIN9qCNcB+mLK5kp/4BzgQJZ3Z6VRM21Z1rKMr+vG8piazexEpMpSCB8bGLLJHKyVFL0ier3CAEYzdQHhKKW84= ; Received: from unknown (HELO pogo) (david-b@69.226.224.20 with plain) by smtp127.sbc.mail.sp1.yahoo.com with SMTP; 11 Mar 2009 20:35:00 -0000 X-YMail-OSG: fQ_z7VoVM1lOJchwpRmekS5viVyuaBgarSUTMQOX2CSygVTY_wF6hfxKvU2pu16ZeDqAZQCut0JeW3_Kl1soCTl7oe4YbfW6f5_SGfuCikmPBMVLxeAh0DN3ZIz38B804C5Hpj83ueTOI4aQwtGV7dT6LTx03qD7c8q7BwKMyzvnjhKLG2jviXwkPL3RKi.UmTr1fh1LpDDOn5TlRpZKj.o5HrhzXg-- X-Yahoo-Newman-Property: ymail-3 From: David Brownell To: me@felipebalbi.com Subject: Re: [patch 2.6.29-rc7-omap-git] Overo: MMC regulator configuration Date: Wed, 11 Mar 2009 12:34:59 -0800 User-Agent: KMail/1.9.10 Cc: OMAP , Steve Sakoman , Mark Brown References: <200903111158.29860.david-b@pacbell.net> <20090311193347.GC19730@gandalf> In-Reply-To: <20090311193347.GC19730@gandalf> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200903111334.59499.david-b@pacbell.net> Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org On Wednesday 11 March 2009, Felipe Balbi wrote: > with this patch I get: > > regulator: Unable to get requested regulator: vmmc_aux That's completely safe; annoying and useless noise from the regulator core, that's all. > ------------[ cut here ]------------ > WARNING: at drivers/regulator/core.c:1216 regulator_disable+0x64/0x90() > unbalanced disables for supply mmci-omap-hs.0-vmmc This is that bug in the regulator framework which Mark is so reluctant to fix ... the one whereby regulators aren't initialized into a known state. In this case, the Overo probably booted from MMC, and U-Boot left the MMC1 regulator on. The first thing the MMC stack does is force the regulator off, so Linux can properly enumerate the device. (Of course, some boards will boot from flash and not turn the regulator on...) I posted a patch to address that regulator framework bug earlier: http://marc.info/?l=linux-omap&m=123654568330807&w=2 I've appended a slightly simplified version of that patch below. > Modules linked in: > [] (dump_stack+0x0/0x14) from [] (warn_slowpath+0x6c/0x88) > [] (warn_slowpath+0x0/0x88) from [] (regulator_disable+0x64/0x90) >  r3:cf9deae0 r2:c0425c79 >  r7:cfa16e00 r6:cfa06c38 r5:cfa16e00 r4:fffffffb > [] (regulator_disable+0x0/0x90) from [] (mmc_regulator_set_ocr+0xb0/0xcc) >  r7:cfa16e00 r6:00000001 r5:00000000 r4:00000000 > [] (mmc_regulator_set_ocr+0x0/0xcc) from [] (twl_mmc1_set_power+0xc0/0xec) >  r7:cfa93538 r6:00000000 r5:00000000 r4:c04a5b58 > [] (twl_mmc1_set_power+0x0/0xec) from [] (omap_mmc_set_ios+0x54/0x314) >  r7:cfa93538 r6:cfa935c0 r5:cfa93400 r4:cf9a93c0 > [] (omap_mmc_set_ios+0x0/0x314) from [] (mmc_power_off+0x90/0x9c) >  r7:00000000 r6:00000000 r5:cfa93400 r4:00000000 > [] (mmc_power_off+0x0/0x9c) from [] (mmc_start_host+0x14/0x24) >  r5:00000000 r4:cfa93400 > [] (mmc_start_host+0x0/0x24) from [] (mmc_add_host+0x64/0x70) >  r5:00000000 r4:cfa93400 > [] (mmc_add_host+0x0/0x70) from [] (omap_mmc_probe+0x464/0x614) >  r5:cfa935c0 r4:cfa93470 > [] (omap_mmc_probe+0x0/0x614) from [] (platform_drv_probe+0x20/0x24) > [] (platform_drv_probe+0x0/0x24) from [] (driver_probe_device+0xd4/0x180) > [] (driver_probe_device+0x0/0x180) from [] (__driver_attach+0x68/0x8c) >  r7:cfa15120 r6:c0499e34 r5:cfa06290 r4:cfa06208 > [] (__driver_attach+0x0/0x8c) from [] (bus_for_each_dev+0x4c/0x80) >  r7:cfa15120 r6:c0499e34 r5:c01d6e00 r4:00000000 > [] (bus_for_each_dev+0x0/0x80) from [] (driver_attach+0x20/0x28) >  r6:c0499e34 r5:00000000 r4:00000000 > [] (driver_attach+0x0/0x28) from [] (bus_add_driver+0xa4/0x20c) > [] (bus_add_driver+0x0/0x20c) from [] (driver_register+0x98/0x120) >  r8:00000000 r7:c002182c r6:c0499e34 r5:00000000 r4:c0028a58 > [] (driver_register+0x0/0x120) from [] (platform_driver_register+0x6c/0x88) >  r9:00000000 r8:00000000 r7:c002182c r6:00000000 r5:00000000 > r4:c0028a58 > [] (platform_driver_register+0x0/0x88) from [] (omap_mmc_init+0x14/0x1c) > [] (omap_mmc_init+0x0/0x1c) from [] (__exception_text_end+0x5c/0x19c) > [] (__exception_text_end+0x0/0x19c) from [] (kernel_init+0x80/0xf4) >  r8:00000000 r7:00000000 r6:00000000 r5:00000000 r4:c0028a58 > [] (kernel_init+0x0/0xf4) from [] (do_exit+0x0/0x6a4) >  r4:00000000 > ---[ end trace e00c52255ce89b8f ]--- > mmci-omap-hs mmci-omap-hs.1: Failed to get debounce clock > regulator: Unable to get requested regulator: vmmc > > does this depend on any patch dave ? Using current omap HEAD. I was probably running with the patch appended below. - Dave --- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ========== CUT HERE From: David Brownell Make the regulator setup code simpler and more consistent: - The only difference between "boot_on" and "always_on" is that an "always_on" regulator won't be disabled. Both will be active (and usecount will be 1) on return from setup. - Regulators not marked as "boot_on" or "always_on" won't be active (and usecount will be 0) on return from setup. The exception to that simple policy is when there's a non-Linux interface to the regulator ... e.g. if either a DSP or the CPU running Linux can enable the regulator, and the DSP needs it to be on, then it will be on. Signed-off-by: David Brownell --- drivers/regulator/core.c | 47 +++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -711,6 +711,7 @@ static int set_machine_constraints(struc int ret = 0; const char *name; struct regulator_ops *ops = rdev->desc->ops; + int enable = 0; if (constraints->name) name = constraints->name; @@ -799,10 +800,6 @@ static int set_machine_constraints(struc } } - /* are we enabled at boot time by firmware / bootloader */ - if (rdev->constraints->boot_on) - rdev->use_count = 1; - /* do we need to setup our suspend state */ if (constraints->initial_state) { ret = suspend_prepare(rdev, constraints->initial_state); @@ -814,21 +811,39 @@ static int set_machine_constraints(struc } } - /* if always_on is set then turn the regulator on if it's not - * already on. */ - if (constraints->always_on && ops->enable && - ((ops->is_enabled && !ops->is_enabled(rdev)) || - (!ops->is_enabled && !constraints->boot_on))) { - ret = ops->enable(rdev); - if (ret < 0) { - printk(KERN_ERR "%s: failed to enable %s\n", - __func__, name); - rdev->constraints = NULL; - goto out; + /* Should this be enabled when we return from here? The difference + * between "boot_on" and "always_on" is that "always_on" regulators + * won't ever be disabled. + */ + if (constraints->boot_on || constraints->always_on) + enable = 1; + + /* Make sure the regulator isn't wrongly enabled or disabled. + * Bootloaders are often sloppy about leaving things on; and + * sometimes Linux wants to use a different model. + */ + if (enable) { + if (ops->enable) { + ret = ops->enable(rdev); + if (ret < 0) + pr_warning("%s: %s disable --> %d\n", + __func__, name, ret); + } + if (ret >= 0) + rdev->use_count = 1; + } else { + if (ops->disable) { + ret = ops->disable(rdev); + if (ret < 0) + pr_warning("%s: %s disable --> %d\n", + __func__, name, ret); } } - print_constraints(rdev); + if (ret < 0) + rdev->constraints = NULL; + else + print_constraints(rdev); out: return ret; }