Message ID | 1571756812-19005-3-git-send-email-akinobu.mita@gmail.com |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | leds: add generic LED level meter driver | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/dt-meta-schema | success |
Akinobu On 10/22/19 10:06 AM, Akinobu Mita wrote: > Add DT binding for leds-meter. $subject should be dt-bindings: leds: Add leds-meter binding And this patch should be first in the series > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Bjorn Andersson <bjorn@kryo.se> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Jean-Jacques Hiblot <jjhiblot@ti.com> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Dan Murphy <dmurphy@ti.com> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > --- > .../devicetree/bindings/leds/leds-meter.yaml | 42 ++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-meter.yaml > > diff --git a/Documentation/devicetree/bindings/leds/leds-meter.yaml b/Documentation/devicetree/bindings/leds/leds-meter.yaml > new file mode 100644 > index 0000000..d5dfa261 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-meter.yaml > @@ -0,0 +1,42 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-meter.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Generic LED level meter > + > +maintainers: > + - Akinobu Mita <akinobu.mita@gmail.com> > + > +description: > + Generic LED level meter consists of multiple LED devices by different drivers. > + > +properties: > + compatible: > + const: meter-leds > + > + leds: This seems a bit generic for the property name > + $ref: /schemas/types.yaml#/definitions/phandle-array > + minItems: 1 > + description: List of phandles to LED node that are members of a level meter. > + > + brightness-weights: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 1 > + description: Each integer represents a contribution ratio within a level > + meter. This description is a bit vague I would not be sure what I would set this to. Dan > + > +required: > + - compatible > + - leds > + > +examples: > + - | > + leds { > + compatible = "meter-leds"; > + leds = <&led0>, <&led1>, <&led2>, <&led3>; > + brightness-weights = <3 1 1 1>; > + }; > + > +...
2019年10月23日(水) 21:35 Dan Murphy <dmurphy@ti.com>: > > Akinobu > > On 10/22/19 10:06 AM, Akinobu Mita wrote: > > Add DT binding for leds-meter. > > $subject should be > > dt-bindings: leds: Add leds-meter binding > > And this patch should be first in the series OK. > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Bjorn Andersson <bjorn@kryo.se> > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > > Cc: Jean-Jacques Hiblot <jjhiblot@ti.com> > > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> > > Cc: Pavel Machek <pavel@ucw.cz> > > Cc: Dan Murphy <dmurphy@ti.com> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > > --- > > .../devicetree/bindings/leds/leds-meter.yaml | 42 ++++++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/leds/leds-meter.yaml > > > > diff --git a/Documentation/devicetree/bindings/leds/leds-meter.yaml b/Documentation/devicetree/bindings/leds/leds-meter.yaml > > new file mode 100644 > > index 0000000..d5dfa261 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/leds/leds-meter.yaml > > @@ -0,0 +1,42 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/leds/leds-meter.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Generic LED level meter > > + > > +maintainers: > > + - Akinobu Mita <akinobu.mita@gmail.com> > > + > > +description: > > + Generic LED level meter consists of multiple LED devices by different drivers. > > + > > +properties: > > + compatible: > > + const: meter-leds > > + > > + leds: > > This seems a bit generic for the property name I have no problem with changing to more descriptive name like "meter-leds". But this property is parsed by of_led_get(), so we need to change of_led_get() to take additional 'prefix' parameter. I would like to hear led-backlight authors' opinion. > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + minItems: 1 > > + description: List of phandles to LED node that are members of a level meter. > > + > > + brightness-weights: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + minItems: 1 > > + description: Each integer represents a contribution ratio within a level > > + meter. > > This description is a bit vague I would not be sure what I would set > this to. OK. I'll add some description and examples like I wrote in the commit message for patch 1/2.
On Tue, Oct 22, 2019 at 10:07 AM Akinobu Mita <akinobu.mita@gmail.com> wrote: > > Add DT binding for leds-meter. What's an leds meter? Need a better explanation to understand if this makes sense at all, but some comments on the schema below. > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Bjorn Andersson <bjorn@kryo.se> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Jean-Jacques Hiblot <jjhiblot@ti.com> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Dan Murphy <dmurphy@ti.com> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > --- > .../devicetree/bindings/leds/leds-meter.yaml | 42 ++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-meter.yaml > > diff --git a/Documentation/devicetree/bindings/leds/leds-meter.yaml b/Documentation/devicetree/bindings/leds/leds-meter.yaml > new file mode 100644 > index 0000000..d5dfa261 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-meter.yaml > @@ -0,0 +1,42 @@ > +# SPDX-License-Identifier: GPL-2.0 (GPL-2.0-only OR BSD-2-Clause) for new bindings. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-meter.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Generic LED level meter > + > +maintainers: > + - Akinobu Mita <akinobu.mita@gmail.com> > + > +description: > + Generic LED level meter consists of multiple LED devices by different drivers. > + > +properties: > + compatible: > + const: meter-leds > + > + leds: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + minItems: 1 No need for this as the minimum for arrays is already 1. And it doesn't work either. You'd need the $ref under an 'allOf'. > + description: List of phandles to LED node that are members of a level meter. > + > + brightness-weights: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 1 > + description: Each integer represents a contribution ratio within a level > + meter. > + > +required: > + - compatible > + - leds Add a: additionalProperties: false > + > +examples: > + - | > + leds { Needs to be a name that's not also a property name. 'leds-meter' or 'meter-leds' perhaps. And define the name above under a $nodename property. > + compatible = "meter-leds"; > + leds = <&led0>, <&led1>, <&led2>, <&led3>; > + brightness-weights = <3 1 1 1>; > + }; > + > +... > -- > 2.7.4 >
On Wed 2019-10-23 09:56:03, Rob Herring wrote: > On Tue, Oct 22, 2019 at 10:07 AM Akinobu Mita <akinobu.mita@gmail.com> wrote: > > > > Add DT binding for leds-meter. > > What's an leds meter? Need a better explanation to understand if this > makes sense at all, but some comments on the schema below. It groups several LEDs into one "virtual" LED. I'm not sure I like it. What is it good for? We do not have many triggers that dim the LEDs, and if it is only used from userspace, it can stay in userspace...? Best regards, Pavel
diff --git a/Documentation/devicetree/bindings/leds/leds-meter.yaml b/Documentation/devicetree/bindings/leds/leds-meter.yaml new file mode 100644 index 0000000..d5dfa261 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-meter.yaml @@ -0,0 +1,42 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/leds-meter.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic LED level meter + +maintainers: + - Akinobu Mita <akinobu.mita@gmail.com> + +description: + Generic LED level meter consists of multiple LED devices by different drivers. + +properties: + compatible: + const: meter-leds + + leds: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + description: List of phandles to LED node that are members of a level meter. + + brightness-weights: + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 1 + description: Each integer represents a contribution ratio within a level + meter. + +required: + - compatible + - leds + +examples: + - | + leds { + compatible = "meter-leds"; + leds = <&led0>, <&led1>, <&led2>, <&led3>; + brightness-weights = <3 1 1 1>; + }; + +...
Add DT binding for leds-meter. Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Bjorn Andersson <bjorn@kryo.se> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> Cc: Pavel Machek <pavel@ucw.cz> Cc: Dan Murphy <dmurphy@ti.com> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- .../devicetree/bindings/leds/leds-meter.yaml | 42 ++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-meter.yaml