[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7143faaec2d0b7b0fd7e0f3b512839d8@codeaurora.org>
Date: Thu, 17 May 2018 20:40:12 +0530
From: kgunda@...eaurora.org
To: Rob Herring <robh+dt@...nel.org>
Cc: Bjorn Andersson <bjorn.andersson@...aro.org>,
Lee Jones <lee.jones@...aro.org>,
Daniel Thompson <daniel.thompson@...aro.org>,
Jingoo Han <jingoohan1@...il.com>,
Jacek Anaszewski <jacek.anaszewski@...il.com>,
Pavel Machek <pavel@....cz>,
Mark Rutland <mark.rutland@....com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
Linux LED Subsystem <linux-leds@...r.kernel.org>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
dri-devel <dri-devel@...ts.freedesktop.org>,
linux-fbdev@...r.kernel.org,
linux-arm-msm <linux-arm-msm@...r.kernel.org>,
linux-arm-msm-owner@...r.kernel.org
Subject: Re: [PATCH V1 2/5] backlight: qcom-wled: Add support for WLED4
peripheral
On 2018-05-17 18:01, Rob Herring wrote:
> On Thu, May 17, 2018 at 4:47 AM, <kgunda@...eaurora.org> wrote:
>> On 2018-05-08 15:55, kgunda@...eaurora.org wrote:
>>>
>>> On 2018-05-07 21:50, Bjorn Andersson wrote:
>>>>
>>>> On Thu 03 May 02:57 PDT 2018, Kiran Gunda wrote:
>>>>
>>>>> WLED4 peripheral is present on some PMICs like pmi8998
>>>>> and pm660l. It has a different register map and also
>>>>> configurations are different. Add support for it.
>>>>>
>>>>
>>>> Several things are going on in this patch, it needs to be split to
>>>> not hide the functional changes from the structural/renames.
>>>>
>>> Ok. I will split it in the next series.
>>>>>
>>>>> Signed-off-by: Kiran Gunda <kgunda@...eaurora.org>
>>>>> ---
>>>>> .../bindings/leds/backlight/qcom-wled.txt | 172 ++++-
>>>>> drivers/video/backlight/qcom-wled.c | 749
>>>>> +++++++++++++++------
>>>>> 2 files changed, 696 insertions(+), 225 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>>>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>>>> index fb39e32..0ceffa1 100644
>>>>> ---
>>>>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>>>> @@ -1,30 +1,129 @@
>>>>> Binding for Qualcomm Technologies, Inc. WLED driver
>>>>>
>>>>> -Required properties:
>>>>> -- compatible: should be "qcom,pm8941-wled"
>>>>> -- reg: slave address
>>>>> -
>>>>> -Optional properties:
>>>>> -- default-brightness: brightness value on boot, value from: 0-4095
>>>>> - default: 2048
>>>>> -- label: The name of the backlight device
>>>>> -- qcom,cs-out: bool; enable current sink output
>>>>> -- qcom,cabc: bool; enable content adaptive backlight control
>>>>> -- qcom,ext-gen: bool; use externally generated modulator signal to
>>>>> dim
>>>>> -- qcom,current-limit: mA; per-string current limit; value from 0
>>>>> to 25
>>>>> - default: 20mA
>>>>> -- qcom,current-boost-limit: mA; boost current limit; one of:
>>>>> - 105, 385, 525, 805, 980, 1260, 1400, 1680
>>>>> - default: 805mA
>>>>> -- qcom,switching-freq: kHz; switching frequency; one of:
>>>>> - 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371,
>>>>> - 1600, 1920, 2400, 3200, 4800, 9600,
>>>>> - default: 1600kHz
>>>>> -- qcom,ovp: V; Over-voltage protection limit; one of:
>>>>> - 27, 29, 32, 35
>>>>> - default: 29V
>>>>> -- qcom,num-strings: #; number of led strings attached; value from
>>>>> 1 to
>>>>> 3
>>>>> - default: 2
>>>>> +WLED (White Light Emitting Diode) driver is used for controlling
>>>>> display
>>>>> +backlight that is part of PMIC on Qualcomm Technologies, Inc.
>>>>> reference
>>>>> +platforms. The PMIC is connected to the host processor via SPMI
>>>>> bus.
>>>>> +
>>>>> +- compatible
>>>>> + Usage: required
>>>>> + Value type: <string>
>>>>> + Definition: should be "qcom,pm8941-wled" or
>>>>> "qcom,pmi8998-wled".
>>>>> + or "qcom,pm660l-wled".
>>>>
>>>>
>>>> Better written as
>>>>
>>>> should be one of:
>>>> X
>>>> Y
>>>> Z
>>>>
>>> Will do it in the next series.
>>>>>
>>>>> +
>>>>> +- reg
>>>>> + Usage: required
>>>>> + Value type: <prop encoded array>
>>>>> + Definition: Base address of the WLED modules.
>>>>> +
>>>>> +- interrupts
>>>>> + Usage: optional
>>>>> + Value type: <prop encoded array>
>>>>> + Definition: Interrupts associated with WLED. Interrupts
>>>>> can be
>>>>> + specified as per the encoding listed under
>>>>> + Documentation/devicetree/bindings/spmi/
>>>>> + qcom,spmi-pmic-arb.txt.
>>>>
>>>>
>>>> Better to describe that this should be the "short" and "ovp"
>>>> interrupts
>>>> in this property than in the interrupt-names.
>>>>
>>> Ok. I will do it in the next series.
>>>>>
>>>>> +
>>>>> +- interrupt-names
>>>>> + Usage: optional
>>>>> + Value type: <string>
>>>>> + Definition: Interrupt names associated with the
>>>>> interrupts.
>>>>> + Must be "short" and "ovp". The short circuit
>>>>> detection
>>>>> + is not supported for PM8941.
>>>>> +
>>>>> +- label
>>>>> + Usage: required
>>>>> + Value type: <string>
>>>>> + Definition: The name of the backlight device
>>>>> +
>>>>> +- default-brightness
>>>>> + Usage: optional
>>>>> + Value type: <u32>
>>>>> + Definition: brightness value on boot, value from: 0-4095
>>>>> + Default: 2048
>>>>> +
>>>>> +- qcom,current-limit
>>>>> + Usage: optional
>>>>> + Value type: <u32>
>>>>> + Definition: uA; per-string current limit
>>>>
>>>>
>>>> You can't change unit on an existing property, that breaks any
>>>> existing
>>>> dts using the qcom,pm8941-wled compatible.
>>>>
>>>
>>>>> + value:
>>>>> + For pm8941: from 0 to 25000 with 5000 ua step
>>>>> + Default 20000 uA
>>>>> + For pmi8998: from 0 to 30000 with 5000 ua
>>>>> step
>>>>> + Default 25000 uA.
>>>>
>>>>
>>>> These values could be described just as well in mA, so keep the
>>>> original
>>>> unit - in particular since the boot-limit is in mA...
>>>>
>>> Ok. Will keep the original as is in the next series.
>>
>> Here, I may have to go with the approach as in "qcom,ovp". Because for
>> pm8941
>> the current step is 1 mA (I have wrongly mentioned as 5000uA here) and
>> for
>> PMI8998
>> the current step is 2.5 mA. Hence, I will add another variable
>> "qcom,current-limit-ua"
>> just like "qcom,ovp-mv".
>
> Use unit suffixes defined in bindings/property-units.txt.
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-arm-msm" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for pointing it ! hope I can use "qcom,current-limit-microamp"
and
"qcom,ovp-millivolt". I am asking this because i found only
"-microvolt".
"-millivolt" is not present in the bindings you pointed.
Powered by blists - more mailing lists