mbox series

[V4,0/4] Add support for WLED5

Message ID 1584985618-25689-1-git-send-email-kgunda@codeaurora.org
Headers show
Series Add support for WLED5 | expand

Message

Kiran Gunda March 23, 2020, 5:46 p.m. UTC
Currently, WLED driver supports only WLED4 peripherals that is present
on pmi8998 and pm660L. This patch series  converts the existing WLED4
bindings from .txt to .yaml format and adds the support for WLED5 peripheral
that is present on PM8150L.

PM8150L WLED supports the following.
    - Two modulators and each sink can use any of the modulator
    - Multiple CABC selection options
    - Multiple brightness width selection (12 bits to 15 bits)

Changes from V1:
	- Rebased on top of the below commit.
	  backlight: qcom-wled: Fix unsigned comparison to zero

Changes from V2:
	- Addressed Bjorn's comments by splitting the WLED4 changes
	  in a seperate patch.
	- Added WLED5 auto calibration support

Changes from V3:
        - Addressed comments from Daniel Thompson and Rob Herring
        - Seperated the WLED5 bindings from the driver changes
        - Squashed wled5 auto string detection and wled5 basic changes
          to avoid the NULL callback function pointer issue.

Kiran Gunda (3):
  backlight: qcom-wled: convert the wled bindings to .yaml format
  backlight: qcom-wled: Add callback functions
  backlight: qcom-wled: Add WLED5 bindings

Subbaraman Narayanamurthy (1):
  backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L

 .../bindings/leds/backlight/qcom-wled.txt          | 154 ------
 .../bindings/leds/backlight/qcom-wled.yaml         | 223 ++++++++
 drivers/video/backlight/qcom-wled.c                | 613 ++++++++++++++++++---
 3 files changed, 764 insertions(+), 226 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml

Comments

Daniel Thompson March 25, 2020, 3:37 p.m. UTC | #1
On Mon, Mar 23, 2020 at 11:16:57PM +0530, Kiran Gunda wrote:
> Add WLED5 specific bindings.
> 
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> Signed-off-by: Subbaraman Narayanamurthy <subbaram@codeaurora.org>
> ---
>  .../bindings/leds/backlight/qcom-wled.yaml         | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> index 8a388bf..159115f 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> @@ -20,6 +20,7 @@ properties:
>         - qcom,pm8941-wled
>         - qcom,pmi8998-wled
>         - qcom,pm660l-wled
> +       - qcom,pm8150l-wled
>  
>    reg:
>      maxItems: 1
> @@ -28,10 +29,23 @@ properties:
>      maxItems: 1
>      description:
>        brightness value on boot, value from 0-4095.
> +      For pm8150l this value vary from 0-4095 or 0-32767
> +      depending on the brightness control mode. If CABC is
> +      enabled 0-4095 range is used.

I rather dislike some of the property descriptions using PMIC version
numbers to distinguish between peripheral versions and others using
WLEDx version numbers.

Could the property description be rephrased to use WLED3/4/5 terminology
instead?


>      allOf:
>        - $ref: /schemas/types.yaml#/definitions/uint32
>          default: 2048
>  
> +  max-brightness:
> +    maxItems: 1
> +    description:
> +      Maximum brightness level. Allowed values are,
> +      for pmi8998 it is  0-4095.
> +      For pm8150l, this can be either 4095 or 32767.
> +      If CABC is enabled, this is capped to 4095.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +

Similar thing here, is PMI8998 simply a synonym for WLED4 or there
something special about the PMIC versioning that requires it to be used?


Daniel.


>    label:
>      maxItems: 1
>      description:
> @@ -124,6 +138,31 @@ properties:
>        value for PM8941 from 1 to 3. Default 2
>        For PMI8998 from 1 to 4.
>  
> +  qcom,modulator-sel:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Selects the modulator used for brightness modulation.
> +      Allowed values are,
> +               0 - Modulator A
> +               1 - Modulator B
> +      If not specified, then modulator A will be used by default.
> +      This property is applicable only to WLED5 peripheral.
> +
> +  qcom,cabc-sel:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Selects the CABC pin signal used for brightness modulation.
> +      Allowed values are,
> +              0 - CABC disabled
> +              1 - CABC 1
> +              2 - CABC 2
> +              3 - External signal (e.g. LPG) is used for dimming
> +      This property is applicable only to WLED5 peripheral.
> +
>    interrupts:
>      maxItems: 2
>      description:
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>  a Linux Foundation Collaborative Project
Rob Herring March 31, 2020, 5:56 p.m. UTC | #2
On Mon, Mar 23, 2020 at 11:16:57PM +0530, Kiran Gunda wrote:
> Add WLED5 specific bindings.
> 

More of the same comments here...

> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> Signed-off-by: Subbaraman Narayanamurthy <subbaram@codeaurora.org>
> ---
>  .../bindings/leds/backlight/qcom-wled.yaml         | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> index 8a388bf..159115f 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> @@ -20,6 +20,7 @@ properties:
>         - qcom,pm8941-wled
>         - qcom,pmi8998-wled
>         - qcom,pm660l-wled
> +       - qcom,pm8150l-wled
>  
>    reg:
>      maxItems: 1
> @@ -28,10 +29,23 @@ properties:
>      maxItems: 1
>      description:
>        brightness value on boot, value from 0-4095.
> +      For pm8150l this value vary from 0-4095 or 0-32767
> +      depending on the brightness control mode. If CABC is
> +      enabled 0-4095 range is used.

Constraints.

>      allOf:
>        - $ref: /schemas/types.yaml#/definitions/uint32
>          default: 2048
>  
> +  max-brightness:
> +    maxItems: 1
> +    description:
> +      Maximum brightness level. Allowed values are,
> +      for pmi8998 it is  0-4095.
> +      For pm8150l, this can be either 4095 or 32767.

Constraints!

> +      If CABC is enabled, this is capped to 4095.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32

Standard property. Assume it has a type definition.'

> +
>    label:
>      maxItems: 1
>      description:
> @@ -124,6 +138,31 @@ properties:
>        value for PM8941 from 1 to 3. Default 2
>        For PMI8998 from 1 to 4.
>  
> +  qcom,modulator-sel:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Selects the modulator used for brightness modulation.
> +      Allowed values are,
> +               0 - Modulator A
> +               1 - Modulator B
> +      If not specified, then modulator A will be used by default.
> +      This property is applicable only to WLED5 peripheral.
> +
> +  qcom,cabc-sel:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Selects the CABC pin signal used for brightness modulation.
> +      Allowed values are,
> +              0 - CABC disabled
> +              1 - CABC 1
> +              2 - CABC 2
> +              3 - External signal (e.g. LPG) is used for dimming
> +      This property is applicable only to WLED5 peripheral.
> +
>    interrupts:
>      maxItems: 2
>      description:
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>  a Linux Foundation Collaborative Project
>
Kiran Gunda April 3, 2020, 11:15 a.m. UTC | #3
On 2020-03-31 23:24, Rob Herring wrote:
> On Mon, Mar 23, 2020 at 11:16:55PM +0530, Kiran Gunda wrote:
>> Convert the qcom-wled bindings from .txt to .yaml format.
>> 
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> Signed-off-by: Subbaraman Narayanamurthy <subbaram@codeaurora.org>
>> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
>> ---
>>  .../bindings/leds/backlight/qcom-wled.txt          | 154 
>> -----------------
>>  .../bindings/leds/backlight/qcom-wled.yaml         | 184 
>> +++++++++++++++++++++
>>  2 files changed, 184 insertions(+), 154 deletions(-)
>>  delete mode 100644 
>> Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>  create mode 100644 
>> Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> 
>> diff --git 
>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml 
>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> new file mode 100644
>> index 0000000..8a388bf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> @@ -0,0 +1,184 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/leds/backlight/qcom-wled.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Binding for Qualcomm Technologies, Inc. WLED driver
>> +
>> +maintainers:
>> +  - Lee Jones <lee.jones@linaro.org>
> 
> Should be the h/w owner (you), not who applies patches.
> 
will address in next post.
>> +
>> +description: |
>> +  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.
>> +
>> +properties:
>> +  compatible :
> 
> Drop the space ^
> 
will address in next post.
>> +    enum:
>> +       - qcom,pm8941-wled
>> +       - qcom,pmi8998-wled
>> +       - qcom,pm660l-wled
> 
> Wrong indent (1 space too many).
> 
will address in next post.
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  default-brightness:
>> +    maxItems: 1
> 
> maxItems is for arrays and this is a single scalar.
> 
>> +    description:
>> +      brightness value on boot, value from 0-4095.
> 
> 0-4095 sounds like a constraint.
> 
will address in next post.
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
> 
> There should be a common definition for this.
> 
will address in next post.
>> +        default: 2048
>> +
>> +  label:
>> +    maxItems: 1
>> +    description:
>> +      The name of the backlight device.
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/string
> 
> Already has a type. Just 'label: true' is enough.
> 
will address in next post.
>> +
>> +  qcom,cs-out:
>> +    description:
>> +      enable current sink output.
>> +      This property is supported only for PM8941.
>> +    type: boolean
>> +
>> +  qcom,cabc:
>> +    description:
>> +      enable content adaptive backlight control.
>> +    type: boolean
>> +
>> +  qcom,ext-gen:
>> +    description:
>> +      use externally generated modulator signal to dim.
>> +      This property is supported only for PM8941.
>> +    type: boolean
>> +
>> +  qcom,current-limit:
>> +    maxItems: 1
> 
> Not an array.
> 
will address in next post.
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      mA; per-string current limit; value from 0 to 25 with
> 
> 25 sounds like a constraint.
> 
will address in next post.
>> +      1 mA step. This property is supported only for pm8941.
>> +    default: 20
>> +
>> +  qcom,current-limit-microamp:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
> 
> properties with unit suffix already have a type.
> 
will address in next post.
>> +    description:
>> +      uA; per-string current limit; value from 0 to 30000 with
>> +      2500 uA step.
> 
> steps can be expressed using 'multipleOf'
> 
will address in next post.
>> +    default: 25
> 
> 25 can never be a multiple of 2500
> 
will correct in next post.
>> +
>> +  qcom,current-boost-limit:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      mA; boost current limit.
>> +      For pm8941 one of 105, 385, 525, 805, 980, 1260, 1400, 1680.
>> +      Default, 805 mA.
>> +      For pmi8998 one of 105, 280, 450, 620, 970, 1150, 1300,
>> +      1500. Default 970 mA.
> 
> Constraints.
> 
will address in next post.
>> +
>> +  qcom,switching-freq:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      kHz; switching frequency; one of 600, 640, 685, 738,
>> +      800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200,
>> +      4800, 9600.
>> +      Default for pm8941 1600 kHz
>> +               for pmi8998 800 kHz
> 
> Constraints!
> 
will address in next post.
>> +
>> +  qcom,ovp:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      V; Over-voltage protection limit; one of 27, 29, 32, 35. 
>> Default 29V
>> +      This property is supported only for PM8941.
>> +
>> +  qcom,ovp-millivolt:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      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:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      number of led strings attached;
>> +      value for PM8941 from 1 to 3. Default 2
>> +      For PMI8998 from 1 to 4.
>> +
>> +  interrupts:
>> +    maxItems: 2
> 
> items:
>   - description: ...
>   - description: ...
> 
will address in next post.
>> +    description:
>> +      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.
> 
> encoding is outside the scope of the binding.
> 
will address in next post.
>> +
>> +  interrupt-names:
>> +    description:
>> +      Interrupt names associated with the interrupts.
>> +      Must be "short" and "ovp". The short circuit detection
>> +      is not supported for PM8941.
> 
> Names should be constraints.
> 
will address in next post.
>> +
>> +  qcom,enabled-strings:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description:
>> +      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.
>> +
>> +  qcom,external-pfet:
>> +    description:
>> +      Specify if external PFET control for short circuit
>> +      protection is used. This property is supported only
>> +      for PMI8998.
>> +    type: boolean
>> +
>> +  qcom,auto-string-detection:
>> +    description:
>> +      Enables auto-detection of the WLED string configuration.
>> +      This feature is not supported for PM8941.
>> +    type: boolean
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - label
>> +
>> +examples:
>> +  - |
>> +    pm8941-wled@d800 {
> 
> backlight@...
> 
will add in next post.
>> +        compatible = "qcom,pm8941-wled";
>> +        reg = <0xd800 0x100>;
>> +        label = "backlight";
>> +
>> +        qcom,cs-out;
>> +        qcom,current-limit = <20>;
>> +        qcom,current-boost-limit = <805>;
>> +        qcom,switching-freq = <1600>;
>> +        qcom,ovp = <29>;
>> +        qcom,num-strings = <2>;
>> +        qcom,enabled-strings = <0 1>;
>> +     };
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>>  a Linux Foundation Collaborative Project
>>
Kiran Gunda April 13, 2020, 2:50 p.m. UTC | #4
On 2020-04-06 14:20, Lee Jones wrote:
> On Fri, 03 Apr 2020, Daniel Thompson wrote:
> 
>> On Fri, Apr 03, 2020 at 04:45:49PM +0530, kgunda@codeaurora.org wrote:
>> > On 2020-03-31 23:24, Rob Herring wrote:
>> > > On Mon, Mar 23, 2020 at 11:16:55PM +0530, Kiran Gunda wrote:
>> > > > diff --git
>> > > > a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> > > > b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> > > > new file mode 100644
>> > > > index 0000000..8a388bf
>> > > > --- /dev/null
>> > > > +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> > > > @@ -0,0 +1,184 @@
>> > > > +# SPDX-License-Identifier: GPL-2.0-only
>> > > > +%YAML 1.2
>> > > > +---
>> > > > +$id: http://devicetree.org/schemas/leds/backlight/qcom-wled.yaml#
>> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > > > +
>> > > > +title: Binding for Qualcomm Technologies, Inc. WLED driver
>> > > > +
>> > > > +maintainers:
>> > > > +  - Lee Jones <lee.jones@linaro.org>
>> > >
>> > > Should be the h/w owner (you), not who applies patches.
>> > >
>> > will address in next post.
>> > <snip>
>> > will address in next post.
>> > <snip>
>> > will address in next post.
>> > <snip>
>> > will address in next post.
>> > <snip>
>> > will address in next post.
>> > <snip>
>> > will address in next post.
>> > <snip>
>> > will address in next post.
>> > <snip>
>> > will address in next post.
>> > <snip>
>> > will address in next post.
>> 
>> If you agree on all points raised I doubt there is any need for a 
>> point
>> by point reply since everyone who reads it will have to scroll down
>> simply to find out that you agree on all points.
>> 
>> Better just to acknowledge the feedback and reply to the first one
>> saying you'll agree on all points and will address all feedback in the
>> next revision (and then trim the reply to keep it short).
> 
> Or better still, just submit the next revision with all the fixes. :)
Noted.