2022-09-28 03:17:30

by Fenglin Wu

[permalink] [raw]
Subject: [PATCH v1 0/2] Add LED driver for flash module in QCOM PMICs

Initial driver and binding document changes for supporting flash LED
module in Qualcomm Technologies, Inc. PMICs.

Fenglin Wu (2):
leds: flash: add driver to support flash LED module in QCOM PMICs
dt-bindings: add bindings for QCOM flash LED

.../bindings/leds/leds-qcom-flash.yaml | 108 +++
drivers/leds/flash/Kconfig | 14 +
drivers/leds/flash/Makefile | 1 +
drivers/leds/flash/leds-qcom-flash.c | 706 ++++++++++++++++++
4 files changed, 829 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-flash.yaml
create mode 100644 drivers/leds/flash/leds-qcom-flash.c

--
2.25.1


2022-09-28 03:17:32

by Fenglin Wu

[permalink] [raw]
Subject: [PATCH v1 2/2] dt-bindings: add bindings for QCOM flash LED

Add binding document for flash LED module inside Qualcomm Technologies,
Inc. PMICs.

Signed-off-by: Fenglin Wu <[email protected]>
---
.../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
@@ -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 <[email protected]>
+
+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
+
+ 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.
+
+ properties:
+ 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
+
+ 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
+
+ flash-max-timeout-us:
+ description: |
+ The maximum timeout value when LED is operating in flash mode.
+ Valid values: 10000 - 1280000, step by 10000
+
+ required:
+ - led-sources
+ - led-max-microamp
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+ flash-led@ee00 {
+ compatible = "qcom,spmi-flash-led";
+ reg = <0xee00>;
+
+ led@0 {
+ function = LED_FUNCTION_FLASH;
+ 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>;
+ };
+
+ led@1 {
+ function = LED_FUNCTION_FLASH;
+ color = <LED_COLOR_ID_YELLOW>;
+ led-sources = <2>, <3>;
+ led-max-microamp = <300000>;
+ flash-max-microamp = <2000000>;
+ flash-max-timeout-us = <1280000>;
+ function-enumerator = <1>;
+ };
+ };
--
2.25.1

2022-09-28 08:23:35

by Krzysztof Kozlowski

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

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 <[email protected]>

You did not Cc me on first patch, so difficult to say how much it
matches the driver... There is also no DTS.

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

> @@ -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 <[email protected]>
> +
> +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...

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

> +
> + properties:

Does not look like you tested the bindings...

You miss here reg.

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

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


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

> + 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

2022-09-28 13:02:25

by Rob Herring (Arm)

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

On Wed, 28 Sep 2022 10:42:39 +0800, Fenglin Wu wrote:
> Add binding document for flash LED module inside Qualcomm Technologies,
> Inc. PMICs.
>
> Signed-off-by: Fenglin Wu <[email protected]>
> ---
> .../bindings/leds/leds-qcom-flash.yaml | 108 ++++++++++++++++++
> 1 file changed, 108 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-flash.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/leds/leds-qcom-flash.example.dts:21.17-32: Warning (reg_format): /example-0/flash-led@ee00:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/leds/leds-qcom-flash.example.dts:23.23-31.19: Warning (unit_address_vs_reg): /example-0/flash-led@ee00/led@0: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/leds/leds-qcom-flash.example.dts:33.23-41.19: Warning (unit_address_vs_reg): /example-0/flash-led@ee00/led@1: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/leds/leds-qcom-flash.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/leds/leds-qcom-flash.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/leds/leds-qcom-flash.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/leds/leds-qcom-flash.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/leds/leds-qcom-flash.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-09-29 02:35:29

by Fenglin Wu

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



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 <[email protected]>
>
> 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/[email protected]/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 <[email protected]>
>> +
>> +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
>

2022-09-29 07:45:29

by Krzysztof Kozlowski

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

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 <[email protected]>
>>
>> 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/[email protected]/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...

>>
>>> ---
>>> .../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 <[email protected]>
>>> +
>>> +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.

Best regards,
Krzysztof

2022-09-29 11:14:43

by Fenglin Wu

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



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 <[email protected]>
>>>
>>> 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/[email protected]/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 <[email protected]>
>>>> +
>>>> +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
>

2022-09-29 11:37:59

by Krzysztof Kozlowski

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

On 29/09/2022 12:56, Fenglin Wu wrote:
>> 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

Yes, just const is not a list, so "const: qcom,spmi-flash-led"

Best regards,
Krzysztof