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

[PATCH v5 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 v5 since v4 at [5]:
    - make several lowercase->uppercase renaming suggested by Lee Jones
    - change aw200xx_probe_get_display_rows() return value type from
      boolean to integer and propagate error return values to the
      dev_err_probe()
    - fix some grammar problems

Changes v4 since v3 at [4]:
    - properly handle max_source = 0 situations
    - fix Rob's dt_binding_check alerts

Changes v3 since v2 at [3]:
    - handle all cases during hwen gpio get routine execution
    - rename 'hwen-gpios' to standard 'enable-gpios'
    - properly handle aw200xx_probe_get_display_rows() ret values
    - fix timestamp format in the comments and commit messages
    - expand LEDS_AW200XX config and dt-bindings description
    - describe reg constraints for all compatible variants
    - add Conor's Acked-by tag

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/
    [3] https://lore.kernel.org/all/20231018182943.18700-1-ddrokosov@salutedevices.com/
    [4] https://lore.kernel.org/all/20231101142445.8753-1-ddrokosov@salutedevices.com/
    [5] https://lore.kernel.org/all/20231121202835.28152-1-ddrokosov@salutedevices.com/

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

George Stark (7):
  leds: aw200xx: calculate dts property display_rows in the 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         | 95 +++++++++++++-----
 drivers/leds/Kconfig                          | 14 ++-
 drivers/leds/leds-aw200xx.c                   | 98 ++++++++++++++++---
 3 files changed, 163 insertions(+), 44 deletions(-)

-- 
2.36.0

[PATCH v5 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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 v5 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 | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 842a22087b16..d8f2fe57cbb7 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,14 @@ 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, "enable",
+					     GPIOD_OUT_HIGH);
+	if (IS_ERR(chip->hwen))
+		return dev_err_probe(&client->dev, PTR_ERR(chip->hwen),
+				     "Cannot get enable GPIO");
+
+	aw200xx_enable(chip);
+
 	ret = aw200xx_chip_check(chip);
 	if (ret)
 		return ret;
@@ -537,6 +566,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 +578,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 v5 03/11] dt-bindings: leds: aw200xx: introduce optional enable-gpios property

[Dmitry Rokosov]

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

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 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..3da3633a242c 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
 
+  enable-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>;
+            enable-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
 
             led@0 {
                 reg = <0x0>;
-- 
2.36.0

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

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

Get rid of device tree property "awinic,display-rows". The property
value actually means number of current switches and depends on how LEDs
are connected to the device. It should be calculated manually by max
used LED number. In the same way it is computed automatically now.
Max used LED is taken from LED definition subnodes.

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

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index d8f2fe57cbb7..91ba580edf87 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);
+	}
+
+	if (max_source == 0)
+		return -EINVAL;
+
+	chip->display_rows = max_source / chip->cdef->display_size_columns + 1;
+
+	return 0;
+}
+
 static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip)
 {
 	struct fwnode_handle *child;
@@ -386,18 +411,10 @@ 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);
+	ret = aw200xx_probe_get_display_rows(dev, chip);
 	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);
-	}
+				     "No valid led definitions found\n");
 
 	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 v5 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 3da3633a242c..a6dced59599d 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
-
   enable-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>;
             enable-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
 
             led@0 {
-- 
2.36.0

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

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

According to the datasheets of AW200xx devices, the software reset takes
at least 1ms. Therefore, it is required to add a delay after the
reset before issuing commands to the 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 91ba580edf87..12c4251211dd 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 the datasheet software reset takes at least 1ms */
+	fsleep(1000);
+
 	regcache_mark_dirty(chip->regmap);
 	return regmap_write(chip->regmap, AW200XX_REG_FCD, AW200XX_FCD_CLEAR);
 }
-- 
2.36.0

[PATCH v5 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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 12c4251211dd..c6a51152a494 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -526,6 +526,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 v5 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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 c6a51152a494..e125b6c68c23 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)
@@ -462,6 +462,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 v5 09/11] leds: aw200xx: add support for aw20108 device

[Dmitry Rokosov]

From: George Stark <gnstark@salutedevices.com>

Add support for the Awinic aw20108 device, which belongs to the same LED
drivers family. The new device supports 108 LEDs using a matrix of 12x9
outputs."

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

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6046dfeca16f..a898a8679334 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -95,14 +95,18 @@ 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,
-	  3 pattern controllers for auto breathing or group dimming control.
+	  This option enables support for the Awinic AW200XX LED controllers.
+	  It is a matrix LED driver programmed via an I2C interface. Devices have
+	  a set of individually controlled LEDs and support 3 pattern controllers
+	  for auto breathing or group dimming control. Supported devices:
+	    - AW20036 (3x12) 36 LEDs
+	    - AW20054 (6x9)  54 LEDs
+	    - AW20072 (6x12) 72 LEDs
+	    - AW20108 (9x12) 108 LEDs
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-aw200xx.
diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index e125b6c68c23..1d3943f86f7f 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.
  *
@@ -622,10 +622,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);
@@ -634,6 +641,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 v5 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>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../devicetree/bindings/leds/awinic,aw200xx.yaml    | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
index a6dced59599d..67c1d960db1d 100644
--- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
+++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
@@ -10,15 +10,19 @@ 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,
-  3 pattern controllers for auto breathing or group dimming control.
+  It is a matrix LED driver programmed via an I2C interface. Devices have
+  a set of individually controlled leds and support 3 pattern controllers
+  for auto breathing or group dimming control. Supported devices:
+    - AW20036 (3x12) 36 LEDs
+    - AW20054 (6x9)  54 LEDs
+    - AW20072 (6x12) 72 LEDs
+    - AW20108 (9x12) 108 LEDs
 
   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 +30,7 @@ properties:
       - awinic,aw20036
       - awinic,aw20054
       - awinic,aw20072
+      - awinic,aw20108
 
   reg:
     maxItems: 1
-- 
2.36.0

[PATCH v5 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>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/leds/awinic,aw200xx.yaml         | 59 ++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
index 67c1d960db1d..54d6d1f08e24 100644
--- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
+++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
@@ -45,7 +45,7 @@ properties:
     maxItems: 1
 
 patternProperties:
-  "^led@[0-9a-f]$":
+  "^led@[0-9a-f]+$":
     type: object
     $ref: common.yaml#
     unevaluatedProperties: false
@@ -69,6 +69,63 @@ patternProperties:
           where max-current-switch-number is determinated by led configuration
           and depends on how leds are physically connected to the led driver.
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: awinic,aw20036
+    then:
+      patternProperties:
+        "^led@[0-9a-f]+$":
+          properties:
+            reg:
+              items:
+                minimum: 0
+                maximum: 36
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: awinic,aw20054
+    then:
+      patternProperties:
+        "^led@[0-9a-f]+$":
+          properties:
+            reg:
+              items:
+                minimum: 0
+                maximum: 54
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: awinic,aw20072
+    then:
+      patternProperties:
+        "^led@[0-9a-f]+$":
+          properties:
+            reg:
+              items:
+                minimum: 0
+                maximum: 72
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: awinic,aw20108
+    then:
+      patternProperties:
+        "^led@[0-9a-f]+$":
+          properties:
+            reg:
+              items:
+                minimum: 0
+                maximum: 108
+
 required:
   - compatible
   - reg
-- 
2.36.0

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

[Andy Shevchenko]

On Sat, Nov 25, 2023 at 10:05 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.

...

> +       chip->hwen = devm_gpiod_get_optional(&client->dev, "enable",
> +                                            GPIOD_OUT_HIGH);
> +       if (IS_ERR(chip->hwen))
> +               return dev_err_probe(&client->dev, PTR_ERR(chip->hwen),
> +                                    "Cannot get enable GPIO");

You can make it neater with

  struct device *dev = &client->dev;

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Sat, Nov 25, 2023 at 10:05 PM Dmitry Rokosov
<ddrokosov@salutedevices.com> wrote:
>
> From: George Stark <gnstark@salutedevices.com>
>
> Get rid of device tree property "awinic,display-rows". The property
> value actually means number of current switches and depends on how LEDs
> are connected to the device. It should be calculated manually by max
> used LED number. In the same way it is computed automatically now.
> Max used LED is taken from LED definition subnodes.

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

-- 
With Best Regards,
Andy Shevchenko

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

[Lee Jones]

On Sat, 25 Nov 2023 23:05:08 +0300, Dmitry Rokosov wrote:
> 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].
> 
> [...]

Applied, thanks!

[01/11] leds: aw200xx: fix write to DIM parameter
        commit: 785fec3a8daff2957fd55e49cbdfe0a50866fdb7
[02/11] leds: aw200xx: support HWEN hardware control
        commit: eabe8239022cf3c75b90d9ee07dcfbbe4e50bcac
[03/11] dt-bindings: leds: aw200xx: introduce optional enable-gpios property
        commit: e91899ea3759d04e185721153a036e1a25e315b7
[04/11] leds: aw200xx: calculate dts property display_rows in the driver
        commit: 4ccd392c3ea7ceefbee58622e634d4997ef46acc
[05/11] dt-bindings: leds: aw200xx: remove property "awinic,display-rows"
        commit: 66d078f105837670c52bb31da29e26ad13bc2923
[06/11] leds: aw200xx: add delay after software reset
        commit: aac13e5630d6e081a9f6c5a57e5e6fc1152acca8
[07/11] leds: aw200xx: enable disable_locking flag in regmap config
        commit: 851fa70b9b162bbf5b5f5f92fc450633e6b21a3a
[08/11] leds: aw200xx: improve autodim calculation method
        commit: 5fcc24b92b43f012cbf430244f0698ff588ec9fc
[09/11] leds: aw200xx: add support for aw20108 device
        commit: abc74724d5e714bb3359124f3576d5318828a83e
[10/11] dt-bindings: leds: awinic,aw200xx: add AW20108 device
        commit: d6bbe677add2c560ae4aa2f9dab7a19c287e2193
[11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints
        commit: 5707a06e5391a4eeaf0c2705f973336537a41c79

--
Lee Jones [李琼斯]

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

[Dmitry Rokosov]

Hello Lee,

On Fri, Dec 01, 2023 at 11:09:46AM +0000, Lee Jones wrote:
> On Sat, 25 Nov 2023 23:05:08 +0300, Dmitry Rokosov wrote:
> > 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].
> > 
> > [...]
> 
> Applied, thanks!
> 
> [01/11] leds: aw200xx: fix write to DIM parameter
>         commit: 785fec3a8daff2957fd55e49cbdfe0a50866fdb7
> [02/11] leds: aw200xx: support HWEN hardware control
>         commit: eabe8239022cf3c75b90d9ee07dcfbbe4e50bcac
> [03/11] dt-bindings: leds: aw200xx: introduce optional enable-gpios property
>         commit: e91899ea3759d04e185721153a036e1a25e315b7
> [04/11] leds: aw200xx: calculate dts property display_rows in the driver
>         commit: 4ccd392c3ea7ceefbee58622e634d4997ef46acc
> [05/11] dt-bindings: leds: aw200xx: remove property "awinic,display-rows"
>         commit: 66d078f105837670c52bb31da29e26ad13bc2923
> [06/11] leds: aw200xx: add delay after software reset
>         commit: aac13e5630d6e081a9f6c5a57e5e6fc1152acca8
> [07/11] leds: aw200xx: enable disable_locking flag in regmap config
>         commit: 851fa70b9b162bbf5b5f5f92fc450633e6b21a3a
> [08/11] leds: aw200xx: improve autodim calculation method
>         commit: 5fcc24b92b43f012cbf430244f0698ff588ec9fc
> [09/11] leds: aw200xx: add support for aw20108 device
>         commit: abc74724d5e714bb3359124f3576d5318828a83e
> [10/11] dt-bindings: leds: awinic,aw200xx: add AW20108 device
>         commit: d6bbe677add2c560ae4aa2f9dab7a19c287e2193
> [11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints
>         commit: 5707a06e5391a4eeaf0c2705f973336537a41c79

Thank you for applying the patch series!

Could you please advise where I can find the above commits? I've tried
looking in the 'lee/leds' branch, but I couldn't find anything. I want
to cherry-pick the commits that you applied to my internal branch, which
I sync with the upstream periodically.

-- 
Thank you,
Dmitry

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

[Lee Jones]

On Wed, 06 Dec 2023, Dmitry Rokosov wrote:

> Hello Lee,
> 
> On Fri, Dec 01, 2023 at 11:09:46AM +0000, Lee Jones wrote:
> > On Sat, 25 Nov 2023 23:05:08 +0300, Dmitry Rokosov wrote:
> > > 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].
> > > 
> > > [...]
> > 
> > Applied, thanks!
> > 
> > [01/11] leds: aw200xx: fix write to DIM parameter
> >         commit: 785fec3a8daff2957fd55e49cbdfe0a50866fdb7
> > [02/11] leds: aw200xx: support HWEN hardware control
> >         commit: eabe8239022cf3c75b90d9ee07dcfbbe4e50bcac
> > [03/11] dt-bindings: leds: aw200xx: introduce optional enable-gpios property
> >         commit: e91899ea3759d04e185721153a036e1a25e315b7
> > [04/11] leds: aw200xx: calculate dts property display_rows in the driver
> >         commit: 4ccd392c3ea7ceefbee58622e634d4997ef46acc
> > [05/11] dt-bindings: leds: aw200xx: remove property "awinic,display-rows"
> >         commit: 66d078f105837670c52bb31da29e26ad13bc2923
> > [06/11] leds: aw200xx: add delay after software reset
> >         commit: aac13e5630d6e081a9f6c5a57e5e6fc1152acca8
> > [07/11] leds: aw200xx: enable disable_locking flag in regmap config
> >         commit: 851fa70b9b162bbf5b5f5f92fc450633e6b21a3a
> > [08/11] leds: aw200xx: improve autodim calculation method
> >         commit: 5fcc24b92b43f012cbf430244f0698ff588ec9fc
> > [09/11] leds: aw200xx: add support for aw20108 device
> >         commit: abc74724d5e714bb3359124f3576d5318828a83e
> > [10/11] dt-bindings: leds: awinic,aw200xx: add AW20108 device
> >         commit: d6bbe677add2c560ae4aa2f9dab7a19c287e2193
> > [11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints
> >         commit: 5707a06e5391a4eeaf0c2705f973336537a41c79
> 
> Thank you for applying the patch series!
> 
> Could you please advise where I can find the above commits? I've tried
> looking in the 'lee/leds' branch, but I couldn't find anything. I want

They there now and should be in -next by tomorrow.

> to cherry-pick the commits that you applied to my internal branch, which
> I sync with the upstream periodically.

I suggest that a rebase might be a better approach.

-- 
Lee Jones [李琼斯]

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

[Dmitry Rokosov]

On Wed, Dec 06, 2023 at 01:11:34PM +0000, Lee Jones wrote:
> On Wed, 06 Dec 2023, Dmitry Rokosov wrote:
> 
> > Hello Lee,
> > 
> > On Fri, Dec 01, 2023 at 11:09:46AM +0000, Lee Jones wrote:
> > > On Sat, 25 Nov 2023 23:05:08 +0300, Dmitry Rokosov wrote:
> > > > 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].
> > > > 
> > > > [...]
> > > 
> > > Applied, thanks!
> > > 
> > > [01/11] leds: aw200xx: fix write to DIM parameter
> > >         commit: 785fec3a8daff2957fd55e49cbdfe0a50866fdb7
> > > [02/11] leds: aw200xx: support HWEN hardware control
> > >         commit: eabe8239022cf3c75b90d9ee07dcfbbe4e50bcac
> > > [03/11] dt-bindings: leds: aw200xx: introduce optional enable-gpios property
> > >         commit: e91899ea3759d04e185721153a036e1a25e315b7
> > > [04/11] leds: aw200xx: calculate dts property display_rows in the driver
> > >         commit: 4ccd392c3ea7ceefbee58622e634d4997ef46acc
> > > [05/11] dt-bindings: leds: aw200xx: remove property "awinic,display-rows"
> > >         commit: 66d078f105837670c52bb31da29e26ad13bc2923
> > > [06/11] leds: aw200xx: add delay after software reset
> > >         commit: aac13e5630d6e081a9f6c5a57e5e6fc1152acca8
> > > [07/11] leds: aw200xx: enable disable_locking flag in regmap config
> > >         commit: 851fa70b9b162bbf5b5f5f92fc450633e6b21a3a
> > > [08/11] leds: aw200xx: improve autodim calculation method
> > >         commit: 5fcc24b92b43f012cbf430244f0698ff588ec9fc
> > > [09/11] leds: aw200xx: add support for aw20108 device
> > >         commit: abc74724d5e714bb3359124f3576d5318828a83e
> > > [10/11] dt-bindings: leds: awinic,aw200xx: add AW20108 device
> > >         commit: d6bbe677add2c560ae4aa2f9dab7a19c287e2193
> > > [11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints
> > >         commit: 5707a06e5391a4eeaf0c2705f973336537a41c79
> > 
> > Thank you for applying the patch series!
> > 
> > Could you please advise where I can find the above commits? I've tried
> > looking in the 'lee/leds' branch, but I couldn't find anything. I want
> 
> They there now and should be in -next by tomorrow.
> 

Thank you, got it!

> > to cherry-pick the commits that you applied to my internal branch, which
> > I sync with the upstream periodically.
> 
> I suggest that a rebase might be a better approach.

Of course, you are absolutely correct! We regularly perform rebases on
our mainline mirror. However, the patches that we are preparing for the
upstream are managed through our internal Gerrit system. When the
maintainer merges these patches into their tree, we also merge them on
the Gerrit side. Therefore, I would like to inquire about the current
status of these patches.

-- 
Thank you,
Dmitry