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: 8894341 Return-Path: X-Original-To: patchwork-linux-fbdev@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 4E3369F39A for ; Wed, 20 Apr 2016 19:17:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4F10A20254 for ; Wed, 20 Apr 2016 19:17:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5E7EA2024F for ; Wed, 20 Apr 2016 19:17:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751002AbcDTTRb (ORCPT ); Wed, 20 Apr 2016 15:17:31 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:60121 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750941AbcDTTRa (ORCPT ); Wed, 20 Apr 2016 15:17:30 -0400 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 Cc: kernel@pengutronix.de, Jean-Christophe Plagniol-Villard , linux-fbdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org 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-fbdev@vger.kernel.org Sender: linux-fbdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham 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);