Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 2 Aug 2023 18:25:02 -0600
From:   Rob Herring <robh@kernel.org>
To:     Anjelique Melendez <quic_amelende@quicinc.com>
Cc:     pavel@ucw.cz, lee@kernel.org, thierry.reding@gmail.com,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        agross@kernel.org, andersson@kernel.org, luca.weiss@fairphone.com,
        konrad.dybcio@linaro.org, u.kleine-koenig@pengutronix.de,
        quic_subbaram@quicinc.com, linux-leds@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org, linux-pwm@vger.kernel.org
Subject: Re: [PATCH v2 2/7] dt-bindings: leds: leds-qcom-lpg: Add support for
 LPG PPG
Message-ID: <20230803002502.GA1569972-robh@kernel.org>
References: <20230725193423.25047-1-quic_amelende@quicinc.com>
 <20230725193423.25047-3-quic_amelende@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230725193423.25047-3-quic_amelende@quicinc.com>
Precedence: bulk

On Tue, Jul 25, 2023 at 12:34:18PM -0700, Anjelique Melendez wrote:
> Update leds-qcom-lpg bindings to support LPG PPG.
> 
> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
> ---
>  .../bindings/leds/leds-qcom-lpg.yaml          | 92 ++++++++++++++++++-
>  1 file changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> index e6f1999cb22f..6feca859fb74 100644
> --- a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml
> @@ -11,7 +11,7 @@ maintainers:
>  
>  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
> +  a ramp generator with lookup table (LUT), the light pulse generator and a three
>    channel current sink. These blocks are found in a wide range of Qualcomm PMICs.
>  
>  properties:
> @@ -63,6 +63,27 @@ properties:
>          - description: dtest line to attach
>          - description: flags for the attachment
>  
> +  nvmem:
> +    description: >
> +      This property is required for PMICs that supports PPG, which is when a
> +      PMIC stores LPG per-channel data and pattern LUT in SDAM modules instead
> +      of in a LUT peripheral. For PMICs, such as PM8350C, per-channel data
> +      and pattern LUT is separated into 2 SDAM modules. In that case, phandles
> +      to both SDAM modules need to be specified.
> +    minItems: 1
> +    maxItems: 2
> +
> +  nvmem-names:
> +    minItems: 1
> +    maxItems: 2
> +
> +  qcom,pbs:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: >
> +      Phandle of the Qualcomm Programmable Boot Sequencer node (PBS).
> +      PBS node is used to trigger LPG pattern sequences for PMICs that support
> +      single SDAM PPG.
> +
>    multi-led:
>      type: object
>      $ref: leds-class-multicolor.yaml#
> @@ -106,6 +127,44 @@ required:
>  
>  additionalProperties: false
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,pmi632-lpg
> +    then:
> +      properties:
> +        nvmem:
> +          maxItems: 1
> +        nvmem-names:
> +          items:
> +            - const: lpg_chan_sdam
> +        qcom,pbs:
> +          maxItems: 1
> +      required:
> +        - nvmem
> +        - nvmem-names
> +        - qcom,pbs
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pm8350c-pwm
> +              - qcom,pm8550-pwm
> +    then:
> +      properties:
> +        nvmem:
> +          minItems: 2
> +        nvmem-names:
> +          items:
> +            - const: lpg_chan_sdam
> +            - const: lut_sdam

This can go into the main section and then here you just say 
'minItems: 2'. And similar for the 1st if/then.

> +      required:
> +        - nvmem
> +        - nvmem-names

Looks like these are always required.

> +
>  examples:
>    - |
>      #include <dt-bindings/leds/common.h>
> @@ -191,4 +250,35 @@ examples:
>        compatible = "qcom,pm8916-pwm";
>        #pwm-cells = <2>;
>      };
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    led-controller {
> +      compatible = "qcom,pmi632-lpg";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      #pwm-cells = <2>;
> +      nvmem-names = "lpg_chan_sdam";
> +      nvmem = <&pmi632_sdam_7>;
> +      qcom,pbs = <&pmi632_pbs_client3>;
> +
> +      led@1 {
> +        reg = <1>;
> +        color = <LED_COLOR_ID_RED>;
> +        label = "red";
> +      };
> +
> +      led@2 {
> +        reg = <2>;
> +        color = <LED_COLOR_ID_GREEN>;
> +        label = "green";
> +      };
> +
> +      led@3 {
> +        reg = <3>;
> +        color = <LED_COLOR_ID_BLUE>;
> +        label = "blue";
> +      };
> +    };
> +
>  ...
> -- 
> 2.41.0
>