From patchwork Wed Apr 20 19:17:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= X-Patchwork-Id: 8894411 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 0D4B5BF440 for ; Wed, 20 Apr 2016 19:20:54 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 214B120260 for ; Wed, 20 Apr 2016 19:20:53 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2C55B2025A for ; Wed, 20 Apr 2016 19:20:52 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1asxdM-0003Hw-JX; Wed, 20 Apr 2016 19:17:56 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1asxdJ-000377-D6 for linux-arm-kernel@lists.infradead.org; Wed, 20 Apr 2016 19:17:54 +0000 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0] ident=Debian-exim) by metis.ext.pengutronix.de with esmtps (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1asxcu-0002b7-GX; Wed, 20 Apr 2016 21:17:28 +0200 Received: from ukl by ptx.hi.pengutronix.de with local (Exim 4.84_2) (envelope-from ) id 1asxct-0007Qb-G7; Wed, 20 Apr 2016 21:17:27 +0200 Date: Wed, 20 Apr 2016 21:17:27 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Tomi Valkeinen Subject: Re: [PATCH 2/3] video: fbdev: imxfb: enable lcd regulator in .probe Message-ID: <20160420191727.GS29108@pengutronix.de> References: <1457380425-20244-1-git-send-email-u.kleine-koenig@pengutronix.de> <1457380425-20244-3-git-send-email-u.kleine-koenig@pengutronix.de> <56E2AA80.4050908@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <56E2AA80.4050908@ti.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160420_121753_838007_0DE6DDB0 X-CRM114-Status: GOOD ( 22.61 ) X-Spam-Score: -2.9 (--) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-fbdev@vger.kernel.org, Jean-Christophe Plagniol-Villard , linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hello Tomi, On Fri, Mar 11, 2016 at 01:22:40PM +0200, Tomi Valkeinen wrote: > > On 07/03/16 21:53, Uwe Kleine-König wrote: > > This asserts that the display is on after the driver is initialized. > > Otherwise, depending on how the boot loader handled the display, it is > > either disabled as the regulator doesn't seem in use, or it stays off. > > > > Signed-off-by: Uwe Kleine-König > > --- > > drivers/video/fbdev/imxfb.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c > > index c5fcedde2a60..3dd2824e6773 100644 > > --- a/drivers/video/fbdev/imxfb.c > > +++ b/drivers/video/fbdev/imxfb.c > > @@ -979,8 +979,17 @@ static int imxfb_probe(struct platform_device *pdev) > > imxfb_enable_controller(fbi); > > fbi->pdev = pdev; > > > > + if (!IS_ERR(fbi->lcd_pwr)) { > > + ret = regulator_enable(fbi->lcd_pwr); > > + if (ret) > > + goto failed_regulator; > > + } > > + > > return 0; > > > > +failed_regulator: > > + imxfb_disable_controller(fbi); > > + > > failed_lcd: > > unregister_framebuffer(info); > > So I didn't go through the code in detail, but this doesn't look correct > to me. > > Where is the regulator disabled which now gets enabled in probe? It isn't, the display should be on after all :-) > imxfb_lcd_set_power() handles the regulator enable/disable, so doesn't > this mean the regulator would always be enabled? You first enable it in > probe, then imxfb_lcd_set_power() enables it at some point (?), so the > enable-count is two then. imxfb_lcd_set_power isn't called during boot. > To be honest, I've never used 'struct lcd_ops', but I think the enabling > of the regulator should happen somehow via that. If the regulator needs > to be enabled at probe time, then the probe should somehow cause > lcd_ops->set_power to get called. > > Why does the regulator need to be enabled at probe? Or are you saying > imxfb_lcd_set_power() is never called in your case? Right. I just confirmed that with next-20160419 with this patch on top: With that: # dmesg | grep -iE '(lcd|fb|power)' [ 0.562633] backlight supply power not found, using dummy regulator [ 0.568246] imx-fb 53fbc000.lcdc: i.MX Framebuffer driver [ 0.568326] imxfb_init_fbinfo [ 0.576275] Enabling LCD controller [ 1.652166] sdhci-esdhc-imx 53fb4000.esdhc: Got CD GPIO [ 1.658109] sdhci-esdhc-imx 53fb4000.esdhc: Got WP GPIO [ 1.703638] mmc0: SDHCI controller on 53fb4000.esdhc [53fb4000.esdhc] using DMA [ 4.094923] lcd supply: disabling The bootloader enables the lcd before booting into Linux, with the last message the display gets disabled. Best regards Uwe diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c index 76b6a7784b06..93b803ebd61d 100644 --- a/drivers/video/fbdev/imxfb.c +++ b/drivers/video/fbdev/imxfb.c @@ -1,3 +1,4 @@ +#define DEBUG /* * Freescale i.MX Frame Buffer device driver * @@ -768,6 +769,7 @@ static int imxfb_lcd_set_power(struct lcd_device *lcddev, int power) { struct imxfb_info *fbi = dev_get_drvdata(&lcddev->dev); + dev_dbg(&lcddev->dev, "%s(power=%d)\n", __func__, power); if (!IS_ERR(fbi->lcd_pwr)) { if (power) return regulator_enable(fbi->lcd_pwr);