Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751554AbbGLKhL (ORCPT <rfc822;w@1wt.eu>);
	Sun, 12 Jul 2015 06:37:11 -0400
Received: from mail-pd0-f172.google.com ([209.85.192.172]:33347 "EHLO
	mail-pd0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751120AbbGLKhJ (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Sun, 12 Jul 2015 06:37:09 -0400
From: "Jingoo Han" <jingoohan1@gmail.com>
To: "'Kim, Milo'" <milo.kim@ti.com>
Cc: "'Lee Jones'" <lee.jones@linaro.org>,
        "'Linux Kernel Mailing List'" <linux-kernel@vger.kernel.org>,
        "'Sean Paul'" <seanpaul@chromium.org>,
        "'Jingoo Han'" <jingoohan1@gmail.com>
References: <1436516798-4132-1-git-send-email-milo.kim@ti.com> <CAOw6vbJy0p7Hg+W4=3sddj1rTOXxf4v9S2Gp-fPnJq3rVD4Fdg@mail.gmail.com> <55A02E89.4090403@ti.com> <CAOw6vbL7epag+2GuR8yRsLxuio7v+OxwekS=33kVNTAUfBuyNQ@mail.gmail.com> <55A03086.7050207@ti.com> <55A034F2.9010404@ti.com>
In-Reply-To: <55A034F2.9010404@ti.com>
Subject: Re: [PATCH v2] backlight: lp855x: use private data for regulator control
Date: Sun, 12 Jul 2015 19:37:02 +0900
Message-ID: <000401d0bc8e$b382dc20$1a889460$@com>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Office Outlook 12.0
Thread-Index: AdC7VQHEjhexWD+KQMCD/LZxFF1czQBOT/Hw
Content-Language: ko
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2977
Lines: 82

On Saturday, July 11, 2015 6:11 AM, Kim, Milo wrote:
> On 7/11/2015 5:52 AM, Kim, Milo wrote:
> > Hi Paul,
> >
> > On 7/11/2015 5:49 AM, Sean Paul wrote:
> >> On Fri, Jul 10, 2015 at 4:43 PM, Kim, Milo <milo.kim@ti.com> wrote:
> >>> Hi Paul,
> >>>
> >>>
> >>> On 7/11/2015 12:01 AM, Sean Paul wrote:
> >>>>
> >>>> On Fri, Jul 10, 2015 at 4:26 AM, Milo Kim <milo.kim@ti.com> 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 '<name>-supply' in LP855x DT binding.
> >>>>>
> >>>>> Cc: Sean Paul <seanpaul@chromium.org>
> >>>>> Cc: Jingoo Han <jingoohan1@gmail.com>
> >>>>> Cc: Lee Jones <lee.jones@linaro.org>
> >>>>> Cc: linux-kernel@vger.kernel.org
> >>>>> Signed-off-by: Milo Kim <milo.kim@ti.com>
> >>>>> ---
> >>>>>     drivers/video/backlight/lp855x_bl.c  | 20 +++++++++-----------
> >>>>>     include/linux/platform_data/lp855x.h |  2 --
> >>>>>     2 files changed, 9 insertions(+), 13 deletions(-)
> >>>>>

[.....]

> >>>>
> >>>>
> >>>> 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().
> >>>
> >>
> >> I respectfully disagree. devm_regulator_get can return EPROBE_DEFER if
> >> the regulator is valid (and specified in the dt), but not ready to be
> >> used yet. In this case, your patch will assume it doesn't exist and
> >> will never use it. This Is Bad.
> >
> > So do you think this power supply should be mandatory in this driver?
> 
> The devm_regulator_get_optinonal() seems the right API for this case.
> Let me take a look at and get back to you.

The devm_regulator_get_optinonal() looks better. But, I am not sure.
Please review devm_regulator_get_optinonal() carefully, before sending
the V3 patch.

Best regards,
Jingoo Han

> 
> 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/