diff mbox series

[v2,2/2] dt-bindings: add bindings for QCOM flash LED

Message ID 20220929121544.1064279-3-quic_fenglinw@quicinc.com
State New
Headers show
Series [v2,1/2] leds: flash: add driver to support flash LED module in QCOM PMICs | expand

Commit Message

Fenglin Wu Sept. 29, 2022, 12:15 p.m. UTC
Add binding document for flash LED module inside Qualcomm Technologies,
Inc. PMICs.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
 .../bindings/leds/qcom,spmi-flash-led.yaml    | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml

Comments

Rob Herring (Arm) Sept. 30, 2022, 7:33 p.m. UTC | #1
On Thu, Sep 29, 2022 at 02:40:05PM +0200, Krzysztof Kozlowski wrote:
> On 29/09/2022 14:15, Fenglin Wu wrote:
> > Add binding document for flash LED module inside Qualcomm Technologies,
> > Inc. PMICs.
> > 
> > Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> > +  reg:
> > +    description: address offset of the flash LED controller
> > +    maxItems: 1
> > +
> > +patternProperties:
> > +  "^led[0-3]$":
> 
> In such case: ^led-[0-9]$"
> 
> > +    type: object
> > +    $ref: common.yaml#
> > +    unevaluatedProperties: false
> > +    description: |
> > +      Represents the physical LED components which are connected to the
> > +      flash LED channels' output.
> > +
> > +    properties:
> > +      led-sources:

This is for when the power source and LED connection are programmable. 
IOW, when 'reg' is not enough to describe the configuration. If you only 
have LED channels 1-4 with a fixed connection to LED pins/output 1-4, 
then use 'reg'.

> > +        description: |
> > +          The HW indices of the flash LED channels that connect to the
> > +          physical LED
> > +        allOf:
> > +          - minItems: 1
> > +            maxItems: 2
> > +            items:
> > +              enum: [1, 2, 3, 4]
> > +
> > +      led-max-microamp:
> > +        description: |
> > +          The maximum current value when LED is not operating in flash mode (i.e. torch mode)
> > +          Valid values when an LED is connected to one flash LED channel:
> > +            5000 - 500000, step by 5000
> > +          Valid values when an LED is connected to two flash LED channels:
> > +            10000 - 1000000, step by 10000
> > +        minimum: 5000
> > +        maximum: 1000000

anyOf:
  - minimum: 5000
    maximum: 500000
    multipleOf: 5000
  - minimum: 10000
    maximum: 1000000
    multipleOf: 10000

Drop any description that's captured by the constraints.

> > +
> > +      flash-max-microamp:
> > +        description: |
> > +          The maximum current value when LED is operating in flash mode.
> > +          Valid values when an LED is connected to one flash LED channel:
> > +            12500 - 1500000, step by 12500
> > +          Valid values when an LED is connected to two flash LED channels:
> > +            25000 - 2000000, step by 12500
> > +        minimum: 12500
> > +        maximum: 2000000
> > +
> > +      flash-max-timeout-us:
> > +        description: |
> > +          The maximum timeout value when LED is operating in flash mode.
> > +          Valid values: 10000 - 1280000, step by 10000
> > +        minimum: 10000
> > +        maximum: 1280000

Similar comment for these 2.

> > +
> > +    required:
> > +      - led-sources
> > +      - led-max-microamp
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/leds/common.h>
> > +    spmi_bus {
> 
> No underscores in node names, so just "bus"

SPMI is something else though...
Krzysztof Kozlowski Oct. 3, 2022, 12:49 p.m. UTC | #2
On 30/09/2022 21:33, Rob Herring wrote:
>>> +
>>> +    required:
>>> +      - led-sources
>>> +      - led-max-microamp
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/leds/common.h>
>>> +    spmi_bus {
>>
>> No underscores in node names, so just "bus"
> 
> SPMI is something else though...

True, then rather spmi - already used in DTS.

Best regards,
Krzysztof
Fenglin Wu Oct. 10, 2022, 9:47 a.m. UTC | #3
On 2022/10/1 3:33, Rob Herring wrote:
> On Thu, Sep 29, 2022 at 02:40:05PM +0200, Krzysztof Kozlowski wrote:
>> On 29/09/2022 14:15, Fenglin Wu wrote:
>>> Add binding document for flash LED module inside Qualcomm Technologies,
>>> Inc. PMICs.
>>>
>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> +  reg:
>>> +    description: address offset of the flash LED controller
>>> +    maxItems: 1
>>> +
>>> +patternProperties:
>>> +  "^led[0-3]$":
>>
>> In such case: ^led-[0-9]$"
>>
>>> +    type: object
>>> +    $ref: common.yaml#
>>> +    unevaluatedProperties: false
>>> +    description: |
>>> +      Represents the physical LED components which are connected to the
>>> +      flash LED channels' output.
>>> +
>>> +    properties:
>>> +      led-sources:
> 
> This is for when the power source and LED connection are programmable.
> IOW, when 'reg' is not enough to describe the configuration. If you only
> have LED channels 1-4 with a fixed connection to LED pins/output 1-4,
> then use 'reg'.
> 
I think using led-sources here is more appropriate. The LED connection 
can be programmable to match with the board design. The flash module has 
4 channels (current outputs, indexed from 1 to 4) and the LEDs can be 
connected to either one or two of them depends on the design. Such as, 
if the design only requires LEDs with 1.5 A maximum current, then the HW 
just connects one channel to each LED and specify the corresponding 
channel index in the led-sources. Or if the design requires LEDs 
supporting up to 2 A maximum current, then the HW needs to gang 2 
channels' output together and specify the HW indices in led-sources 
accordingly.

>>> +        description: |
>>> +          The HW indices of the flash LED channels that connect to the
>>> +          physical LED
>>> +        allOf:
>>> +          - minItems: 1
>>> +            maxItems: 2
>>> +            items:
>>> +              enum: [1, 2, 3, 4]
>>> +
>>> +      led-max-microamp:
>>> +        description: |
>>> +          The maximum current value when LED is not operating in flash mode (i.e. torch mode)
>>> +          Valid values when an LED is connected to one flash LED channel:
>>> +            5000 - 500000, step by 5000
>>> +          Valid values when an LED is connected to two flash LED channels:
>>> +            10000 - 1000000, step by 10000
>>> +        minimum: 5000
>>> +        maximum: 1000000
> 
> anyOf:
>    - minimum: 5000
>      maximum: 500000
>      multipleOf: 5000
>    - minimum: 10000
>      maximum: 1000000
>      multipleOf: 10000
> 
> Drop any description that's captured by the constraints.
Thanks for the suggestion. I will update it accordingly.
> 
>>> +
>>> +      flash-max-microamp:
>>> +        description: |
>>> +          The maximum current value when LED is operating in flash mode.
>>> +          Valid values when an LED is connected to one flash LED channel:
>>> +            12500 - 1500000, step by 12500
>>> +          Valid values when an LED is connected to two flash LED channels:
>>> +            25000 - 2000000, step by 12500
>>> +        minimum: 12500
>>> +        maximum: 2000000
>>> +
>>> +      flash-max-timeout-us:
>>> +        description: |
>>> +          The maximum timeout value when LED is operating in flash mode.
>>> +          Valid values: 10000 - 1280000, step by 10000
>>> +        minimum: 10000
>>> +        maximum: 1280000
> 
> Similar comment for these 2.
Done
> 
>>> +
>>> +    required:
>>> +      - led-sources
>>> +      - led-max-microamp
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/leds/common.h>
>>> +    spmi_bus {
>>
>> No underscores in node names, so just "bus"
> 
> SPMI is something else though...
Sure, will update it to spmi
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml b/Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml
new file mode 100644
index 000000000000..3ab1113a7b28
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml
@@ -0,0 +1,120 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/qcom,spmi-flash-led.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Flash LED device inside Qualcomm Technologies, Inc. PMICs
+
+maintainers:
+  - Fenglin Wu <quic_fenglinw@quicinc.com>
+
+description: |
+  Flash LED controller is present inside some Qualcomm Technologies, Inc. PMICs.
+  The flash LED module can have different number of LED channels supported
+  e.g. 3 or 4. There are some different registers between them but they can
+  both support maximum current up to 1.5 A per channel and they can also support
+  ganging 2 channels together to supply maximum current up to 2 A. The current
+  will be split symmetrically on each channel and they will be enabled and
+  disabled at the same time.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,pm8150c-flash-led
+          - qcom,pm8150l-flash-led
+          - qcom,pm8350c-flash-led
+      - const: qcom,spmi-flash-led
+  reg:
+    description: address offset of the flash LED controller
+    maxItems: 1
+
+patternProperties:
+  "^led[0-3]$":
+    type: object
+    $ref: common.yaml#
+    unevaluatedProperties: false
+    description: |
+      Represents the physical LED components which are connected to the
+      flash LED channels' output.
+
+    properties:
+      led-sources:
+        description: |
+          The HW indices of the flash LED channels that connect to the
+          physical LED
+        allOf:
+          - minItems: 1
+            maxItems: 2
+            items:
+              enum: [1, 2, 3, 4]
+
+      led-max-microamp:
+        description: |
+          The maximum current value when LED is not operating in flash mode (i.e. torch mode)
+          Valid values when an LED is connected to one flash LED channel:
+            5000 - 500000, step by 5000
+          Valid values when an LED is connected to two flash LED channels:
+            10000 - 1000000, step by 10000
+        minimum: 5000
+        maximum: 1000000
+
+      flash-max-microamp:
+        description: |
+          The maximum current value when LED is operating in flash mode.
+          Valid values when an LED is connected to one flash LED channel:
+            12500 - 1500000, step by 12500
+          Valid values when an LED is connected to two flash LED channels:
+            25000 - 2000000, step by 12500
+        minimum: 12500
+        maximum: 2000000
+
+      flash-max-timeout-us:
+        description: |
+          The maximum timeout value when LED is operating in flash mode.
+          Valid values: 10000 - 1280000, step by 10000
+        minimum: 10000
+        maximum: 1280000
+
+    required:
+      - led-sources
+      - led-max-microamp
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+    spmi_bus {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        led-controller@ee00 {
+            compatible = "qcom,pm8350c-flash-led", "qcom,spmi-flash-led";
+            reg = <0xee00>;
+
+            led0 {
+                function = LED_FUNCTION_FLASH;
+                color = <LED_COLOR_ID_WHITE>;
+                led-sources = <1>, <4>;
+                led-max-microamp = <300000>;
+                flash-max-microamp = <2000000>;
+                flash-max-timeout-us = <1280000>;
+                function-enumerator = <0>;
+            };
+
+            led1 {
+                function = LED_FUNCTION_FLASH;
+                color = <LED_COLOR_ID_YELLOW>;
+                led-sources = <2>, <3>;
+                led-max-microamp = <300000>;
+                flash-max-microamp = <2000000>;
+                flash-max-timeout-us = <1280000>;
+                function-enumerator = <1>;
+            };
+        };
+    };