diff mbox series

[v14,1/2] dt-bindings: leds: Add Qualcomm Light Pulse Generator binding

Message ID 20220303214300.59468-1-bjorn.andersson@linaro.org
State New
Headers show
Series [v14,1/2] dt-bindings: leds: Add Qualcomm Light Pulse Generator binding | expand

Commit Message

Bjorn Andersson March 3, 2022, 9:42 p.m. UTC
This adds the binding document describing the three hardware blocks
related to the Light Pulse Generator found in a wide range of Qualcomm
PMICs.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---

Changes since v13:
- None

Changes since v12:
- None

 .../bindings/leds/leds-qcom-lpg.yaml          | 173 ++++++++++++++++++
 1 file changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml

Comments

Doug Anderson March 3, 2022, 10:10 p.m. UTC | #1
Hi,

On Thu, Mar 3, 2022 at 1:41 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> The Light Pulse Generator (LPG) is a PWM-block found in a wide range of
> PMICs from Qualcomm. These PMICs typically comes with 1-8 LPG instances,
> with their output being routed to various other components, such as
> current sinks or GPIOs.
>
> Each LPG instance can operate on fixed parameters or based on a shared
> lookup-table, altering the duty cycle over time. This provides the means
> for hardware assisted transitions of LED brightness.
>
> A typical use case for the fixed parameter mode is to drive a PWM
> backlight control signal, the driver therefor allows each LPG instance
> to be exposed to the kernel either through the LED framework or the PWM
> framework.
>
> A typical use case for the LED configuration is to drive RGB LEDs in
> smartphones etc, for which the driver supports multiple channels to be
> ganged up to a MULTICOLOR LED. In this configuration the pattern
> generators will be synchronized, to allow for multi-color patterns.
>
> The idea of modelling this as a LED driver ontop of a PWM driver was
> considered, but setting the properties related to patterns does not fit
> in the PWM API. Similarly the idea of just duplicating the lower bits in
> a PWM and LED driver separately was considered, but this would not allow
> the PWM channels and LEDs to be configured on a per-board basis. The
> driver implements the more complex LED interface, and provides a PWM
> interface on the side of that, in the same driver.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v13:
> - Fixed mixed space/tab indentation in documentation
> - Added 0 as to lpg_clk_rates[] to match the hardware state, to avoid + 1 in
>   lpg_apply_freq() and - 1 in lpg_pwm_get_state()
> - Don't divide with 0 if current clock is 0 in lpg_pwm_get_state(), just return
>   period = duty = 0 in this case
> - Renamed "clk" in struct lpg_channel to clk_sel
> - Renamed "pre_div" in struct lpg_channel to pre_div_sel
>
> Changes since v12:
> - Initialize ret in lpg_pwm_apply()
>
> Changes since v11:
> - Extended commit message to cover decision to put pwm_chip in the LED driver
> - Added Documentation, in particular for the hw_pattern format
> - Added a lock to synchronize requests from LED and PWM frameworks
> - Turned out that the 9bit selector differs per channel in some PMICs, so
>   replaced bitmask in lpg_data with lookup based on QPNP SUBTYPE
> - Fixed kerneldoc for the struct device pointer in struct lpg
> - Rewrote conditional in lut_free() to make it easier to read
> - Corrected and deduplicated max_period expression in lpg_calc_freq()
> - Extended nom/dom to numerator/denominator in lpg_calc_freq()
> - Replaced 1 << 9 with LPG_RESOLUTION in one more place in lpg_calc_freq()
> - Use FIELD_PREP() in lpg_apply_freq() as masks was introduced for reading the
>   same in get_state()
> - Cleaned up the pattern format, to allow specifying both low and high pause
>   with and without pingpong mode.
> - Only update frequency and pwm_value if PWM channel is enabled in lpg_pwm_apply
> - Make lpg_pwm_get_state() read the hardware state, in order to pick up e.g.
>   bootloader backlight configuration
> - Use devm_bitmap_zalloc() to allocate the lut_bitmap
> - Use dev_err_probe() in lpg_probe()
> - Extended Kconfig help text to mention module name and satisfy checkpatch
>
>  Documentation/leds/leds-qcom-lpg.rst |   76 ++
>  drivers/leds/Kconfig                 |    3 +
>  drivers/leds/Makefile                |    3 +
>  drivers/leds/rgb/Kconfig             |   18 +
>  drivers/leds/rgb/Makefile            |    3 +
>  drivers/leds/rgb/leds-qcom-lpg.c     | 1405 ++++++++++++++++++++++++++
>  6 files changed, 1508 insertions(+)

Gets rid of the KASAN error and PWM still works for me, so happy to add back:

Tested-by: Douglas Anderson <dianders@chromium.org>

I haven't done a full review of the driver but I did a once-over of
the changes between v12 and v13 and they look good to me.

-Doug
Doug Anderson April 6, 2022, 3:18 p.m. UTC | #2
Hi Pavel,

On Thu, Mar 3, 2022 at 2:10 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Mar 3, 2022 at 1:41 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > The Light Pulse Generator (LPG) is a PWM-block found in a wide range of
> > PMICs from Qualcomm. These PMICs typically comes with 1-8 LPG instances,
> > with their output being routed to various other components, such as
> > current sinks or GPIOs.
> >
> > Each LPG instance can operate on fixed parameters or based on a shared
> > lookup-table, altering the duty cycle over time. This provides the means
> > for hardware assisted transitions of LED brightness.
> >
> > A typical use case for the fixed parameter mode is to drive a PWM
> > backlight control signal, the driver therefor allows each LPG instance
> > to be exposed to the kernel either through the LED framework or the PWM
> > framework.
> >
> > A typical use case for the LED configuration is to drive RGB LEDs in
> > smartphones etc, for which the driver supports multiple channels to be
> > ganged up to a MULTICOLOR LED. In this configuration the pattern
> > generators will be synchronized, to allow for multi-color patterns.
> >
> > The idea of modelling this as a LED driver ontop of a PWM driver was
> > considered, but setting the properties related to patterns does not fit
> > in the PWM API. Similarly the idea of just duplicating the lower bits in
> > a PWM and LED driver separately was considered, but this would not allow
> > the PWM channels and LEDs to be configured on a per-board basis. The
> > driver implements the more complex LED interface, and provides a PWM
> > interface on the side of that, in the same driver.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v13:
> > - Fixed mixed space/tab indentation in documentation
> > - Added 0 as to lpg_clk_rates[] to match the hardware state, to avoid + 1 in
> >   lpg_apply_freq() and - 1 in lpg_pwm_get_state()
> > - Don't divide with 0 if current clock is 0 in lpg_pwm_get_state(), just return
> >   period = duty = 0 in this case
> > - Renamed "clk" in struct lpg_channel to clk_sel
> > - Renamed "pre_div" in struct lpg_channel to pre_div_sel
> >
> > Changes since v12:
> > - Initialize ret in lpg_pwm_apply()
> >
> > Changes since v11:
> > - Extended commit message to cover decision to put pwm_chip in the LED driver
> > - Added Documentation, in particular for the hw_pattern format
> > - Added a lock to synchronize requests from LED and PWM frameworks
> > - Turned out that the 9bit selector differs per channel in some PMICs, so
> >   replaced bitmask in lpg_data with lookup based on QPNP SUBTYPE
> > - Fixed kerneldoc for the struct device pointer in struct lpg
> > - Rewrote conditional in lut_free() to make it easier to read
> > - Corrected and deduplicated max_period expression in lpg_calc_freq()
> > - Extended nom/dom to numerator/denominator in lpg_calc_freq()
> > - Replaced 1 << 9 with LPG_RESOLUTION in one more place in lpg_calc_freq()
> > - Use FIELD_PREP() in lpg_apply_freq() as masks was introduced for reading the
> >   same in get_state()
> > - Cleaned up the pattern format, to allow specifying both low and high pause
> >   with and without pingpong mode.
> > - Only update frequency and pwm_value if PWM channel is enabled in lpg_pwm_apply
> > - Make lpg_pwm_get_state() read the hardware state, in order to pick up e.g.
> >   bootloader backlight configuration
> > - Use devm_bitmap_zalloc() to allocate the lut_bitmap
> > - Use dev_err_probe() in lpg_probe()
> > - Extended Kconfig help text to mention module name and satisfy checkpatch
> >
> >  Documentation/leds/leds-qcom-lpg.rst |   76 ++
> >  drivers/leds/Kconfig                 |    3 +
> >  drivers/leds/Makefile                |    3 +
> >  drivers/leds/rgb/Kconfig             |   18 +
> >  drivers/leds/rgb/Makefile            |    3 +
> >  drivers/leds/rgb/leds-qcom-lpg.c     | 1405 ++++++++++++++++++++++++++
> >  6 files changed, 1508 insertions(+)
>
> Gets rid of the KASAN error and PWM still works for me, so happy to add back:
>
> Tested-by: Douglas Anderson <dianders@chromium.org>
>
> I haven't done a full review of the driver but I did a once-over of
> the changes between v12 and v13 and they look good to me.

With v5.18-rc1 released, this seems like it would be an ideal time to
land this driver and its bindings in a for-next branch for the leds
subsystem. Is there anything blocking it? Are you the right person to
land them? Ideally the bindings / driver (patch #1 and #2) from
Satya's series [1] could land right atop it since it's ready too?

[1] https://lore.kernel.org/r/1645509309-16142-1-git-send-email-quic_c_skakit@quicinc.com/

Thanks!

-Doug
Doug Anderson April 28, 2022, 4:42 p.m. UTC | #3
Hi Pavel,

On Wed, Apr 6, 2022 at 8:18 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi Pavel,
>
> On Thu, Mar 3, 2022 at 2:10 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Thu, Mar 3, 2022 at 1:41 PM Bjorn Andersson
> > <bjorn.andersson@linaro.org> wrote:
> > >
> > > The Light Pulse Generator (LPG) is a PWM-block found in a wide range of
> > > PMICs from Qualcomm. These PMICs typically comes with 1-8 LPG instances,
> > > with their output being routed to various other components, such as
> > > current sinks or GPIOs.
> > >
> > > Each LPG instance can operate on fixed parameters or based on a shared
> > > lookup-table, altering the duty cycle over time. This provides the means
> > > for hardware assisted transitions of LED brightness.
> > >
> > > A typical use case for the fixed parameter mode is to drive a PWM
> > > backlight control signal, the driver therefor allows each LPG instance
> > > to be exposed to the kernel either through the LED framework or the PWM
> > > framework.
> > >
> > > A typical use case for the LED configuration is to drive RGB LEDs in
> > > smartphones etc, for which the driver supports multiple channels to be
> > > ganged up to a MULTICOLOR LED. In this configuration the pattern
> > > generators will be synchronized, to allow for multi-color patterns.
> > >
> > > The idea of modelling this as a LED driver ontop of a PWM driver was
> > > considered, but setting the properties related to patterns does not fit
> > > in the PWM API. Similarly the idea of just duplicating the lower bits in
> > > a PWM and LED driver separately was considered, but this would not allow
> > > the PWM channels and LEDs to be configured on a per-board basis. The
> > > driver implements the more complex LED interface, and provides a PWM
> > > interface on the side of that, in the same driver.
> > >
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > ---
> > >
> > > Changes since v13:
> > > - Fixed mixed space/tab indentation in documentation
> > > - Added 0 as to lpg_clk_rates[] to match the hardware state, to avoid + 1 in
> > >   lpg_apply_freq() and - 1 in lpg_pwm_get_state()
> > > - Don't divide with 0 if current clock is 0 in lpg_pwm_get_state(), just return
> > >   period = duty = 0 in this case
> > > - Renamed "clk" in struct lpg_channel to clk_sel
> > > - Renamed "pre_div" in struct lpg_channel to pre_div_sel
> > >
> > > Changes since v12:
> > > - Initialize ret in lpg_pwm_apply()
> > >
> > > Changes since v11:
> > > - Extended commit message to cover decision to put pwm_chip in the LED driver
> > > - Added Documentation, in particular for the hw_pattern format
> > > - Added a lock to synchronize requests from LED and PWM frameworks
> > > - Turned out that the 9bit selector differs per channel in some PMICs, so
> > >   replaced bitmask in lpg_data with lookup based on QPNP SUBTYPE
> > > - Fixed kerneldoc for the struct device pointer in struct lpg
> > > - Rewrote conditional in lut_free() to make it easier to read
> > > - Corrected and deduplicated max_period expression in lpg_calc_freq()
> > > - Extended nom/dom to numerator/denominator in lpg_calc_freq()
> > > - Replaced 1 << 9 with LPG_RESOLUTION in one more place in lpg_calc_freq()
> > > - Use FIELD_PREP() in lpg_apply_freq() as masks was introduced for reading the
> > >   same in get_state()
> > > - Cleaned up the pattern format, to allow specifying both low and high pause
> > >   with and without pingpong mode.
> > > - Only update frequency and pwm_value if PWM channel is enabled in lpg_pwm_apply
> > > - Make lpg_pwm_get_state() read the hardware state, in order to pick up e.g.
> > >   bootloader backlight configuration
> > > - Use devm_bitmap_zalloc() to allocate the lut_bitmap
> > > - Use dev_err_probe() in lpg_probe()
> > > - Extended Kconfig help text to mention module name and satisfy checkpatch
> > >
> > >  Documentation/leds/leds-qcom-lpg.rst |   76 ++
> > >  drivers/leds/Kconfig                 |    3 +
> > >  drivers/leds/Makefile                |    3 +
> > >  drivers/leds/rgb/Kconfig             |   18 +
> > >  drivers/leds/rgb/Makefile            |    3 +
> > >  drivers/leds/rgb/leds-qcom-lpg.c     | 1405 ++++++++++++++++++++++++++
> > >  6 files changed, 1508 insertions(+)
> >
> > Gets rid of the KASAN error and PWM still works for me, so happy to add back:
> >
> > Tested-by: Douglas Anderson <dianders@chromium.org>
> >
> > I haven't done a full review of the driver but I did a once-over of
> > the changes between v12 and v13 and they look good to me.
>
> With v5.18-rc1 released, this seems like it would be an ideal time to
> land this driver and its bindings in a for-next branch for the leds
> subsystem. Is there anything blocking it? Are you the right person to
> land them? Ideally the bindings / driver (patch #1 and #2) from
> Satya's series [1] could land right atop it since it's ready too?
>
> [1] https://lore.kernel.org/r/1645509309-16142-1-git-send-email-quic_c_skakit@quicinc.com/

I don't mean to be a huge pest, but we're already at v5.18-rc4 (almost
at -rc5) and these two series are still pending. I'm worried that
we're going to miss the window to land them again. Can you give any
update about them?

Thanks!

-Doug
Marijn Suijten April 30, 2022, 10:06 a.m. UTC | #4
On 2022-03-03 13:42:59, Bjorn Andersson wrote:
> This adds the binding document describing the three hardware blocks
> related to the Light Pulse Generator found in a wide range of Qualcomm
> PMICs.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

Apart the nits below:

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> 
> Changes since v13:
> - None
> 
> Changes since v12:
> - None
> 
>  .../bindings/leds/leds-qcom-lpg.yaml          | 173 ++++++++++++++++++
>  1 file changed, 173 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> new file mode 100644
> index 000000000000..336bd8e10efd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> @@ -0,0 +1,173 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-qcom-lpg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Light Pulse Generator
> +
> +maintainers:
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> +
> +description: >
> +  The Qualcomm Light Pulse Generator consists of three different hardware blocks;
> +  a ramp generator with lookup table, the light pulse generator and a three
> +  channel current sink. These blocks are found in a wide range of Qualcomm PMICs.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,pm8150b-lpg
> +      - qcom,pm8150l-lpg
> +      - qcom,pm8916-pwm
> +      - qcom,pm8941-lpg
> +      - qcom,pm8994-lpg
> +      - qcom,pmc8180c-lpg
> +      - qcom,pmi8994-lpg
> +      - qcom,pmi8998-lpg
> +
> +  "#pwm-cells":
> +    const: 2
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  qcom,power-source:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      power-source used to drive the output, as defined in the datasheet.
> +      Should be specified if the TRILED block is present
> +    enum: [0, 1, 3]
> +
> +  qcom,dtest:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description: >
> +      A list of integer pairs, where each pair represent the dtest line the
> +      particular channel should be connected to and the flags denoting how the
> +      value should be outputed, as defined in the datasheet. The number of

Nit: I think outputed is with double-t, though just using "output" here
should work as well.

> +      pairs should be the same as the number of channels.
> +    items:
> +      items:
> +        - description: dtest line to attach
> +        - description: flags for the attachment
> +
> +  multi-led:

As mentioned on IRC, doesn't this need an @[0-9a-f] designator, as
specified in leds-class-multicolor.yaml and to match `#address-cells` on
this level?  Same for the examples.

> +    type: object
> +    $ref: leds-class-multicolor.yaml#
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +    patternProperties:
> +      "^led@[0-9a-f]$":
> +        type: object
> +        $ref: common.yaml#

I'm not familiar enough with DT bindings - doesn't this need the
requirements for `reg` just like the identical patternProperties for
led@ below?

> +
> +patternProperties:
> +  "^led@[0-9a-f]$":
> +    type: object
> +    $ref: common.yaml#
> +
> +    properties:
> +      reg: true
> +
> +    required:
> +      - reg
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    led-controller {
> +      compatible = "qcom,pmi8994-lpg";
> +
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      qcom,power-source = <1>;
> +
> +      qcom,dtest = <0 0>,
> +                   <0 0>,
> +                   <0 0>,
> +                   <4 1>;
> +
> +      led@1 {
> +        reg = <1>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        function = LED_FUNCTION_INDICATOR;
> +        function-enumerator = <1>;
> +      };
> +
> +      led@2 {
> +        reg = <2>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        function = LED_FUNCTION_INDICATOR;
> +        function-enumerator = <0>;
> +        default-state = "on";
> +      };
> +
> +      led@3 {
> +        reg = <3>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        function = LED_FUNCTION_INDICATOR;
> +        function-enumerator = <2>;
> +      };
> +
> +      led@4 {
> +        reg = <4>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        function = LED_FUNCTION_INDICATOR;
> +        function-enumerator = <3>;
> +      };
> +    };
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    led-controller {
> +      compatible = "qcom,pmi8994-lpg";
> +
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      qcom,power-source = <1>;
> +
> +      multi-led {
> +        color = <LED_COLOR_ID_RGB>;
> +        function = LED_FUNCTION_STATUS;
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        led@1 {
> +          reg = <1>;
> +          color = <LED_COLOR_ID_RED>;
> +        };
> +
> +        led@2 {
> +          reg = <2>;
> +          color = <LED_COLOR_ID_GREEN>;
> +        };
> +
> +        led@3 {
> +          reg = <3>;
> +          color = <LED_COLOR_ID_BLUE>;
> +        };
> +      };
> +    };
> +  - |
> +    pwm-controller {
> +      compatible = "qcom,pm8916-pwm";
> +      #pwm-cells = <2>;
> +    };
> +...
> -- 
> 2.33.1
>
Marijn Suijten April 30, 2022, 12:13 p.m. UTC | #5
On 2022-03-03 13:42:59, Bjorn Andersson wrote:
> This adds the binding document describing the three hardware blocks
> related to the Light Pulse Generator found in a wide range of Qualcomm
> PMICs.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> 
> Changes since v13:
> - None
> 
> Changes since v12:
> - None
> 
>  .../bindings/leds/leds-qcom-lpg.yaml          | 173 ++++++++++++++++++
>  1 file changed, 173 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> new file mode 100644
> index 000000000000..336bd8e10efd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> @@ -0,0 +1,173 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-qcom-lpg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Light Pulse Generator
> +
> +maintainers:
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> +
> +description: >
> +  The Qualcomm Light Pulse Generator consists of three different hardware blocks;
> +  a ramp generator with lookup table, the light pulse generator and a three
> +  channel current sink. These blocks are found in a wide range of Qualcomm PMICs.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,pm8150b-lpg
> +      - qcom,pm8150l-lpg
> +      - qcom,pm8916-pwm
> +      - qcom,pm8941-lpg
> +      - qcom,pm8994-lpg
> +      - qcom,pmc8180c-lpg
> +      - qcom,pmi8994-lpg
> +      - qcom,pmi8998-lpg
> +
> +  "#pwm-cells":
> +    const: 2
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  qcom,power-source:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      power-source used to drive the output, as defined in the datasheet.
> +      Should be specified if the TRILED block is present

Upon closer inspection this is only true if the TRILED block also has a
SRC register which appears to be optional.  Is it worth mentioning that
here too?

- Marijn

> +    enum: [0, 1, 3]
> +
> +  qcom,dtest:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description: >
> +      A list of integer pairs, where each pair represent the dtest line the
> +      particular channel should be connected to and the flags denoting how the
> +      value should be outputed, as defined in the datasheet. The number of
> +      pairs should be the same as the number of channels.
> +    items:
> +      items:
> +        - description: dtest line to attach
> +        - description: flags for the attachment
> +
> +  multi-led:
> +    type: object
> +    $ref: leds-class-multicolor.yaml#
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +    patternProperties:
> +      "^led@[0-9a-f]$":
> +        type: object
> +        $ref: common.yaml#
> +
> +patternProperties:
> +  "^led@[0-9a-f]$":
> +    type: object
> +    $ref: common.yaml#
> +
> +    properties:
> +      reg: true
> +
> +    required:
> +      - reg
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    led-controller {
> +      compatible = "qcom,pmi8994-lpg";
> +
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      qcom,power-source = <1>;
> +
> +      qcom,dtest = <0 0>,
> +                   <0 0>,
> +                   <0 0>,
> +                   <4 1>;
> +
> +      led@1 {
> +        reg = <1>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        function = LED_FUNCTION_INDICATOR;
> +        function-enumerator = <1>;
> +      };
> +
> +      led@2 {
> +        reg = <2>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        function = LED_FUNCTION_INDICATOR;
> +        function-enumerator = <0>;
> +        default-state = "on";
> +      };
> +
> +      led@3 {
> +        reg = <3>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        function = LED_FUNCTION_INDICATOR;
> +        function-enumerator = <2>;
> +      };
> +
> +      led@4 {
> +        reg = <4>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        function = LED_FUNCTION_INDICATOR;
> +        function-enumerator = <3>;
> +      };
> +    };
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    led-controller {
> +      compatible = "qcom,pmi8994-lpg";
> +
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      qcom,power-source = <1>;
> +
> +      multi-led {
> +        color = <LED_COLOR_ID_RGB>;
> +        function = LED_FUNCTION_STATUS;
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        led@1 {
> +          reg = <1>;
> +          color = <LED_COLOR_ID_RED>;
> +        };
> +
> +        led@2 {
> +          reg = <2>;
> +          color = <LED_COLOR_ID_GREEN>;
> +        };
> +
> +        led@3 {
> +          reg = <3>;
> +          color = <LED_COLOR_ID_BLUE>;
> +        };
> +      };
> +    };
> +  - |
> +    pwm-controller {
> +      compatible = "qcom,pm8916-pwm";
> +      #pwm-cells = <2>;
> +    };
> +...
> -- 
> 2.33.1
>
Pavel Machek May 4, 2022, 7:24 a.m. UTC | #6
Hi!

> This adds the binding document describing the three hardware blocks
> related to the Light Pulse Generator found in a wide range of Qualcomm
> PMICs.

Sorry for the delays. I have collected tested/review tags and push the
result to:

To gitolite.kernel.org:pub/scm/linux/kernel/git/pavel/linux-leds.git
   312310928417..24e2d05d1b68  for-next -> for-next

I'll need to check pattern usage in the driver, and there are some
small fixes needed as evidenced in the reviews.

Best regards,
								Pavel
Bjorn Andersson May 4, 2022, 4:21 p.m. UTC | #7
On Wed 04 May 00:24 PDT 2022, Pavel Machek wrote:

> Hi!
> 
> > This adds the binding document describing the three hardware blocks
> > related to the Light Pulse Generator found in a wide range of Qualcomm
> > PMICs.
> 
> Sorry for the delays. I have collected tested/review tags and push the
> result to:
> 
> To gitolite.kernel.org:pub/scm/linux/kernel/git/pavel/linux-leds.git
>    312310928417..24e2d05d1b68  for-next -> for-next
> 

Much appreciated, this will unblock a few different use cases for us -
perhaps the most important one backlight control on devices such as the
Lenovo Flex 5G :)

> I'll need to check pattern usage in the driver, and there are some
> small fixes needed as evidenced in the reviews.
> 

I will go through the Marijn's feedback in detail and am looking forward
to hear from you on the pattern front, and will look into preparing
incremental patches for the changes needed.

Thanks,
Bjorn
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
new file mode 100644
index 000000000000..336bd8e10efd
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
@@ -0,0 +1,173 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-qcom-lpg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Light Pulse Generator
+
+maintainers:
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+description: >
+  The Qualcomm Light Pulse Generator consists of three different hardware blocks;
+  a ramp generator with lookup table, the light pulse generator and a three
+  channel current sink. These blocks are found in a wide range of Qualcomm PMICs.
+
+properties:
+  compatible:
+    enum:
+      - qcom,pm8150b-lpg
+      - qcom,pm8150l-lpg
+      - qcom,pm8916-pwm
+      - qcom,pm8941-lpg
+      - qcom,pm8994-lpg
+      - qcom,pmc8180c-lpg
+      - qcom,pmi8994-lpg
+      - qcom,pmi8998-lpg
+
+  "#pwm-cells":
+    const: 2
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  qcom,power-source:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      power-source used to drive the output, as defined in the datasheet.
+      Should be specified if the TRILED block is present
+    enum: [0, 1, 3]
+
+  qcom,dtest:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description: >
+      A list of integer pairs, where each pair represent the dtest line the
+      particular channel should be connected to and the flags denoting how the
+      value should be outputed, as defined in the datasheet. The number of
+      pairs should be the same as the number of channels.
+    items:
+      items:
+        - description: dtest line to attach
+        - description: flags for the attachment
+
+  multi-led:
+    type: object
+    $ref: leds-class-multicolor.yaml#
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+    patternProperties:
+      "^led@[0-9a-f]$":
+        type: object
+        $ref: common.yaml#
+
+patternProperties:
+  "^led@[0-9a-f]$":
+    type: object
+    $ref: common.yaml#
+
+    properties:
+      reg: true
+
+    required:
+      - reg
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    led-controller {
+      compatible = "qcom,pmi8994-lpg";
+
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      qcom,power-source = <1>;
+
+      qcom,dtest = <0 0>,
+                   <0 0>,
+                   <0 0>,
+                   <4 1>;
+
+      led@1 {
+        reg = <1>;
+        color = <LED_COLOR_ID_GREEN>;
+        function = LED_FUNCTION_INDICATOR;
+        function-enumerator = <1>;
+      };
+
+      led@2 {
+        reg = <2>;
+        color = <LED_COLOR_ID_GREEN>;
+        function = LED_FUNCTION_INDICATOR;
+        function-enumerator = <0>;
+        default-state = "on";
+      };
+
+      led@3 {
+        reg = <3>;
+        color = <LED_COLOR_ID_GREEN>;
+        function = LED_FUNCTION_INDICATOR;
+        function-enumerator = <2>;
+      };
+
+      led@4 {
+        reg = <4>;
+        color = <LED_COLOR_ID_GREEN>;
+        function = LED_FUNCTION_INDICATOR;
+        function-enumerator = <3>;
+      };
+    };
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    led-controller {
+      compatible = "qcom,pmi8994-lpg";
+
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      qcom,power-source = <1>;
+
+      multi-led {
+        color = <LED_COLOR_ID_RGB>;
+        function = LED_FUNCTION_STATUS;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led@1 {
+          reg = <1>;
+          color = <LED_COLOR_ID_RED>;
+        };
+
+        led@2 {
+          reg = <2>;
+          color = <LED_COLOR_ID_GREEN>;
+        };
+
+        led@3 {
+          reg = <3>;
+          color = <LED_COLOR_ID_BLUE>;
+        };
+      };
+    };
+  - |
+    pwm-controller {
+      compatible = "qcom,pm8916-pwm";
+      #pwm-cells = <2>;
+    };
+...