| Message ID | 20210623035039.772660-1-bjorn.andersson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v9,1/2] dt-bindings: leds: Add Qualcomm Light Pulse Generator binding | expand |
On Tue, 22 Jun 2021 20:50:38 -0700 Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml The file name should be based on one of the compatible strings, for example the first one: qcom,pm8150b-lpg.yaml > + led@1 { > + reg = <1>; > + label = "green:user1"; > + }; `label` is deprecated, please don't use in new bindings in examples. Instead use color, function and function-enumerator, i.e. color = <LED_COLOR_ID_GREEN>; function = LED_FUNCTION_xxx; function-enumerator = <N>;
Hello Bjorn, Am Thu, Jun 24, 2021 at 06:44:54PM -0500 schrieb Bjorn Andersson: > On Thu 24 Jun 18:19 CDT 2021, Marek Behun wrote: > > please don't use in new bindings in examples. > > Instead use color, function and function-enumerator, i.e. > > > > color = <LED_COLOR_ID_GREEN>; > > function = LED_FUNCTION_xxx; > > function-enumerator = <N>; > > > > Can you point me to something helping me regarding what "function" to > use? > > For this particular devboard that the example comes from I have 4 LEDs > that are named "user1", "user2", "user3" and "user4" in the board > documentation. I can make up whatever for the example, but I would like > to get the following dts additions follow the expected guidelines. I asked myself the same question in the past. The wohle list is in 'include/dt-bindings/leds/common.h' and I in my personal project I opted for LED_FUNCTION_INDICATOR, but yes, the confusion is real. Greets Alex
Quoting Bjorn Andersson (2021-06-22 20:50:38) > diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > new file mode 100644 > index 000000000000..10aee61a7ffc > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > @@ -0,0 +1,164 @@ [....] > +examples: > + - | > + #include <dt-bindings/leds/common.h> > + > + lpg { Should the node name be led or leds? > + compatible = "qcom,pmi8994-lpg"; Shouldn't there be a reg property? I see the driver has them hardcoded but if this is a child of the spmi node then it should have a reg property (or many reg properties).
On Tue, Jun 22, 2021 at 08:50:38PM -0700, Bjorn Andersson wrote: > This adds the binding document describing the three hardware blocks > related to the Light Pulse Generator found in a wide range of Qualcomm > PMICs. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v8: > - None > > Changes since v7: > - Added qcom,pmc8180c-lpg > - Defined constraints for qcom,power-source > - Changes qcom,dtest to matrix and added constraints > - Changed example from LED_COLOR_ID_MULTI to LED_COLOR_ID_RGB > > .../bindings/leds/leds-qcom-lpg.yaml | 164 ++++++++++++++++++ > 1 file changed, 164 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > > diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > new file mode 100644 > index 000000000000..10aee61a7ffc > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > @@ -0,0 +1,164 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-qcom-lpg.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Light Pulse Generator > + > +maintainers: > + - Bjorn Andersson <bjorn.andersson@linaro.org> > + > +description: > > + The Qualcomm Light Pulse Generator consists of three different hardware blocks; > + a ramp generator with lookup table, the light pulse generator and a three > + channel current sink. These blocks are found in a wide range of Qualcomm PMICs. > + > +properties: > + compatible: > + enum: > + - qcom,pm8150b-lpg > + - qcom,pm8150l-lpg > + - qcom,pm8916-pwm > + - qcom,pm8941-lpg > + - qcom,pm8994-lpg > + - qcom,pmc8180c-lpg > + - qcom,pmi8994-lpg > + - qcom,pmi8998-lpg > + > + "#pwm-cells": > + const: 2 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + qcom,power-source: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + power-source used to drive the output, as defined in the datasheet. > + Should be specified if the TRILED block is present > + enum: [0, 1, 3] > + > + qcom,dtest: > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > + description: > > + A list of integer pairs, where each pair represent the dtest line the > + particular channel should be connected to and the flags denoting how the > + value should be outputed, as defined in the datasheet. The number of > + pairs should be the same as the number of channels. > + items: > + items: > + - description: dtest line to attach > + - description: flags for the attachment > + > + multi-led: > + type: object > + $ref: leds-class-multicolor.yaml# > + properties: > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + "^led@[0-9a-f]$": > + type: object > + $ref: common.yaml# > + > +patternProperties: > + "^led@[0-9a-f]$": > + type: object > + $ref: common.yaml# > + > + properties: > + reg: true > + > + required: > + - reg > + > +required: > + - compatible > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/leds/common.h> > + > + lpg { > + compatible = "qcom,pmi8994-lpg"; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + qcom,power-source = <1>; > + > + qcom,dtest = <0 0>, > + <0 0>, > + <0 0>, > + <4 1>; > + > + led@1 { > + reg = <1>; > + label = "green:user1"; > + }; > + > + led@2 { > + reg = <2>; > + label = "green:user0"; > + default-state = "on"; > + }; > + > + led@3 { > + reg = <3>; > + label = "green:user2"; > + }; > + > + led@4 { > + reg = <4>; > + label = "green:user3"; > + }; > + }; > + - | > + #include <dt-bindings/leds/common.h> > + > + lpg { > + compatible = "qcom,pmi8994-lpg"; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + qcom,power-source = <1>; > + > + multi-led { > + color = <LED_COLOR_ID_RGB>; > + function = LED_FUNCTION_STATUS; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + led@1 { > + reg = <1>; > + color = <LED_COLOR_ID_RED>; > + }; > + > + led@2 { > + reg = <2>; > + color = <LED_COLOR_ID_GREEN>; > + }; > + > + led@3 { > + reg = <3>; > + color = <LED_COLOR_ID_BLUE>; > + }; > + }; > + }; > + - | > + lpg { nit: should the node be named 'lpg-pwm'? IIUC a PMIC .dtsi could have both a 'lpg' and a 'lpg-pwm' node, even though only one of them can be enabled at any time. > + compatible = "qcom,pm8916-pwm"; > + #pwm-cells = <2>; > + };
On Thu 09 Sep 10:18 CDT 2021, Matthias Kaehlcke wrote: > On Tue, Jun 22, 2021 at 08:50:38PM -0700, Bjorn Andersson wrote: [..] > > + - | > > + #include <dt-bindings/leds/common.h> > > + > > + lpg { > > + compatible = "qcom,pmi8994-lpg"; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + qcom,power-source = <1>; > > + > > + multi-led { > > + color = <LED_COLOR_ID_RGB>; > > + function = LED_FUNCTION_STATUS; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + led@1 { > > + reg = <1>; > > + color = <LED_COLOR_ID_RED>; > > + }; > > + > > + led@2 { > > + reg = <2>; > > + color = <LED_COLOR_ID_GREEN>; > > + }; > > + > > + led@3 { > > + reg = <3>; > > + color = <LED_COLOR_ID_BLUE>; > > + }; > > + }; > > + }; > > + - | > > + lpg { > > nit: should the node be named 'lpg-pwm'? > > IIUC a PMIC .dtsi could have both a 'lpg' and a 'lpg-pwm' node, even though > only one of them can be enabled at any time. > No, there's only the one "LPG", with N channels. The lpg exposes a pwm chip and the child nodes may describe LEDs connected to the channels. So this example is the configuration where there's no LEDs attached. The compatible is "pwm", because the PM8916 lacks the pattern and RGB blocks that makes up the LPG - and is hence named "PWM" in the datasheet instead. So perhaps the example should be generically named "pwm" instead. In all other PMICs I know of the hardware block is named "lpg". Regards, Bjorn > > + compatible = "qcom,pm8916-pwm"; > > + #pwm-cells = <2>; > > + };
Hei hei, Am Wed, Sep 08, 2021 at 03:39:51AM +0000 schrieb Stephen Boyd: > Quoting Bjorn Andersson (2021-06-22 20:50:38) > > diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > > new file mode 100644 > > index 000000000000..10aee61a7ffc > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > > @@ -0,0 +1,164 @@ > [....] > > +examples: > > + - | > > + #include <dt-bindings/leds/common.h> > > + > > + lpg { > > Should the node name be led or leds? If I scan through Documentation/devicetree/bindings/leds and look at other devices, it should probably be named "led-controller", right? At least that's what Rob suggested when I converted the leds-pwm binding last year, using generic node names: https://lore.kernel.org/linux-leds/20200922155747.GA2734659@bogus/ Greets Alex > > > + compatible = "qcom,pmi8994-lpg"; > > Shouldn't there be a reg property? I see the driver has them hardcoded > but if this is a child of the spmi node then it should have a reg > property (or many reg properties).
diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml new file mode 100644 index 000000000000..10aee61a7ffc --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml @@ -0,0 +1,164 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/leds-qcom-lpg.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Light Pulse Generator + +maintainers: + - Bjorn Andersson <bjorn.andersson@linaro.org> + +description: > + The Qualcomm Light Pulse Generator consists of three different hardware blocks; + a ramp generator with lookup table, the light pulse generator and a three + channel current sink. These blocks are found in a wide range of Qualcomm PMICs. + +properties: + compatible: + enum: + - qcom,pm8150b-lpg + - qcom,pm8150l-lpg + - qcom,pm8916-pwm + - qcom,pm8941-lpg + - qcom,pm8994-lpg + - qcom,pmc8180c-lpg + - qcom,pmi8994-lpg + - qcom,pmi8998-lpg + + "#pwm-cells": + const: 2 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + qcom,power-source: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + power-source used to drive the output, as defined in the datasheet. + Should be specified if the TRILED block is present + enum: [0, 1, 3] + + qcom,dtest: + $ref: /schemas/types.yaml#/definitions/uint32-matrix + description: > + A list of integer pairs, where each pair represent the dtest line the + particular channel should be connected to and the flags denoting how the + value should be outputed, as defined in the datasheet. The number of + pairs should be the same as the number of channels. + items: + items: + - description: dtest line to attach + - description: flags for the attachment + + multi-led: + type: object + $ref: leds-class-multicolor.yaml# + properties: + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + "^led@[0-9a-f]$": + type: object + $ref: common.yaml# + +patternProperties: + "^led@[0-9a-f]$": + type: object + $ref: common.yaml# + + properties: + reg: true + + required: + - reg + +required: + - compatible + +additionalProperties: false + +examples: + - | + #include <dt-bindings/leds/common.h> + + lpg { + compatible = "qcom,pmi8994-lpg"; + + #address-cells = <1>; + #size-cells = <0>; + + qcom,power-source = <1>; + + qcom,dtest = <0 0>, + <0 0>, + <0 0>, + <4 1>; + + led@1 { + reg = <1>; + label = "green:user1"; + }; + + led@2 { + reg = <2>; + label = "green:user0"; + default-state = "on"; + }; + + led@3 { + reg = <3>; + label = "green:user2"; + }; + + led@4 { + reg = <4>; + label = "green:user3"; + }; + }; + - | + #include <dt-bindings/leds/common.h> + + lpg { + compatible = "qcom,pmi8994-lpg"; + + #address-cells = <1>; + #size-cells = <0>; + + qcom,power-source = <1>; + + multi-led { + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_STATUS; + + #address-cells = <1>; + #size-cells = <0>; + + led@1 { + reg = <1>; + color = <LED_COLOR_ID_RED>; + }; + + led@2 { + reg = <2>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@3 { + reg = <3>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + }; + - | + lpg { + compatible = "qcom,pm8916-pwm"; + #pwm-cells = <2>; + }; +...
This adds the binding document describing the three hardware blocks related to the Light Pulse Generator found in a wide range of Qualcomm PMICs. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- Changes since v8: - None Changes since v7: - Added qcom,pmc8180c-lpg - Defined constraints for qcom,power-source - Changes qcom,dtest to matrix and added constraints - Changed example from LED_COLOR_ID_MULTI to LED_COLOR_ID_RGB .../bindings/leds/leds-qcom-lpg.yaml | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml