Re: [PATCH v1 2/2] dt-bindings: add bindings for QCOM flash LED
From: Fenglin Wu
Date: Wed Sep 28 2022 - 22:21:51 EST
On 2022/9/28 16:21, Krzysztof Kozlowski wrote:
On 28/09/2022 04:42, Fenglin Wu wrote:
Add binding document for flash LED module inside Qualcomm Technologies,
Inc. PMICs.
Signed-off-by: Fenglin Wu <quic_fenglinw@xxxxxxxxxxx>
You did not Cc me on first patch, so difficult to say how much it
matches the driver... There is also no DTS.
Thanks for reviewing the binding change, I sent the driver changes in
the same series and you can check it here:
https://lore.kernel.org/linux-leds/6c0e5083-baae-3ed3-5eed-e08bbb9e7576@xxxxxxxxxx/T/#m97f71ce3f291f62d65f8107352d8ab9507093ab2
I will add you in email to list when sending next patchset.
---
.../bindings/leds/leds-qcom-flash.yaml | 108 ++++++++++++++++++
1 file changed, 108 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-flash.yaml
diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-flash.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-flash.yaml
new file mode 100644
index 000000000000..52a99182961b
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-qcom-flash.yaml
Filename matching compatible if there is one fallback (e.g.
qcom,spmi-flash-led.yaml).
Sure, I will update the file name to match with the fallback compatible
string.
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-qcom-flash.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Flash LED device inside Qualcomm Technologies, Inc. PMICs
+
+maintainers:
+ - Fenglin Wu <quic_fenglinw@xxxxxxxxxxx>
+
+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,spmi-flash-led
+ - qcom,pm8150c-flash-led
+ - qcom,pm8150l-flash-led
+ - qcom,pm8350c-flash-led
I doubt these are all different. You should use fallback, which also
will make use of the "items" you used...
pm8150c and pm8150l are different PMIC variants which have same flash
LED module with 3 flash LED channels, while pm8350c has a different
flash LED module with 4 flash LED channels. They can all use
"qcom,spmi-flash-led" as the fallback because the driver has code logic
to detect HW sub-types. But I was thinking to give out the PMIC names
here so anyone who is using the driver could easily identify if the
driver is suitable for the HW that he/she is using.
+
+ 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.
Does not look like wrapped at 80.
Other places as well.
Sure, will wrap the lines at 80, I thought not exceeding 110 is also
acceptable.
+
+ properties:
Does not look like you tested the bindings...
You miss here reg.
will update the node name without using unit name.
+ 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
You need minimum and maximum.
Sure, I will add them
+
+ 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
You need minimum and maximum.
Sure, I will add them
+
+ flash-max-timeout-us:
+ description: |
+ The maximum timeout value when LED is operating in flash mode.
+ Valid values: 10000 - 1280000, step by 10000
You need minimum and maximum.
+
+ required:
+ - led-sources
+ - led-max-microamp
reg.
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+ flash-led@ee00 {
Node name: led-controller
+ compatible = "qcom,spmi-flash-led";
+ reg = <0xee00>;
+
+ led@0 {
Test your bindings...
+ function = LED_FUNCTION_FLASH;
Use 4 spaces for indentation of example.
sure, I will update it.
+ 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>;
+ };
+
Best regards,
Krzysztof