Hi Nicolas, On 1/26/21 3:04 PM, Nícolas F. R. A. Prado wrote:
Add devicetree binding for QCOM SPMI Flash LEDs, which are part of PM8941, and are used both as lantern and camera flash. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxxx> --- Changes in v2: - Add this commit .../bindings/leds/leds-qcom-spmi-flash.yaml | 94 +++++++++++++++++++ .../dt-bindings/leds/leds-qcom-spmi-flash.h | 15 +++ 2 files changed, 109 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-spmi-flash.yaml create mode 100644 include/dt-bindings/leds/leds-qcom-spmi-flash.h diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-spmi-flash.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-spmi-flash.yaml new file mode 100644 index 000000000000..169716e14f67 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-qcom-spmi-flash.yaml @@ -0,0 +1,94 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/leds-qcom-spmi-flash.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm SPMI Flash LEDs + +maintainers: + - Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxxx> + +description: | + The Qualcomm SPMI Flash LEDs are part of Qualcomm PMICs and are used primarily + as a camera or video flash. They can also be used as a lantern when on torch + mode. + The PMIC is connected to Host processor via SPMI bus. + +properties: + compatible: + const: qcom,spmi-flash + + reg: + maxItems: 1 + + flash-boost-supply: + description: SMBB regulator for LED flash mode + + torch-boost-supply: + description: SMBB regulator for LED torch mode + +patternProperties: + "^led[0-1]$": + type: object + $ref: common.yaml# + + properties: + qcom,clamp-curr: + description: current to clamp at, in uA
You're already using led-max-microamp and flash-max-microamp, so I'm not sure how this is related to those.
+ $ref: /schemas/types.yaml#definitions/uint32 + + qcom,headroom: + description: | + headroom to use. Use one of QCOM_SPMI_FLASH_HEADROOM_* defined in + include/dt-bindings/leds/leds-qcom-spmi-flash.h
More information would be welcome here on how it impacts device operation.
+ $ref: /schemas/types.yaml#definitions/uint32 + minimum: 0 + maximum: 3 + + qcom,startup-dly: + description: | + delay before flashing. Use one of QCOM_SPMI_FLASH_STARTUP_DLY_* + defined in include/dt-bindings/leds/leds-qcom-spmi-flash.h
I don't see much gain in having this exposed. We don't have related constructs in the API to handle that, so I propose to always set it to 0 and not expose this setting at all.
+ $ref: /schemas/types.yaml#definitions/uint32 + minimum: 0 + maximum: 3 + + qcom,safety-timer: + description: include for safety timer use, otherwise watchdog timer will be used + type: boolean
What's the difference between watchdog timer and safety timer? Either way, I propose to choose the option best fitting for handling flash timeout setting and hide this inside the driver.
+required: + - compatible + - reg + - flash-boost-supply + - torch-boost-supply + +additionalProperties: false + +examples: + - | + #include <dt-bindings/leds/common.h> + #include <dt-bindings/leds/leds-qcom-spmi-flash.h> + + qcom,leds@d300 { + compatible = "qcom,spmi-flash"; + reg = <0xd300 0x100>; + flash-boost-supply = <&pm8941_5vs1>; + torch-boost-supply = <&pm8941_5v>; + + led0 { + led-sources = <0>;
Please use reg property for this purpose instead.
+ function = LED_FUNCTION_FLASH; + color = <LED_COLOR_ID_WHITE>; + led-max-microamp = <200000>; + flash-max-microamp = <1000000>; + flash-max-timeout-us = <1280000>;
You should mention this properties above and provide constraints (min, max, step).
+ default-state = "off";
There's no point in having this in the example.
+ qcom,clamp-curr = <200000>; > + qcom,headroom = <QCOM_SPMI_FLASH_HEADROOM_500MV>; + qcom,startup-dly = <QCOM_SPMI_FLASH_STARTUP_DLY_128US>; + qcom,safety-timer; + }; + }; +... diff --git a/include/dt-bindings/leds/leds-qcom-spmi-flash.h b/include/dt-bindings/leds/leds-qcom-spmi-flash.h new file mode 100644 index 000000000000..8bd54a8e831d --- /dev/null +++ b/include/dt-bindings/leds/leds-qcom-spmi-flash.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _DT_BINDINGS_LEDS_QCOM_SPMI_FLASH_H +#define _DT_BINDINGS_LEDS_QCOM_SPMI_FLASH_H + +#define QCOM_SPMI_FLASH_HEADROOM_250MV 0 +#define QCOM_SPMI_FLASH_HEADROOM_300MV 1 +#define QCOM_SPMI_FLASH_HEADROOM_400MV 2 +#define QCOM_SPMI_FLASH_HEADROOM_500MV 3 + +#define QCOM_SPMI_FLASH_STARTUP_DLY_10US 0 +#define QCOM_SPMI_FLASH_STARTUP_DLY_32US 1 +#define QCOM_SPMI_FLASH_STARTUP_DLY_64US 2 +#define QCOM_SPMI_FLASH_STARTUP_DLY_128US 3 + +#endif
-- Best regards, Jacek Anaszewski