diff mbox

[V3,3/7] backlight: qcom-wled: Add new properties for PMI8998

Message ID 1529406822-15379-4-git-send-email-kgunda@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Kiran Gunda June 19, 2018, 11:13 a.m. UTC
Update the bindings with the new properties used for
PMI8998.

Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
 .../bindings/leds/backlight/qcom-wled.txt          | 84 ++++++++++++++++++++--
 1 file changed, 77 insertions(+), 7 deletions(-)

Comments

Bjorn Andersson June 19, 2018, 11:03 p.m. UTC | #1
On Tue 19 Jun 04:13 PDT 2018, Kiran Gunda wrote:
> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
[..]
>  - qcom,num-strings
>  	Usage:        optional
>  	Value type:   <u32>
>  	Definition:   #; number of led strings attached;
> -		      value from 1 to 3. default: 2
> -		      This property is supported only for PM8941.
> +		      value: For PM8941 from 1 to 3. default: 2
> +			     For PMI8998 from 1 to 4. default: 4
[..]
> +- qcom,enabled-strings
> +	Usage:        optional
> +	Value tyoe:   <u32 array>
> +	Definition:   Array of the WLED strings numbered from 0 to 3. Each
> +		      string of leds are operated individually. Specify the
> +		      list of strings used by the device. Any combination of
> +		      led strings can be used.
> +		      for pm8941: Default values are [00 01].
> +		      for pmi8998: Default values are [00 01 02 03].

I would suggest omitting the defaults, as we can see in several places
in this document we end up having to update the document with new
defaults for each platform.

Also, per the defaults of the optional qcom,num-strings these are
already the defaults...

[..]
> +pmi8998-wled@d800 {
> +	compatible = "qcom,pmi8998-wled";
> +	reg = <0xd800 0xd900>;
> +	label = "backlight";
> +
> +	interrupts = <3 0xd8 2 IRQ_TYPE_EDGE_RISING>,
> +		     <3 0xd8 1 IRQ_TYPE_EDGE_RISING>;
> +	interrupt-names = "short", "ovp";
> +	qcom,current-limit-microamp = <25000>;
> +	qcom,current-boost-limit = <805>;
> +	qcom,switching-freq = <1600>;
> +	qcom,ovp-millivolt = <29600>;
> +	qcom,num-strings = <4>;
> +	qcom,enabled-strings = <0x00 0x01 0x02 0x03>;

Please omit the pmi8998 example as well, there's no real benefit of
adding similar examples for each platform.

Regards,
Bjorn
Kiran Gunda June 20, 2018, 5:25 a.m. UTC | #2
On 2018-06-20 04:33, Bjorn Andersson wrote:
> On Tue 19 Jun 04:13 PDT 2018, Kiran Gunda wrote:
>> diff --git 
>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt 
>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
> [..]
>>  - qcom,num-strings
>>  	Usage:        optional
>>  	Value type:   <u32>
>>  	Definition:   #; number of led strings attached;
>> -		      value from 1 to 3. default: 2
>> -		      This property is supported only for PM8941.
>> +		      value: For PM8941 from 1 to 3. default: 2
>> +			     For PMI8998 from 1 to 4. default: 4
> [..]
>> +- qcom,enabled-strings
>> +	Usage:        optional
>> +	Value tyoe:   <u32 array>
>> +	Definition:   Array of the WLED strings numbered from 0 to 3. Each
>> +		      string of leds are operated individually. Specify the
>> +		      list of strings used by the device. Any combination of
>> +		      led strings can be used.
>> +		      for pm8941: Default values are [00 01].
>> +		      for pmi8998: Default values are [00 01 02 03].
> 
> I would suggest omitting the defaults, as we can see in several places
> in this document we end up having to update the document with new
> defaults for each platform.
> 
> Also, per the defaults of the optional qcom,num-strings these are
> already the defaults...
> 
Sure. I will remove the defaults in the next series.
> [..]
>> +pmi8998-wled@d800 {
>> +	compatible = "qcom,pmi8998-wled";
>> +	reg = <0xd800 0xd900>;
>> +	label = "backlight";
>> +
>> +	interrupts = <3 0xd8 2 IRQ_TYPE_EDGE_RISING>,
>> +		     <3 0xd8 1 IRQ_TYPE_EDGE_RISING>;
>> +	interrupt-names = "short", "ovp";
>> +	qcom,current-limit-microamp = <25000>;
>> +	qcom,current-boost-limit = <805>;
>> +	qcom,switching-freq = <1600>;
>> +	qcom,ovp-millivolt = <29600>;
>> +	qcom,num-strings = <4>;
>> +	qcom,enabled-strings = <0x00 0x01 0x02 0x03>;
> 
> Please omit the pmi8998 example as well, there's no real benefit of
> adding similar examples for each platform.
> 
Sure. I will remove it.
> Regards,
> Bjorn
Rob Herring (Arm) June 20, 2018, 7:05 p.m. UTC | #3
On Tue, Jun 19, 2018 at 04:43:38PM +0530, Kiran Gunda wrote:
> Update the bindings with the new properties used for
> PMI8998.
> 
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> ---
>  .../bindings/leds/backlight/qcom-wled.txt          | 84 ++++++++++++++++++++--
>  1 file changed, 77 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
> index 14f28f2..503ce87 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
> @@ -48,11 +48,15 @@ platforms. The PMIC is connected to the host processor via SPMI bus.
>  - qcom,current-limit
>  	Usage:        optional
>  	Value type:   <u32>
> -	Definition:   mA; per-string current limit
> -		      value: For pm8941: from 0 to 25 with 5 mA step
> -			     Default 20 mA.
> -			     For pmi8998: from 0 to 30 with 5 mA step
> -			     Default 25 mA.
> +	Definition:   mA; per-string current limit; value from 0 to 25 with
> +		      1 mA step. Default 20 mA.
> +		      This property is supported only for pm8941.
> +
> +- qcom,current-limit-microamp
> +	Usage:        optional
> +	Value type:   <u32>
> +	Definition:   uA; per-string current limit; value from 0 to 30000 with
> +		      2500 uA step. Default 25000 uA.

This doesn't really seem worth adding just to add '-microamp'.


>  - qcom,current-boost-limit
>  	Usage:        optional
> @@ -79,12 +83,61 @@ platforms. The PMIC is connected to the host processor via SPMI bus.
>  		      27, 29, 32, 35. default: 29V
>  		      This property is supported only for PM8941.
>  
> +- qcom,ovp-millivolt

Is this the same as qcom,ovp? If so, same comment.

> +	Usage:        optional
> +	Value type:   <u32>
> +	Definition:   mV; Over-voltage protection limit;
> +		      For pmi8998: one of 18100, 19600, 29600, 31100
> +		      Default: 29600 mV
> +		      If this property is not specified for PM8941, it
> +		      falls back to "qcom,ovp" property.
> +
>  - qcom,num-strings
>  	Usage:        optional
>  	Value type:   <u32>
>  	Definition:   #; number of led strings attached;
> -		      value from 1 to 3. default: 2
> -		      This property is supported only for PM8941.
> +		      value: For PM8941 from 1 to 3. default: 2
> +			     For PMI8998 from 1 to 4. default: 4
> +
> +- interrupts
> +	Usage:        optional
> +	Value type:   <prop encoded array>
> +	Definition:   Interrupts associated with WLED. This should be
> +		      "short" and "ovp" interrupts. Interrupts can be
> +		      specified as per the encoding listed under
> +		      Documentation/devicetree/bindings/spmi/
> +		      qcom,spmi-pmic-arb.txt.
> +
> +- 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.
> +
> +- qcom,enabled-strings
> +	Usage:        optional
> +	Value tyoe:   <u32 array>
> +	Definition:   Array of the WLED strings numbered from 0 to 3. Each
> +		      string of leds are operated individually. Specify the
> +		      list of strings used by the device. Any combination of
> +		      led strings can be used.
> +		      for pm8941: Default values are [00 01].
> +		      for pmi8998: Default values are [00 01 02 03].

u32 or u8 because dts syntax for 8-bit array is [].

> +
> +- qcom,external-pfet
> +	Usage:        optional
> +	Value type:   <bool>
> +	Definition:   Specify if external PFET control for short circuit
> +		      protection is used. This property is supported only
> +		      for PMI8998.
> +
> +- qcom,auto-string-detection
> +	Usage:        optional
> +	Value type:   <bool>
> +	Definition:   Enables auto-detection of the WLED string configuration.
> +		      This feature is not supported for PM8941.
> +
>  
>  Example:
>  
> @@ -99,4 +152,21 @@ pm8941-wled@d800 {
>  	qcom,switching-freq = <1600>;
>  	qcom,ovp = <29>;
>  	qcom,num-strings = <2>;
> +	qcom,enabled-strings = <0x00 0x01>;
> +};
> +
> +pmi8998-wled@d800 {

led-controller {

And needs a unit-address.

> +	compatible = "qcom,pmi8998-wled";
> +	reg = <0xd800 0xd900>;
> +	label = "backlight";
> +
> +	interrupts = <3 0xd8 2 IRQ_TYPE_EDGE_RISING>,
> +		     <3 0xd8 1 IRQ_TYPE_EDGE_RISING>;
> +	interrupt-names = "short", "ovp";
> +	qcom,current-limit-microamp = <25000>;
> +	qcom,current-boost-limit = <805>;
> +	qcom,switching-freq = <1600>;
> +	qcom,ovp-millivolt = <29600>;
> +	qcom,num-strings = <4>;
> +	qcom,enabled-strings = <0x00 0x01 0x02 0x03>;
>  };
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>  a Linux Foundation Collaborative Project
>
Kiran Gunda June 21, 2018, 5:14 a.m. UTC | #4
On 2018-06-21 00:35, Rob Herring wrote: 

> On Tue, Jun 19, 2018 at 04:43:38PM +0530, Kiran Gunda wrote: 
> 
>> Update the bindings with the new properties used for
>> PMI8998.
>> 
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> ---
>> .../bindings/leds/backlight/qcom-wled.txt          | 84 ++++++++++++++++++++--
>> 1 file changed, 77 insertions(+), 7 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>> index 14f28f2..503ce87 100644
>> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>> @@ -48,11 +48,15 @@ platforms. The PMIC is connected to the host processor via SPMI bus.
>> - qcom,current-limit
>> Usage:        optional
>> Value type:   <u32>
>> -    Definition:   mA; per-string current limit
>> -              value: For pm8941: from 0 to 25 with 5 mA step
>> -                 Default 20 mA.
>> -                 For pmi8998: from 0 to 30 with 5 mA step
>> -                 Default 25 mA.
>> +    Definition:   mA; per-string current limit; value from 0 to 25 with
>> +              1 mA step. Default 20 mA.
>> +              This property is supported only for pm8941.
>> +
>> +- qcom,current-limit-microamp
>> +    Usage:        optional
>> +    Value type:   <u32>
>> +    Definition:   uA; per-string current limit; value from 0 to 30000 with
>> +              2500 uA step. Default 25000 uA.
> 
> This doesn't really seem worth adding just to add '-microamp'.
> Thanks for reviewing it!. I added this because the step value for PM8941(WLED3) and PMI8998(WLED4)

> are different. for WLED3 the step is 5mA and for WLED4 the step is 2.5mA. To mantain

> the backward compatibility i have added the new property with out modifying the existing

> one (qcom,current-limit).
> 
>> - qcom,current-boost-limit
>> Usage:        optional
>> @@ -79,12 +83,61 @@ platforms. The PMIC is connected to the host processor via SPMI bus.
>> 27, 29, 32, 35. default: 29V
>> This property is supported only for PM8941.
>> 
>> +- qcom,ovp-millivolt
> 
> Is this the same as qcom,ovp? If so, same comment.

> Yes. It is same. WLED3 has the OVP values 27V, 29V, 32V, 35V, where as

> WELD4 has 18.1V, 19.6V, 29.6V, 31.1V.
> 
>> +    Usage:        optional
>> +    Value type:   <u32>
>> +    Definition:   mV; Over-voltage protection limit;
>> +              For pmi8998: one of 18100, 19600, 29600, 31100
>> +              Default: 29600 mV
>> +              If this property is not specified for PM8941, it
>> +              falls back to "qcom,ovp" property.
>> +
>> - qcom,num-strings
>> Usage:        optional
>> Value type:   <u32>
>> Definition:   #; number of led strings attached;
>> -              value from 1 to 3. default: 2
>> -              This property is supported only for PM8941.
>> +              value: For PM8941 from 1 to 3. default: 2
>> +                 For PMI8998 from 1 to 4. default: 4
>> +
>> +- interrupts
>> +    Usage:        optional
>> +    Value type:   <prop encoded array>
>> +    Definition:   Interrupts associated with WLED. This should be
>> +              "short" and "ovp" interrupts. Interrupts can be
>> +              specified as per the encoding listed under
>> +              Documentation/devicetree/bindings/spmi/
>> +              qcom,spmi-pmic-arb.txt.
>> +
>> +- 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.
>> +
>> +- qcom,enabled-strings
>> +    Usage:        optional
>> +    Value tyoe:   <u32 array>
>> +    Definition:   Array of the WLED strings numbered from 0 to 3. Each
>> +              string of leds are operated individually. Specify the
>> +              list of strings used by the device. Any combination of
>> +              led strings can be used.
>> +              for pm8941: Default values are [00 01].
>> +              for pmi8998: Default values are [00 01 02 03].
> 
> u32 or u8 because dts syntax for 8-bit array is [].

> It is u32. I will correct dts syntax in next series as <0x00 0x01 0x02 0x03>,

> which is mentioned in the example.

+
+- qcom,external-pfet
+    Usage:        optional
+    Value type:   <bool>
+    Definition:   Specify if external PFET control for short circuit
+              protection is used. This property is supported only
+              for PMI8998.
+
+- qcom,auto-string-detection
+    Usage:        optional
+    Value type:   <bool>
+    Definition:   Enables auto-detection of the WLED string
configuration.
+              This feature is not supported for PM8941.
+

 Example:

@@ -99,4 +152,21 @@ pm8941-wled@d800 {
     qcom,switching-freq = <1600>;
     qcom,ovp = <29>;
     qcom,num-strings = <2>;
+    qcom,enabled-strings = <0x00 0x01>;
+};
+
+pmi8998-wled@d800 {
led-controller {

And needs a unit-address. 
Ok. Will modify as per your suggestion in the next series.

+    compatible = "qcom,pmi8998-wled";
+    reg = <0xd800 0xd900>;
+    label = "backlight";
+
+    interrupts = <3 0xd8 2 IRQ_TYPE_EDGE_RISING>,
+             <3 0xd8 1 IRQ_TYPE_EDGE_RISING>;
+    interrupt-names = "short", "ovp";
+    qcom,current-limit-microamp = <25000>;
+    qcom,current-boost-limit = <805>;
+    qcom,switching-freq = <1600>;
+    qcom,ovp-millivolt = <29600>;
+    qcom,num-strings = <4>;
+    qcom,enabled-strings = <0x00 0x01 0x02 0x03>;
 };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
 a Linux Foundation Collaborative Project
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
index 14f28f2..503ce87 100644
--- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
@@ -48,11 +48,15 @@  platforms. The PMIC is connected to the host processor via SPMI bus.
 - qcom,current-limit
 	Usage:        optional
 	Value type:   <u32>
-	Definition:   mA; per-string current limit
-		      value: For pm8941: from 0 to 25 with 5 mA step
-			     Default 20 mA.
-			     For pmi8998: from 0 to 30 with 5 mA step
-			     Default 25 mA.
+	Definition:   mA; per-string current limit; value from 0 to 25 with
+		      1 mA step. Default 20 mA.
+		      This property is supported only for pm8941.
+
+- qcom,current-limit-microamp
+	Usage:        optional
+	Value type:   <u32>
+	Definition:   uA; per-string current limit; value from 0 to 30000 with
+		      2500 uA step. Default 25000 uA.
 
 - qcom,current-boost-limit
 	Usage:        optional
@@ -79,12 +83,61 @@  platforms. The PMIC is connected to the host processor via SPMI bus.
 		      27, 29, 32, 35. default: 29V
 		      This property is supported only for PM8941.
 
+- qcom,ovp-millivolt
+	Usage:        optional
+	Value type:   <u32>
+	Definition:   mV; Over-voltage protection limit;
+		      For pmi8998: one of 18100, 19600, 29600, 31100
+		      Default: 29600 mV
+		      If this property is not specified for PM8941, it
+		      falls back to "qcom,ovp" property.
+
 - qcom,num-strings
 	Usage:        optional
 	Value type:   <u32>
 	Definition:   #; number of led strings attached;
-		      value from 1 to 3. default: 2
-		      This property is supported only for PM8941.
+		      value: For PM8941 from 1 to 3. default: 2
+			     For PMI8998 from 1 to 4. default: 4
+
+- interrupts
+	Usage:        optional
+	Value type:   <prop encoded array>
+	Definition:   Interrupts associated with WLED. This should be
+		      "short" and "ovp" interrupts. Interrupts can be
+		      specified as per the encoding listed under
+		      Documentation/devicetree/bindings/spmi/
+		      qcom,spmi-pmic-arb.txt.
+
+- 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.
+
+- qcom,enabled-strings
+	Usage:        optional
+	Value tyoe:   <u32 array>
+	Definition:   Array of the WLED strings numbered from 0 to 3. Each
+		      string of leds are operated individually. Specify the
+		      list of strings used by the device. Any combination of
+		      led strings can be used.
+		      for pm8941: Default values are [00 01].
+		      for pmi8998: Default values are [00 01 02 03].
+
+- qcom,external-pfet
+	Usage:        optional
+	Value type:   <bool>
+	Definition:   Specify if external PFET control for short circuit
+		      protection is used. This property is supported only
+		      for PMI8998.
+
+- qcom,auto-string-detection
+	Usage:        optional
+	Value type:   <bool>
+	Definition:   Enables auto-detection of the WLED string configuration.
+		      This feature is not supported for PM8941.
+
 
 Example:
 
@@ -99,4 +152,21 @@  pm8941-wled@d800 {
 	qcom,switching-freq = <1600>;
 	qcom,ovp = <29>;
 	qcom,num-strings = <2>;
+	qcom,enabled-strings = <0x00 0x01>;
+};
+
+pmi8998-wled@d800 {
+	compatible = "qcom,pmi8998-wled";
+	reg = <0xd800 0xd900>;
+	label = "backlight";
+
+	interrupts = <3 0xd8 2 IRQ_TYPE_EDGE_RISING>,
+		     <3 0xd8 1 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "short", "ovp";
+	qcom,current-limit-microamp = <25000>;
+	qcom,current-boost-limit = <805>;
+	qcom,switching-freq = <1600>;
+	qcom,ovp-millivolt = <29600>;
+	qcom,num-strings = <4>;
+	qcom,enabled-strings = <0x00 0x01 0x02 0x03>;
 };