Re: [PATCH v1 2/2] dt-bindings: add bindings for QCOM flash LED

From: Fenglin Wu
Date: Thu Sep 29 2022 - 06:57:00 EST




On 2022/9/29 15:06, Krzysztof Kozlowski wrote:
On 29/09/2022 04:20, Fenglin Wu wrote:


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.

Don't add just mine. Use instead scripts/get_maintainers.pl. For small
patchsets recipients should get everything. For big patchsets it is
usually split, where everyone receive only cover letter. It's not the
case here...

Thanks for the suggestion.
I actually used scripts/get_maintainers.pl when pushing the patches. I will double check it when sending v2.
Thanks

---
.../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.

If driver binds to only one compatible, it is expected to be the
fallback for all others. There might be exception for this rule but it
does not look like here.

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.

I did not say to remove other compatibles, but to use one fallback for
all of them.

Do you mean to update it similar to this?

compatible:
items:
- enum:
- qcom,pm8150c-flash-led
- qcom,pm8150l-flash-led
- qcom,pm8350c-flash-led
- const:
- qcom,spmi-flash-led

Best regards,
Krzysztof