lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 12 Jul 2015 19:37:02 +0900 From: "Jingoo Han" <jingoohan1@...il.com> To: "'Kim, Milo'" <milo.kim@...com> Cc: "'Lee Jones'" <lee.jones@...aro.org>, "'Linux Kernel Mailing List'" <linux-kernel@...r.kernel.org>, "'Sean Paul'" <seanpaul@...omium.org>, "'Jingoo Han'" <jingoohan1@...il.com> Subject: Re: [PATCH v2] backlight: lp855x: use private data for regulator control 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@...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@...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@...omium.org> > >>>>> Cc: Jingoo Han <jingoohan1@...il.com> > >>>>> Cc: Lee Jones <lee.jones@...aro.org> > >>>>> Cc: linux-kernel@...r.kernel.org > >>>>> Signed-off-by: Milo Kim <milo.kim@...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@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists