| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <597508ab-7d53-e1cc-a63f-b66e5dc7016d@gmail.com>
Date: Sat, 30 Jan 2021 21:32:23 +0100
From: Jacek Anaszewski <jacek.anaszewski@...il.com>
To: Nícolas F. R. A. Prado
<nfraprado@...tonmail.com>, Pavel Machek <pavel@....cz>,
Dan Murphy <dmurphy@...com>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Andy Gross <agross@...nel.org>,
Rob Herring <robh+dt@...nel.org>
Cc: linux-leds@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
Brian Masney <masneyb@...tation.org>,
Luca Weiss <luca@...tu.xyz>,
Russell King <linux@...linux.org.uk>,
Georgi Djakov <georgi.djakov@...aro.org>,
linux-kernel@...r.kernel.org,
~postmarketos/upstreaming@...ts.sr.ht,
lkcamp@...ts.libreplanetbr.org, andrealmeid@...labora.com
Subject: Re: [PATCH v2 1/4] dt-bindings: leds: Add binding for qcom-spmi-flash
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@...tonmail.com>
> ---
> 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@...tonmail.com>
> +
> +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@...0 {
> + 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
Powered by blists - more mailing lists