Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933337AbbGJUoY (ORCPT ); Fri, 10 Jul 2015 16:44:24 -0400 Received: from bear.ext.ti.com ([192.94.94.41]:40154 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932831AbbGJUoR (ORCPT ); Fri, 10 Jul 2015 16:44:17 -0400 Message-ID: <55A02E89.4090403@ti.com> Date: Sat, 11 Jul 2015 05:43:53 +0900 From: "Kim, Milo" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Sean Paul CC: , Lee Jones , Linux Kernel Mailing List Subject: Re: [PATCH v2] backlight: lp855x: use private data for regulator control References: <1436516798-4132-1-git-send-email-milo.kim@ti.com> In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3219 Lines: 83 Hi Paul, On 7/11/2015 12:01 AM, Sean Paul wrote: > On Fri, Jul 10, 2015 at 4:26 AM, Milo Kim wrote: >> LP855x backlight device can be enabled by external VDD input. >> The 'supply' data is used for this purpose. >> It's kind of private data which runs internally, so there is no reason to >> expose to the platform data. >> >> And devm_regulator_get() is moved from _parse_dt() to _probe(). >> Regulator consumer(lp855x) can control regulator not only from DT but also >> from platform data configuration in a source file such like board-*.c. >> >> If 'power' regulator driver is not ready, lp855x should continue to work >> because the power supply control is optional. So -EPROBE_DEFER return code >> is removed. >> >> v1->v2: >> Keeps optional property '-supply' in LP855x DT binding. >> >> Cc: Sean Paul >> Cc: Jingoo Han >> Cc: Lee Jones >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Milo Kim >> --- >> drivers/video/backlight/lp855x_bl.c | 20 +++++++++----------- >> include/linux/platform_data/lp855x.h | 2 -- >> 2 files changed, 9 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c >> index a26d3bb..277d5ca 100644 >> --- a/drivers/video/backlight/lp855x_bl.c >> +++ b/drivers/video/backlight/lp855x_bl.c >> @@ -73,6 +73,7 @@ struct lp855x { >> struct device *dev; >> struct lp855x_platform_data *pdata; >> struct pwm_device *pwm; >> + struct regulator *supply; /* regulator for VDD input */ >> }; >> >> static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data) >> @@ -384,13 +385,6 @@ static int lp855x_parse_dt(struct lp855x *lp) >> pdata->rom_data = &rom[0]; >> } >> >> - pdata->supply = devm_regulator_get(dev, "power"); >> - if (IS_ERR(pdata->supply)) { >> - if (PTR_ERR(pdata->supply) == -EPROBE_DEFER) >> - return -EPROBE_DEFER; >> - pdata->supply = NULL; >> - } >> - >> lp->pdata = pdata; >> >> return 0; >> @@ -431,8 +425,12 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) >> else >> lp->mode = REGISTER_BASED; >> >> - if (lp->pdata->supply) { >> - ret = regulator_enable(lp->pdata->supply); >> + lp->supply = devm_regulator_get(lp->dev, "power"); >> + if (IS_ERR(lp->supply)) >> + lp->supply = NULL; > > > Hi Milo, > You removed the probe deferral handling on the regulator and broke > probe deferral in cases where the regulator isn't ready. This power supply is optional. Even if lp855x can not get regulator driver, it should work. (And I saw same comment in the DT. The 'power-supply' property is optional). So -EPORBE_DEFER is not necessary in _probe(). Best regards, Milo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/