[PATCH v2 00/11] leds: aw200xx: several driver updates

[PATCH v2 00/11] leds: aw200xx: several driver updates

[Dmitry Rokosov]

The following patch series includes several updates for the AW200XX LED
driver:
    - some small fixes and optimizations to the driver implementation:
      delays, autodimming calculation, disable_locking regmap flag,
      display_rows calculation in runtime;
    - fix LED device tree node pattern to accept LED names counting not
      only from 0 to f;
    - add missing reg constraints;
    - support HWEN hardware control, which allows enabling or disabling
      AW200XX RTL logic from the main SoC using a GPIO pin;
    - introduce the new AW20108 LED controller, the datasheet for this
      controller can be found at [1].

Changes v2 since v1 at [2]:
    - rebase on the latest aw200xx changes from lee/leds git repo
    - some commit messages rewording
    - replace legacy gpio_* API with gpiod_* and devm_gpiod_* API
    - rename dt property awinic,hwen-gpio to hwen-gpios according to
      gpiod API
    - use fsleep() instead of usleep_range() per Andy's suggestion
    - add max_brightness parameter to led cdev to restrict
      set_brightness() overflow
    - provide reg constraints as Rob suggested
    - move hwen-gpios to proper dt node in the bindings example

Links:
    [1] https://doc.awinic.com/doc/20230609wm/8a9a9ac8-1d8f-4e75-bf7a-67a04465c153.pdf
    [2] https://lore.kernel.org/all/20231006160437.15627-1-ddrokosov@salutedevices.com/

Dmitry Rokosov (3):
  leds: aw200xx: support HWEN hardware control
  dt-bindings: leds: aw200xx: introduce optional hwen-gpios property
  dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

George Stark (7):
  leds: aw200xx: calculate dts property display_rows in driver
  dt-bindings: leds: aw200xx: remove property "awinic,display-rows"
  leds: aw200xx: add delay after software reset
  leds: aw200xx: enable disable_locking flag in regmap config
  leds: aw200xx: improve autodim calculation method
  leds: aw200xx: add support for aw20108 device
  dt-bindings: leds: awinic,aw200xx: add AW20108 device

Martin Kurbanov (1):
  leds: aw200xx: fix write to DIM parameter

 .../bindings/leds/awinic,aw200xx.yaml         | 49 ++++------
 drivers/leds/Kconfig                          |  8 +-
 drivers/leds/leds-aw200xx.c                   | 96 +++++++++++++++----
 3 files changed, 102 insertions(+), 51 deletions(-)

-- 
2.36.0

[PATCH v2 01/11] leds: aw200xx: fix write to DIM parameter

[Dmitry Rokosov]

From: Martin Kurbanov <mmkurbanov@salutedevices.com>

If write only DIM value to the page 4, LED brightness will not be
updated, as both DIM and FADE need to be written to the page 4.
Therefore, write DIM to the page 1.

Fixes: 36a87f371b7a ("leds: Add AW20xx driver")
Signed-off-by: Martin Kurbanov <mmkurbanov@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/leds/leds-aw200xx.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index ef4eda6a09ee..842a22087b16 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -74,6 +74,10 @@
 #define AW200XX_LED2REG(x, columns) \
 	((x) + (((x) / (columns)) * (AW200XX_DSIZE_COLUMNS_MAX - (columns))))
 
+/* DIM current configuration register on page 1 */
+#define AW200XX_REG_DIM_PAGE1(x, columns) \
+	AW200XX_REG(AW200XX_PAGE1, AW200XX_LED2REG(x, columns))
+
 /*
  * DIM current configuration register (page 4).
  * The even address for current DIM configuration.
@@ -153,7 +157,8 @@ static ssize_t dim_store(struct device *dev, struct device_attribute *devattr,
 
 	if (dim >= 0) {
 		ret = regmap_write(chip->regmap,
-				   AW200XX_REG_DIM(led->num, columns), dim);
+				   AW200XX_REG_DIM_PAGE1(led->num, columns),
+				   dim);
 		if (ret)
 			goto out_unlock;
 	}
-- 
2.36.0

[PATCH v2 02/11] leds: aw200xx: support HWEN hardware control

[Dmitry Rokosov]

HWEN is hardware control, which is used for enable/disable aw200xx chip.
It's high active, internally pulled down to GND.

After HWEN pin set high the chip begins to load the OTP information,
which takes 200us to complete. About 200us wait time is needed for
internal oscillator startup and display SRAM initialization. After
display SRAM initialization, the registers in page 1 to page 5 can be
configured via i2c interface.

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/leds/leds-aw200xx.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 842a22087b16..911c3154585f 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -10,6 +10,7 @@
 #include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/container_of.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/leds.h>
 #include <linux/mod_devicetable.h>
@@ -116,6 +117,7 @@ struct aw200xx {
 	struct mutex mutex;
 	u32 num_leds;
 	u32 display_rows;
+	struct gpio_desc *hwen;
 	struct aw200xx_led leds[] __counted_by(num_leds);
 };
 
@@ -358,6 +360,25 @@ static int aw200xx_chip_check(const struct aw200xx *const chip)
 	return 0;
 }
 
+static void aw200xx_enable(const struct aw200xx *const chip)
+{
+	gpiod_set_value_cansleep(chip->hwen, 1);
+
+	/*
+	 * After HWEN pin set high the chip begins to load the OTP information,
+	 * which takes 200us to complete. About 200us wait time is needed for
+	 * internal oscillator startup and display SRAM initialization. After
+	 * display SRAM initialization, the registers in page1 to page5 can be
+	 * configured via i2c interface.
+	 */
+	fsleep(400);
+}
+
+static void aw200xx_disable(const struct aw200xx *const chip)
+{
+	return gpiod_set_value_cansleep(chip->hwen, 0);
+}
+
 static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip)
 {
 	struct fwnode_handle *child;
@@ -517,6 +538,10 @@ static int aw200xx_probe(struct i2c_client *client)
 	if (IS_ERR(chip->regmap))
 		return PTR_ERR(chip->regmap);
 
+	chip->hwen = devm_gpiod_get_optional(&client->dev, "hwen", GPIOD_OUT_HIGH);
+
+	aw200xx_enable(chip);
+
 	ret = aw200xx_chip_check(chip);
 	if (ret)
 		return ret;
@@ -537,6 +562,9 @@ static int aw200xx_probe(struct i2c_client *client)
 	ret = aw200xx_chip_init(chip);
 
 out_unlock:
+	if (ret)
+		aw200xx_disable(chip);
+
 	mutex_unlock(&chip->mutex);
 	return ret;
 }
@@ -546,6 +574,7 @@ static void aw200xx_remove(struct i2c_client *client)
 	struct aw200xx *chip = i2c_get_clientdata(client);
 
 	aw200xx_chip_reset(chip);
+	aw200xx_disable(chip);
 	mutex_destroy(&chip->mutex);
 }
 
-- 
2.36.0

[PATCH v2 03/11] dt-bindings: leds: aw200xx: introduce optional hwen-gpios property

[Dmitry Rokosov]

Property 'hwen-gpios' is optional, it can be used by the board
developer to connect AW200XX LED controller with appropriate poweron
GPIO pad.

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
index feb5febaf361..255eb0563737 100644
--- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
+++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
@@ -41,6 +41,9 @@ properties:
     description:
       Leds matrix size
 
+  hwen-gpios:
+    maxItems: 1
+
 patternProperties:
   "^led@[0-9a-f]$":
     type: object
@@ -90,6 +93,7 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
     #include <dt-bindings/leds/common.h>
 
     i2c {
@@ -102,6 +106,7 @@ examples:
             #address-cells = <1>;
             #size-cells = <0>;
             awinic,display-rows = <3>;
+            hwen-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
 
             led@0 {
                 reg = <0x0>;
-- 
2.36.0

[PATCH v2 04/11] leds: aw200xx: calculate dts property display_rows in driver

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

Get rid of device tree property "awinic,display-rows" and calculate it
in driver using led definition nodes. display-row actually means number
of current switches and depends on how leds are connected to the device.

Signed-off-by: George Stark <gnstark@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/leds/leds-aw200xx.c | 39 +++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 911c3154585f..a2a31b8e623e 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -379,6 +379,31 @@ static void aw200xx_disable(const struct aw200xx *const chip)
 	return gpiod_set_value_cansleep(chip->hwen, 0);
 }
 
+static int aw200xx_probe_get_display_rows(struct device *dev, struct aw200xx *chip)
+{
+	struct fwnode_handle *child;
+	u32 max_source = 0;
+
+	device_for_each_child_node(dev, child) {
+		u32 source;
+		int ret;
+
+		ret = fwnode_property_read_u32(child, "reg", &source);
+		if (ret || source >= chip->cdef->channels)
+			continue;
+
+		max_source = max(max_source, source);
+	}
+
+	chip->display_rows = max_source / chip->cdef->display_size_columns + 1;
+	if (!chip->display_rows) {
+		dev_err(dev, "No valid led definitions found\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip)
 {
 	struct fwnode_handle *child;
@@ -386,18 +411,8 @@ static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip)
 	int ret;
 	int i;
 
-	ret = device_property_read_u32(dev, "awinic,display-rows",
-				       &chip->display_rows);
-	if (ret)
-		return dev_err_probe(dev, ret,
-				     "Failed to read 'display-rows' property\n");
-
-	if (!chip->display_rows ||
-	    chip->display_rows > chip->cdef->display_size_rows_max) {
-		return dev_err_probe(dev, ret,
-				     "Invalid leds display size %u\n",
-				     chip->display_rows);
-	}
+	if (aw200xx_probe_get_display_rows(dev, chip))
+		return -EINVAL;
 
 	current_max = aw200xx_imax_from_global(chip, AW200XX_IMAX_MAX_uA);
 	current_min = aw200xx_imax_from_global(chip, AW200XX_IMAX_MIN_uA);
-- 
2.36.0

[PATCH v2 05/11] dt-bindings: leds: aw200xx: remove property "awinic,display-rows"

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

Get rid of the property "awinic,display-rows" and calculate it
in the driver using led definition nodes.

Signed-off-by: George Stark <gnstark@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/leds/awinic,aw200xx.yaml         | 28 +++----------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
index 255eb0563737..ee849ef3236a 100644
--- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
+++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
@@ -36,11 +36,6 @@ properties:
   "#size-cells":
     const: 0
 
-  awinic,display-rows:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description:
-      Leds matrix size
-
   hwen-gpios:
     maxItems: 1
 
@@ -63,31 +58,17 @@ patternProperties:
           since the chip has a single global setting.
           The maximum output current of each LED is calculated by the
           following formula:
-            IMAXled = 160000 * (592 / 600.5) * (1 / display-rows)
+            IMAXled = 160000 * (592 / 600.5) * (1 / max-current-switch-number)
           And the minimum output current formula:
-            IMINled = 3300 * (592 / 600.5) * (1 / display-rows)
+            IMINled = 3300 * (592 / 600.5) * (1 / max-current-switch-number)
+          where max-current-switch-number is determinated by led configuration
+          and depends on how leds are physically connected to the led driver.
 
 required:
   - compatible
   - reg
   - "#address-cells"
   - "#size-cells"
-  - awinic,display-rows
-
-allOf:
-  - if:
-      properties:
-        compatible:
-          contains:
-            const: awinic,aw20036
-    then:
-      properties:
-        awinic,display-rows:
-          enum: [1, 2, 3]
-    else:
-      properties:
-        awinic,display-rows:
-          enum: [1, 2, 3, 4, 5, 6, 7]
 
 additionalProperties: false
 
@@ -105,7 +86,6 @@ examples:
             reg = <0x3a>;
             #address-cells = <1>;
             #size-cells = <0>;
-            awinic,display-rows = <3>;
             hwen-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
 
             led@0 {
-- 
2.36.0

[PATCH v2 06/11] leds: aw200xx: add delay after software reset

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

According to datasheets of aw200xx devices software reset takes at
least 1 ms so add delay after reset before issuing commands to device.

Signed-off-by: George Stark <gnstark@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/leds/leds-aw200xx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index a2a31b8e623e..77760406abbf 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -321,6 +321,9 @@ static int aw200xx_chip_reset(const struct aw200xx *const chip)
 	if (ret)
 		return ret;
 
+	/* according to datasheet software reset takes at least 1 ms */
+	fsleep(1000);
+
 	regcache_mark_dirty(chip->regmap);
 	return regmap_write(chip->regmap, AW200XX_REG_FCD, AW200XX_FCD_CLEAR);
 }
-- 
2.36.0

[PATCH v2 07/11] leds: aw200xx: enable disable_locking flag in regmap config

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

In the driver regmap is always used under mutex so regmap's inner lock
can be disabled.

Signed-off-by: George Stark <gnstark@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/leds/leds-aw200xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 77760406abbf..ad4bfb9f0938 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -524,6 +524,7 @@ static const struct regmap_config aw200xx_regmap_config = {
 	.rd_table = &aw200xx_readable_table,
 	.wr_table = &aw200xx_writeable_table,
 	.cache_type = REGCACHE_MAPLE,
+	.disable_locking = true,
 };
 
 static int aw200xx_probe(struct i2c_client *client)
-- 
2.36.0

[PATCH v2 08/11] leds: aw200xx: improve autodim calculation method

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

It is highly recommended to leverage the DIV_ROUND_UP() function as a
more refined and mathematically precise alternative to employing a
coarse division method.

Signed-off-by: George Stark <gnstark@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/leds/leds-aw200xx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index ad4bfb9f0938..7b8802bd9497 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -87,6 +87,8 @@
 #define AW200XX_REG_DIM(x, columns) \
 	AW200XX_REG(AW200XX_PAGE4, AW200XX_LED2REG(x, columns) * 2)
 #define AW200XX_REG_DIM2FADE(x) ((x) + 1)
+#define AW200XX_REG_FADE2DIM(fade) \
+	DIV_ROUND_UP((fade) * AW200XX_DIM_MAX, AW200XX_FADE_MAX)
 
 /*
  * Duty ratio of display scan (see p.15 of datasheet for formula):
@@ -195,9 +197,7 @@ static int aw200xx_brightness_set(struct led_classdev *cdev,
 
 	dim = led->dim;
 	if (dim < 0)
-		dim = max_t(int,
-			    brightness / (AW200XX_FADE_MAX / AW200XX_DIM_MAX),
-			    1);
+		dim = AW200XX_REG_FADE2DIM(brightness);
 
 	ret = regmap_write(chip->regmap, reg, dim);
 	if (ret)
@@ -460,6 +460,7 @@ static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip)
 		led->num = source;
 		led->chip = chip;
 		led->cdev.brightness_set_blocking = aw200xx_brightness_set;
+		led->cdev.max_brightness = AW200XX_FADE_MAX;
 		led->cdev.groups = dim_groups;
 		init_data.fwnode = child;
 
-- 
2.36.0

[PATCH v2 09/11] leds: aw200xx: add support for aw20108 device

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

Add support for Awinic aw20108 device from the same LED drivers famliy.
New device supports 108 leds using matrix of 12x9 outputs.

Signed-off-by: George Stark <gnstark@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/leds/Kconfig        |  8 ++++----
 drivers/leds/leds-aw200xx.c | 10 +++++++++-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6046dfeca16f..40b3f4191cff 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -95,13 +95,13 @@ config LEDS_ARIEL
 	  Say Y to if your machine is a Dell Wyse 3020 thin client.
 
 config LEDS_AW200XX
-	tristate "LED support for Awinic AW20036/AW20054/AW20072"
+	tristate "LED support for Awinic AW20036/AW20054/AW20072/AW20108"
 	depends on LEDS_CLASS
 	depends on I2C
 	help
-	  This option enables support for the AW20036/AW20054/AW20072 LED driver.
-	  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,
+	  This option enables support for the AW20036/AW20054/AW20072/AW20108
+	  LED driver. It is a 3x12/6x9/6x12/9x12 matrix LED driver programmed via
+	  an I2C interface, up to 36/54/72/108 LEDs or 12/18/24/36 RGBs,
 	  3 pattern controllers for auto breathing or group dimming control.
 
 	  To compile this driver as a module, choose M here: the module
diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 7b8802bd9497..529a4ab9c876 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Awinic AW20036/AW20054/AW20072 LED driver
+ * Awinic AW20036/AW20054/AW20072/AW20108 LED driver
  *
  * Copyright (c) 2023, SberDevices. All Rights Reserved.
  *
@@ -616,10 +616,17 @@ static const struct aw200xx_chipdef aw20072_cdef = {
 	.display_size_columns = 12,
 };
 
+static const struct aw200xx_chipdef aw20108_cdef = {
+	.channels = 108,
+	.display_size_rows_max = 9,
+	.display_size_columns = 12,
+};
+
 static const struct i2c_device_id aw200xx_id[] = {
 	{ "aw20036" },
 	{ "aw20054" },
 	{ "aw20072" },
+	{ "aw20108" },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, aw200xx_id);
@@ -628,6 +635,7 @@ static const struct of_device_id aw200xx_match_table[] = {
 	{ .compatible = "awinic,aw20036", .data = &aw20036_cdef, },
 	{ .compatible = "awinic,aw20054", .data = &aw20054_cdef, },
 	{ .compatible = "awinic,aw20072", .data = &aw20072_cdef, },
+	{ .compatible = "awinic,aw20108", .data = &aw20108_cdef, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, aw200xx_match_table);
-- 
2.36.0

[PATCH v2 10/11] dt-bindings: leds: awinic,aw200xx: add AW20108 device

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

Add aw20108 compatible for Awinic AW20108 led controller.

Signed-off-by: George Stark <gnstark@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 .../devicetree/bindings/leds/awinic,aw200xx.yaml          | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
index ee849ef3236a..efb18ddce383 100644
--- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
+++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
@@ -10,15 +10,16 @@ maintainers:
   - Martin Kurbanov <mmkurbanov@sberdevices.ru>
 
 description: |
-  This controller is present on AW20036/AW20054/AW20072.
-  It is a 3x12/6x9/6x12 matrix LED programmed via
-  an I2C interface, up to 36/54/72 LEDs or 12/18/24 RGBs,
+  This controller is present on AW20036/AW20054/AW20072/AW20108.
+  It is a 3x12/6x9/6x12/9x12 matrix LED programmed via
+  an I2C interface, up to 36/54/72/108 LEDs or 12/18/24/36 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/en/productDetail/AW20036QNR#tech-docs
   aw20054 - https://www.awinic.com/en/productDetail/AW20054QNR#tech-docs
   aw20072 - https://www.awinic.com/en/productDetail/AW20072QNR#tech-docs
+  aw20108 - https://www.awinic.com/en/productDetail/AW20108QNR#tech-docs
 
 properties:
   compatible:
@@ -26,6 +27,7 @@ properties:
       - awinic,aw20036
       - awinic,aw20054
       - awinic,aw20072
+      - awinic,aw20108
 
   reg:
     maxItems: 1
-- 
2.36.0

[PATCH v2 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

[Dmitry Rokosov]

AW200XX controllers have the capability to declare more than 0xf LEDs,
therefore, it is necessary to accept LED names using an appropriate
regex pattern.

The register offsets can be adjusted within the specified range, with
the maximum value corresponding to the highest number of LEDs that can
be connected to the controller.

Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 .../devicetree/bindings/leds/awinic,aw200xx.yaml       | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
index efb18ddce383..677c73aa6232 100644
--- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
+++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
@@ -42,16 +42,18 @@ properties:
     maxItems: 1
 
 patternProperties:
-  "^led@[0-9a-f]$":
+  "^led@[0-9a-f]+$":
     type: object
     $ref: common.yaml#
     unevaluatedProperties: false
 
     properties:
       reg:
-        description:
-          LED number
-        maxItems: 1
+        items:
+          description:
+            LED number
+          minimum: 0
+          maximum: 108
 
       led-max-microamp:
         default: 9780
-- 
2.36.0

Re: [PATCH v2 02/11] leds: aw200xx: support HWEN hardware control

[Andy Shevchenko]

On Wed, Oct 18, 2023 at 9:29 PM Dmitry Rokosov
<ddrokosov@salutedevices.com> wrote:
>
> HWEN is hardware control, which is used for enable/disable aw200xx chip.
> It's high active, internally pulled down to GND.
>
> After HWEN pin set high the chip begins to load the OTP information,
> which takes 200us to complete. About 200us wait time is needed for
> internal oscillator startup and display SRAM initialization. After
> display SRAM initialization, the registers in page 1 to page 5 can be
> configured via i2c interface.

Is there any Documentation update for this new binding?

...

> +       chip->hwen = devm_gpiod_get_optional(&client->dev, "hwen", GPIOD_OUT_HIGH);

With _optional APIs we distinguish 3 cases: found, not found, error.
And error can be (among others) the deferred probe, meaning that GPIO
_is coming_. Hence the rule of thumb for the _optional() APIs is to
check for error and bail out on that condition (note, it's applicable
to any _optional() APIs, not limited by GPIO library).

...

>         aw200xx_chip_reset(chip);
> +       aw200xx_disable(chip);

It seems it can be modeled as a (GPIO) regulator. At least many
drivers do so, but I leave this to the maintainers to decide.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 04/11] leds: aw200xx: calculate dts property display_rows in driver

[Andy Shevchenko]

On Wed, Oct 18, 2023 at 9:30 PM Dmitry Rokosov
<ddrokosov@salutedevices.com> wrote:
>
> From: George Stark <gnstark@salutedevices.com>
>
> Get rid of device tree property "awinic,display-rows" and calculate it
> in driver using led definition nodes. display-row actually means number
> of current switches and depends on how leds are connected to the device.

Still the commit message does not answer the question why it's safe
for the users that have this property enabled in their DTBs (note B
letter).

...

> +       device_for_each_child_node(dev, child) {
> +               u32 source;
> +               int ret;
> +
> +               ret = fwnode_property_read_u32(child, "reg", &source);
> +               if (ret || source >= chip->cdef->channels)

Perhaps a warning?

    dev_warn(dev, "Unable to read from %pfw or apply a source channel
number\n", child);

> +                       continue;
> +
> +               max_source = max(max_source, source);
> +       }

...

> +       chip->display_rows = max_source / chip->cdef->display_size_columns + 1;
> +       if (!chip->display_rows) {
> +               dev_err(dev, "No valid led definitions found\n");
> +               return -EINVAL;

So, this part is in ->probe() flow only, correct? If so,
  return dev_err_probe(...);

> +       }

...

> +       if (aw200xx_probe_get_display_rows(dev, chip))
> +               return -EINVAL;

Why is the error code shadowed?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 06/11] leds: aw200xx: add delay after software reset

[Andy Shevchenko]

On Wed, Oct 18, 2023 at 9:30 PM Dmitry Rokosov
<ddrokosov@salutedevices.com> wrote:
>
> From: George Stark <gnstark@salutedevices.com>
>
> According to datasheets of aw200xx devices software reset takes at
> least 1 ms so add delay after reset before issuing commands to device.

For the "us" unit you chose "XXXus" style, why is "ms" different?

...

> +       /* according to datasheet software reset takes at least 1 ms */

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 09/11] leds: aw200xx: add support for aw20108 device

[Andy Shevchenko]

On Wed, Oct 18, 2023 at 9:30 PM Dmitry Rokosov
<ddrokosov@salutedevices.com> wrote:
>
> From: George Stark <gnstark@salutedevices.com>
>
> Add support for Awinic aw20108 device from the same LED drivers famliy.

family

> New device supports 108 leds using matrix of 12x9 outputs.

LEDs
a matrix

...

> -         This option enables support for the AW20036/AW20054/AW20072 LED driver.
> -         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,
> +         This option enables support for the AW20036/AW20054/AW20072/AW20108
> +         LED driver. It is a 3x12/6x9/6x12/9x12 matrix LED driver programmed via
> +         an I2C interface, up to 36/54/72/108 LEDs or 12/18/24/36 RGBs,
>           3 pattern controllers for auto breathing or group dimming control.

For better maintenance I always suggest in the cases like this to
convert help to provide a list of the supported devices, like:

  This option enables support for the following Awinic LED drivers:
    - AW20036 (3x12)
    - ...
   ...

And if any new comes to this, it will be just a one liner change.

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 09/11] leds: aw200xx: add support for aw20108 device

[Andy Shevchenko]

On Thu, Oct 19, 2023 at 12:10 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Oct 18, 2023 at 9:30 PM Dmitry Rokosov
> <ddrokosov@salutedevices.com> wrote:

...

> > -         This option enables support for the AW20036/AW20054/AW20072 LED driver.
> > -         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,
> > +         This option enables support for the AW20036/AW20054/AW20072/AW20108
> > +         LED driver. It is a 3x12/6x9/6x12/9x12 matrix LED driver programmed via
> > +         an I2C interface, up to 36/54/72/108 LEDs or 12/18/24/36 RGBs,
> >           3 pattern controllers for auto breathing or group dimming control.
>
> For better maintenance I always suggest in the cases like this to
> convert help to provide a list of the supported devices, like:
>
>   This option enables support for the following Awinic LED drivers:
>     - AW20036 (3x12)

(and other specifics can be listed in parentheses, or in free way, but
short enough to occupy only a single line)

>     - ...
>    ...
>
> And if any new comes to this, it will be just a one liner change.

And you may do that conversion as a precursor to this one and you will
see what I mean.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 10/11] dt-bindings: leds: awinic,aw200xx: add AW20108 device

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

On Wed, Oct 18, 2023 at 09:29:42PM +0300, Dmitry Rokosov wrote:
> From: George Stark <gnstark@salutedevices.com>
> 
> Add aw20108 compatible for Awinic AW20108 led controller.
> 
> Signed-off-by: George Stark <gnstark@salutedevices.com>
> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> ---
>  .../devicetree/bindings/leds/awinic,aw200xx.yaml          | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> index ee849ef3236a..efb18ddce383 100644
> --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> @@ -10,15 +10,16 @@ maintainers:
>    - Martin Kurbanov <mmkurbanov@sberdevices.ru>
>  
>  description: |
> -  This controller is present on AW20036/AW20054/AW20072.
> -  It is a 3x12/6x9/6x12 matrix LED programmed via
> -  an I2C interface, up to 36/54/72 LEDs or 12/18/24 RGBs,
> +  This controller is present on AW20036/AW20054/AW20072/AW20108.
> +  It is a 3x12/6x9/6x12/9x12 matrix LED programmed via
> +  an I2C interface, up to 36/54/72/108 LEDs or 12/18/24/36 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/en/productDetail/AW20036QNR#tech-docs
>    aw20054 - https://www.awinic.com/en/productDetail/AW20054QNR#tech-docs
>    aw20072 - https://www.awinic.com/en/productDetail/AW20072QNR#tech-docs
> +  aw20108 - https://www.awinic.com/en/productDetail/AW20108QNR#tech-docs
>  
>  properties:
>    compatible:
> @@ -26,6 +27,7 @@ properties:
>        - awinic,aw20036
>        - awinic,aw20054
>        - awinic,aw20072
> +      - awinic,aw20108
>  
>    reg:
>      maxItems: 1
> -- 
> 2.36.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]

On Wed, Oct 18, 2023 at 09:29:43PM +0300, Dmitry Rokosov wrote:
> AW200XX controllers have the capability to declare more than 0xf LEDs,
> therefore, it is necessary to accept LED names using an appropriate
> regex pattern.
> 
> The register offsets can be adjusted within the specified range, with
> the maximum value corresponding to the highest number of LEDs that can
> be connected to the controller.

Do all of these controllers have identical max numbers of LEDs?

Cheers,
Conor.

> 
> Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> ---
>  .../devicetree/bindings/leds/awinic,aw200xx.yaml       | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> index efb18ddce383..677c73aa6232 100644
> --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> @@ -42,16 +42,18 @@ properties:
>      maxItems: 1
>  
>  patternProperties:
> -  "^led@[0-9a-f]$":
> +  "^led@[0-9a-f]+$":
>      type: object
>      $ref: common.yaml#
>      unevaluatedProperties: false
>  
>      properties:
>        reg:
> -        description:
> -          LED number
> -        maxItems: 1
> +        items:
> +          description:
> +            LED number
> +          minimum: 0
> +          maximum: 108
>  
>        led-max-microamp:
>          default: 9780
> -- 
> 2.36.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 03/11] dt-bindings: leds: aw200xx: introduce optional hwen-gpios property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

On Wed, Oct 18, 2023 at 09:29:35PM +0300, Dmitry Rokosov wrote:
> Property 'hwen-gpios' is optional, it can be used by the board
> developer to connect AW200XX LED controller with appropriate poweron
> GPIO pad.

If the pad is called "poweron", why is the property called "hwen"?

> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> ---
>  Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> index feb5febaf361..255eb0563737 100644
> --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> @@ -41,6 +41,9 @@ properties:
>      description:
>        Leds matrix size
>  
> +  hwen-gpios:
> +    maxItems: 1
> +
>  patternProperties:
>    "^led@[0-9a-f]$":
>      type: object
> @@ -90,6 +93,7 @@ additionalProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/gpio/gpio.h>
>      #include <dt-bindings/leds/common.h>
>  
>      i2c {
> @@ -102,6 +106,7 @@ examples:
>              #address-cells = <1>;
>              #size-cells = <0>;
>              awinic,display-rows = <3>;
> +            hwen-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
>  
>              led@0 {
>                  reg = <0x0>;
> -- 
> 2.36.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 04/11] leds: aw200xx: calculate dts property display_rows in driver

[George Stark]

Hello Andy

On 10/19/23 12:01, Andy Shevchenko wrote:
> On Wed, Oct 18, 2023 at 9:30 PM Dmitry Rokosov
> <ddrokosov@salutedevices.com> wrote:
>>
>> From: George Stark <gnstark@salutedevices.com>
>>
>> Get rid of device tree property "awinic,display-rows" and calculate it
>> in driver using led definition nodes. display-row actually means number
>> of current switches and depends on how leds are connected to the device.
> 
> Still the commit message does not answer the question why it's safe
> for the users that have this property enabled in their DTBs (note B
> letter).
> 


> ...
> 
>> +       device_for_each_child_node(dev, child) {
>> +               u32 source;
>> +               int ret;
>> +
>> +               ret = fwnode_property_read_u32(child, "reg", &source);
>> +               if (ret || source >= chip->cdef->channels)
> 
> Perhaps a warning?
> 
>      dev_warn(dev, "Unable to read from %pfw or apply a source channel
> number\n", child);

I skipped the warning intentionally because we have just the same loop 
several steps below and with the same check. There we have all proper 
warnings and aw200xx_probe_get_display_rows was intended to be short and 
lightweight. I'll redesign it to be even more simple.

> 
>> +                       continue;
>> +
>> +               max_source = max(max_source, source);
>> +       }
> 
> ...
> 
>> +       chip->display_rows = max_source / chip->cdef->display_size_columns + 1;
>> +       if (!chip->display_rows) {
>> +               dev_err(dev, "No valid led definitions found\n");
>> +               return -EINVAL;
> 
> So, this part is in ->probe() flow only, correct? If so,
>    return dev_err_probe(...);
> 
>> +       }
> 
> ...
> 
>> +       if (aw200xx_probe_get_display_rows(dev, chip))
>> +               return -EINVAL;
> 
> Why is the error code shadowed?
> 

-- 
Best regards
George

Re: [PATCH v2 03/11] dt-bindings: leds: aw200xx: introduce optional hwen-gpios property

[Rob Herring]

On Wed, Oct 18, 2023 at 09:29:35PM +0300, Dmitry Rokosov wrote:
> Property 'hwen-gpios' is optional, it can be used by the board
> developer to connect AW200XX LED controller with appropriate poweron
> GPIO pad.
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> ---
>  Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> index feb5febaf361..255eb0563737 100644
> --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> @@ -41,6 +41,9 @@ properties:
>      description:
>        Leds matrix size
>  
> +  hwen-gpios:
> +    maxItems: 1

The standard enable-gpios or powerdown-gpios don't work for you?

Re: [DMARC error] Re: [PATCH v2 03/11] dt-bindings: leds: aw200xx: introduce optional hwen-gpios property

[Dmitry Rokosov]

On Tue, Oct 24, 2023 at 01:30:14PM -0500, Rob Herring wrote:
> On Wed, Oct 18, 2023 at 09:29:35PM +0300, Dmitry Rokosov wrote:
> > Property 'hwen-gpios' is optional, it can be used by the board
> > developer to connect AW200XX LED controller with appropriate poweron
> > GPIO pad.
> > 
> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> > ---
> >  Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > index feb5febaf361..255eb0563737 100644
> > --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > @@ -41,6 +41,9 @@ properties:
> >      description:
> >        Leds matrix size
> >  
> > +  hwen-gpios:
> > +    maxItems: 1
> 
> The standard enable-gpios or powerdown-gpios don't work for you?

HWEN is the name from the official datasheet. I thought it's always
better to use a naming convention that is similar to the notations used
in the datasheet.

-- 
Thank you,
Dmitry

Re: [PATCH v2 03/11] dt-bindings: leds: aw200xx: introduce optional hwen-gpios property

[Dmitry Rokosov]

On Thu, Oct 19, 2023 at 03:11:06PM +0100, Conor Dooley wrote:
> On Wed, Oct 18, 2023 at 09:29:35PM +0300, Dmitry Rokosov wrote:
> > Property 'hwen-gpios' is optional, it can be used by the board
> > developer to connect AW200XX LED controller with appropriate poweron
> > GPIO pad.
> 
> If the pad is called "poweron", why is the property called "hwen"?
> 

I have just referred to GPIO as 'poweron gpio', which is my own figure
of speech. In actuality, this pin is officially referred to as 'hwen' in
the datasheet.

> > 
> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> > ---
> >  Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > index feb5febaf361..255eb0563737 100644
> > --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > @@ -41,6 +41,9 @@ properties:
> >      description:
> >        Leds matrix size
> >  
> > +  hwen-gpios:
> > +    maxItems: 1
> > +
> >  patternProperties:
> >    "^led@[0-9a-f]$":
> >      type: object
> > @@ -90,6 +93,7 @@ additionalProperties: false
> >  
> >  examples:
> >    - |
> > +    #include <dt-bindings/gpio/gpio.h>
> >      #include <dt-bindings/leds/common.h>
> >  
> >      i2c {
> > @@ -102,6 +106,7 @@ examples:
> >              #address-cells = <1>;
> >              #size-cells = <0>;
> >              awinic,display-rows = <3>;
> > +            hwen-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
> >  
> >              led@0 {
> >                  reg = <0x0>;
> > -- 
> > 2.36.0
> > 



-- 
Thank you,
Dmitry

Re: [PATCH v2 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

[Dmitry Rokosov]

On Thu, Oct 19, 2023 at 03:08:38PM +0100, Conor Dooley wrote:
> On Wed, Oct 18, 2023 at 09:29:43PM +0300, Dmitry Rokosov wrote:
> > AW200XX controllers have the capability to declare more than 0xf LEDs,
> > therefore, it is necessary to accept LED names using an appropriate
> > regex pattern.
> > 
> > The register offsets can be adjusted within the specified range, with
> > the maximum value corresponding to the highest number of LEDs that can
> > be connected to the controller.
> 
> Do all of these controllers have identical max numbers of LEDs?

Nope... I believe you are hinting at some conditional logic based on the
value of 'compatible'. I will figure it out and send the appropriate
implementation in the next version.

> > 
> > Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> > ---
> >  .../devicetree/bindings/leds/awinic,aw200xx.yaml       | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > index efb18ddce383..677c73aa6232 100644
> > --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > @@ -42,16 +42,18 @@ properties:
> >      maxItems: 1
> >  
> >  patternProperties:
> > -  "^led@[0-9a-f]$":
> > +  "^led@[0-9a-f]+$":
> >      type: object
> >      $ref: common.yaml#
> >      unevaluatedProperties: false
> >  
> >      properties:
> >        reg:
> > -        description:
> > -          LED number
> > -        maxItems: 1
> > +        items:
> > +          description:
> > +            LED number
> > +          minimum: 0
> > +          maximum: 108
> >  
> >        led-max-microamp:
> >          default: 9780
> > -- 
> > 2.36.0
> > 



-- 
Thank you,
Dmitry

Re: [DMARC error] Re: [PATCH v2 03/11] dt-bindings: leds: aw200xx: introduce optional hwen-gpios property

[Krzysztof Kozlowski]

On 24/10/2023 20:52, Dmitry Rokosov wrote:
> On Tue, Oct 24, 2023 at 01:30:14PM -0500, Rob Herring wrote:
>> On Wed, Oct 18, 2023 at 09:29:35PM +0300, Dmitry Rokosov wrote:
>>> Property 'hwen-gpios' is optional, it can be used by the board
>>> developer to connect AW200XX LED controller with appropriate poweron
>>> GPIO pad.
>>>
>>> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
>>> ---
>>>  Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
>>> index feb5febaf361..255eb0563737 100644
>>> --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
>>> +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
>>> @@ -41,6 +41,9 @@ properties:
>>>      description:
>>>        Leds matrix size
>>>  
>>> +  hwen-gpios:
>>> +    maxItems: 1
>>
>> The standard enable-gpios or powerdown-gpios don't work for you?
> 
> HWEN is the name from the official datasheet. I thought it's always
> better to use a naming convention that is similar to the notations used
> in the datasheet.

I think we have such rule only for supplies, otherwise you will have
multiple variants of the same reset/enable/powerdown-gpios.

Best regards,
Krzysztof