On 24/11/2022 21:48, Martin Kurbanov wrote: > Add YAML devicetree binding for AWINIC AW20036/AW20052/AW20074 > led driver. > > Signed-off-by: Martin Kurbanov <mmkurbanov@xxxxxxxxxxxxxx> > --- > .../bindings/leds/leds-aw200xx.yaml | 110 ++++++++++++++++++ > include/dt-bindings/leds/leds-aw200xx.h | 48 ++++++++ > 2 files changed, 158 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-aw200xx.yaml > create mode 100644 include/dt-bindings/leds/leds-aw200xx.h > > diff --git a/Documentation/devicetree/bindings/leds/leds-aw200xx.yaml b/Documentation/devicetree/bindings/leds/leds-aw200xx.yaml > new file mode 100644 > index 000000000000..3bdadcbc2ee2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-aw200xx.yaml Filename based on compatibles, so "awinic,aw200xx.yaml" > @@ -0,0 +1,110 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-aw200xx.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: AWINIC AW200XX LED Driver What does the "Driver" mean? Linux Driver? If yes, then drop it. Same in other places. > + > +maintainers: > + - Martin Kurbanov <mmkurbanov@xxxxxxxxxxxxxx> > + > +description: | > + This controller is present on AW20036/AW20054/AW20072. > + It is a 3x12/6x9/6x12 matrix LED driver programmed via > + an I2C interface, up to 36/54/72 LEDs or 12/18/24 RGBs, > + 3 pattern controllers for auto breathing or group dimming control. > + > + For more product information please see the link below: > + aw20036 - https://www.awinic.com/Public/Uploads/uploadfile/files/20200509/20200509151532_5eb65894d205a.pdf > + aw20054 - https://www.awinic.com/Public/Uploads/uploadfile/files/20200509/20200509151602_5eb658b2b77cb.pdf > + aw20072 - https://www.awinic.com/Public/Uploads/uploadfile/files/20200509/20200509151754_5eb659227a145.pdf > + > +properties: > + compatible: > + enum: > + - awinic,aw20036 > + - awinic,aw20054 > + - awinic,aw20072 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + interrupts: > + maxItems: 1 > + > + display-size: Is it a standard property? Does not look like. Non-standard properties need vendor prefix and type ($ref). > + maxItems: 1 > + description: > + Leds matrix size, see dt-bindings/leds/leds-aw200xx.h But judging by your constants, you have the same number of columns, just rows differ, so probably you want to describe here number of rows. > + > + imax: > + maxItems: 1 > + description: > + Maximum supply current, see dt-bindings/leds/leds-aw200xx.h No. Use existing properties from common.yaml. This looks like led-max-microamp and it is per LED, not per entire device. > + > +patternProperties: > + "^led@[0-9a-f]$": > + type: object > + $ref: common.yaml# unevaluatedProperties: false > + > + properties: > + reg: > + description: > + LED number > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" > + - display-size > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/leds/common.h> > + #include <dt-bindings/leds/leds-aw200xx.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led-controller@3a { > + compatible = "awinic,aw20036"; > + reg = <0x3a>; > + #address-cells = <1>; > + #size-cells = <0>; > + interrupt-parent = <&gpio_intc>; > + interrupts = <13 IRQ_TYPE_LEVEL_LOW>; > + > + display-size = <AW20036_DSIZE_3X12>; > + imax = <AW200XX_IMAX_60MA>; > + > + led@0 { > + reg = <0x0>; > + color = <LED_COLOR_ID_RED>; > + }; > + > + led@1 { > + reg = <0x1>; > + color = <LED_COLOR_ID_GREEN>; > + }; > + > + led@2 { > + reg = <0x2>; > + color = <LED_COLOR_ID_BLUE>; > + }; > + }; > + }; > + > +... > diff --git a/include/dt-bindings/leds/leds-aw200xx.h b/include/dt-bindings/leds/leds-aw200xx.h > new file mode 100644 > index 000000000000..6b2ba4c3c6b1 > --- /dev/null > +++ b/include/dt-bindings/leds/leds-aw200xx.h > @@ -0,0 +1,48 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ Dual license, like bindings. > +/** > + * This header provides constants for aw200xx LED bindings. > + * > + * Copyright (c) 2022, SberDevices. All Rights Reserved. > + * > + * Author: Martin Kurbanov <mmkurbanov@xxxxxxxxxxxxxx> > + */ > +#ifndef _DT_BINDINGS_LEDS_AW200XX_H > +#define _DT_BINDINGS_LEDS_AW200XX_H > + > +/* Global max current (IMAX) */ > +#define AW200XX_IMAX_3_3MA 8 > +#define AW200XX_IMAX_6_7MA 9 No. Bindings are not for storing register constants. Feel free to store here IDs (ID start from 0 or 1 and is incremented by 1)... but how the IMAX even matches any need for "ID"? > +#define AW200XX_IMAX_10MA 0 > +#define AW200XX_IMAX_13_3MA 11 > +#define AW200XX_IMAX_20MA 1 > +#define AW200XX_IMAX_26_7MA 13 > +#define AW200XX_IMAX_30MA 2 > +#define AW200XX_IMAX_40MA 3 > +#define AW200XX_IMAX_53_3MA 15 > +#define AW200XX_IMAX_60MA 4 > +#define AW200XX_IMAX_80MA 5 > +#define AW200XX_IMAX_120MA 6 > +#define AW200XX_IMAX_160MA 7 > + > +/* Display size for aw20036 */ > +#define AW20036_DSIZE_1X12 0 > +#define AW20036_DSIZE_2X12 1 > +#define AW20036_DSIZE_3X12 2 > + > +/* Display size for aw20054 */ > +#define AW20054_DSIZE_1X9 0 > +#define AW20054_DSIZE_2X9 1 > +#define AW20054_DSIZE_3X9 2 > +#define AW20054_DSIZE_4X9 3 > +#define AW20054_DSIZE_5X9 4 > +#define AW20054_DSIZE_6X9 5 > + > +/* Display size for aw20072 */ > +#define AW20072_DSIZE_1X12 0 > +#define AW20072_DSIZE_2X12 1 > +#define AW20072_DSIZE_3X12 2 > +#define AW20072_DSIZE_4X12 3 > +#define AW20072_DSIZE_5X12 4 > +#define AW20072_DSIZE_6X12 5 Drop all constants and instead use number of rows without specifying it in binding. So in total entire file can be dropped. > + > +#endif /* !_DT_BINDINGS_LEDS_AW200XX_H */ Best regards, Krzysztof
