Messages in this thread | | | Date | Mon, 10 Oct 2022 17:47:20 +0800 | Subject | Re: [PATCH v2 2/2] dt-bindings: add bindings for QCOM flash LED | From | Fenglin Wu <> |
| |
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 >
| |