leds: aw2013: Add AW2027 support

Started by Paul Sajna on Nov 8, 2026 · 9 messages from 2 authors · Last reply Nov 8, 2026

Series

4 patches, 2 files

+146 −37

Subsystems (3)

Maintainers (6)

Show 2 more
COOLING7d SUPERSEDED

Status: Superseded by v7 (posted 2026-11-14). [go to v7]

Revision v4 (4 revisions archived).

Revisions (4)
  1. v2
  2. v3 [diff against previous]
  3. v4 current [diff against previous]
  4. v7 [diff against previous]

open the first message on its own page

  1. [PATCH v4 1/4] dt-bindings: leds: aw2013: Add awinic,aw2027 compatible
    Revisions and related patches
    Revisions (4)
    1. v2
    2. v3 [diff against previous]
    3. v4 current [diff against previous]
    4. v7 [diff against previous]
  2. [PATCH v4 2/4] leds: aw2013: Add AW2027 support
    Revisions and related patches
    Revisions (4)
    1. v2
    2. v3 [diff against previous]
    3. v4 current [diff against previous]
    4. v7 [diff against previous]
  3. [PATCH v4 3/4] leds: aw2013: Use dev_err_probe
    Revisions and related patches
    Revisions (4)
    1. v2
    2. v3 [diff against previous]
    3. v4 current [diff against previous]
    4. v7 [diff against previous]
  4. [PATCH v4 4/4] leds: aw2013: Move assignment of chip->num_leds
    Revisions and related patches
    Revisions (4)
    1. v2
    2. v3 [diff against previous]
    3. v4 current [diff against previous]
    4. v7 [diff against previous]
Message details and related content
Refer to PATCH 2 for a full description of the similarities and
differences between AW2013 and AW2027.

I'm hoping the sashiko reviews about unbalanced regulators that I was 
struggling to fix are covered by 
https://lore.kernel.org/linux-leds/20261021123406.3163547-1-lee@kernel.org/
and I can drop my failed attempts to fix them.
Let me know if I misunderstood.

I'll do my best to address all of Sashiko's comments, but it might take
me some time to circle back, don't bother reviewing until I've done
this.

This PATCH resulted out of discussions pertaining to
sdm660-mainline/linux, a close-to-mainline fork for Qualcomm Snapdragon 660
devices, specifically for the BlackBerry KEY2 Keyboard Backlight,
in the following GitHub PR: https://github.com/sdm660-mainline/linux/pull/218

Signed-off-by: Paul Sajna <redacted>
---
Changes in v4:
- Drop sashiko fixes covered by https://lore.kernel.org/linux-leds/20261021123406.3163547-1-lee@kernel.org/
- Rename the chip id register value to reflect that it covers more chips
  (https://www.awinic.com/en/driver/AW2027DNR/640)
- Address preliminary review comments from Lee Jones
- Link to v3: https://lore.kernel.org/r/20261012-aw2013-aw20xx-rename-v3-0-cab1fdd050b6@postmarketos.org

Changes in v3:
- Drop rename
- Capitalize commit subject
- Extra tweaks to cleanup patches (preventing more unbalanced regulator
  stuff)
- Whitespace fiddling
- Link to v2: https://lore.kernel.org/r/20261005-aw2013-aw20xx-rename-v2-0-108ecbdf2775@postmarketos.org

Changes in v2:
- Reduce renames to internal driver scope, to prevent breaking compat
- Address Sashiko Review Comments
- Split renames into a separate commit to make the actual changes easier
to identify

21 more lines

[PATCH v4 1/4] dt-bindings: leds: aw2013: Add awinic,aw2027 compatible

Message details and related content
The aw2027 is largely the same as the aw2013, with the
exception of the way the max current is written to registers.
This exception makes it different enough to not work without
driver modifications, but similar enough to fall under the
umbrella of the same driver.

Acked-by: Krzysztof Kozlowski <redacted>
Signed-off-by: Paul Sajna <redacted>
---
 Documentation/devicetree/bindings/leds/leds-aw2013.yaml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/leds-aw2013.yaml b/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
index 26238446f2bd..1ad165cfe34a 100644
--- a/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/leds/leds-aw2013.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: AWINIC AW2013 3-channel LED Driver
+title: AWINIC AW2013/AW2027 3-channel LED Driver
 
 maintainers:
   - Nikita Travkin <nikitos.tr@gmail.com>
@@ -15,7 +15,9 @@ description: |
 
 properties:
   compatible:
-    const: awinic,aw2013
+    enum:
+      - awinic,aw2013
+      - awinic,aw2027
 
   reg:
     maxItems: 1
-- 
2.56.0

[PATCH v4 2/4] leds: aw2013: Add AW2027 support

Message details and related content
The AW2027 is basically the same as the AW2013.

It differs in:
- the max current (30mA rather than 15)
- the number of current levels for colour-mixing (16 rather than 4)
- the supply voltage max
- the charge indicator (unsupported by our driver at this time)

The max current register (imax) also moves from LCFG to GCR2, and the
selectable current levels for colour-mixing (LCFG_CUR) take their place.

The current driver implementation provided here sets the global imax to
it's maximum, and scales each led to the closest value using the local
imax. It's the best result achievable short of adding custom sysfs
entries and mucking up the linux interfaces.

Comparison of all AW20XX chips https://www.awinic.com/en/productCompareList?id=47&ids=1022,74,96,75,72,71,928,927,73,76,578
AW2027 datasheet: https://doc.awinic.com/doc/202507/77b87097-27bf-48cd-af9b-9b615064c1e1.pdf
Vendor driver: https://www.awinic.com/en/driver/AW2027DNR/454

Signed-off-by: Paul Sajna <redacted>
---
 drivers/leds/leds-aw2013.c | 145 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 127 insertions(+), 18 deletions(-)
diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
index 216755d6010f..a51b72de21ac 100644
--- a/drivers/leds/leds-aw2013.c
+++ b/drivers/leds/leds-aw2013.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0+
-// Driver for Awinic AW2013 3-channel LED driver
+// Driver for Awinic AW2013/AW2027 3-channel LED drivers
 
 #include <linux/i2c.h>
 #include <linux/leds.h>
@@ -20,13 +20,15 @@
 #define AW2013_GCR 0x01
 #define AW2013_GCR_ENABLE BIT(0)
 
+#define AW20XX_GCR2 0x04
+
 /* LED channel enable register */
 #define AW2013_LCTR 0x30
 #define AW2013_LCTR_LE(x) BIT((x))
 
 /* LED channel control registers */
 #define AW2013_LCFG(x) (0x31 + (x))
-#define AW2013_LCFG_IMAX_MASK (BIT(0) | BIT(1)) // Should be 0-3
+#define AW2013_IMAX_MASK (BIT(0) | BIT(1)) // Should be 0-3
 #define AW2013_LCFG_MD BIT(4)
 #define AW2013_LCFG_FI BIT(5)
 #define AW2013_LCFG_FO BIT(6)
@@ -51,6 +53,16 @@
 
 #define AW2013_TIME_STEP 130 /* ms */
 
+/* AW2027-specific defines */
+#define AW2016_23_27_33_CHIPID 0x09 /* vendor shared same chipid between several chips */
+#define AW2027_REG_MAX 0x7F /* copied from vendor driver, but only up to 0x3F is documented */
+#define AW2027_LOCAL_IMAX_MASK GENMASK(3, 0)
+
+#define AW2027_GLOBAL_IMAX_15MA 0
+#define AW2027_GLOBAL_IMAX_30MA 1
+#define AW2027_GLOBAL_IMAX_5MA  2
+#define AW2027_GLOBAL_IMAX_10MA 3
+
 struct aw2013;
 
 struct aw2013_led {
@@ -60,23 +72,87 @@ struct aw2013_led {
 	unsigned int imax;
 };
 
+struct aw2013_chipdef {
+	u8 chip_id;
+	const struct regmap_config *regmap_cfg;
+	u32 default_imax;
+	bool gcr2_imax;
+	unsigned int current_levels;
+	unsigned int current_max;
+};
+
 struct aw2013 {
 	struct mutex mutex; /* held when writing to registers */
 	struct regulator_bulk_data regulators[2];
 	struct i2c_client *client;
 	struct aw2013_led leds[AW2013_MAX_LEDS];
 	struct regmap *regmap;
+	const struct aw2013_chipdef *cdef;
 	int num_leds;
 	bool enabled;
 };
 
-static int aw2013_chip_init(struct aw2013 *chip)
+static const struct regmap_config aw2013_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = AW2013_REG_MAX,
+};
+
+static const struct regmap_config aw2027_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = AW2027_REG_MAX,
+};
+
+static const struct aw2013_chipdef aw2013_chipdef = {
+	.chip_id = AW2013_RSTR_CHIP_ID,
+	.regmap_cfg = &aw2013_regmap_config,
+	.default_imax = 1, // 5mA
+	.gcr2_imax = false,
+	.current_levels = 4,
+	.current_max = 15000,
+};
+
+static const struct aw2013_chipdef aw2027_chipdef = {
+	.chip_id = AW2016_23_27_33_CHIPID,
+	.regmap_cfg = &aw2027_regmap_config,
+	.default_imax = 3, // 6mA
+	.gcr2_imax = true,
+	.current_levels = 16,
+	.current_max = 30000,
+};
+
+/**
+ * aw2013_microamp_to_imax() - Finds the closest current step to the given microamp
+ * @microamp: Desired current in microamps
+ * @cdef: Chip definition
+ *
+ * Return: The closest current-step to the given microamp
+ */
+static inline u32 aw2013_microamp_to_imax(u32 microamp, const struct aw2013_chipdef *cdef)
+{
+	return min_t(u32, (microamp * (cdef->current_levels - 1))
+			  / cdef->current_max,
+		     cdef->current_levels - 1);
+}
+
+static int aw2027_set_imax(struct aw2013 *chip)
 {
 	int i, ret;
 
-	ret = regmap_write(chip->regmap, AW2013_GCR, AW2013_GCR_ENABLE);
+	/*
+	 * AW2027 supports 4-step global imax, and also a 16-step control for limiting
+	 * individual current per LED. This doesn't map to the single value the
+	 * devicetree provides super well, so set global imax to maximum, and
+	 * local imax to whatever gets us closest to the value in the devicetree.
+	 */
+	ret = regmap_update_bits(chip->regmap,
+				 AW20XX_GCR2,
+				 AW2013_IMAX_MASK,
+				 AW2027_GLOBAL_IMAX_30MA);
 	if (ret) {
-		dev_err(&chip->client->dev, "Failed to enable the chip: %d\n",
+		dev_err(&chip->client->dev,
+			"Failed to set maximum global current: %d\n",
 			ret);
 		return ret;
 	}
@@ -84,7 +160,7 @@ static int aw2013_chip_init(struct aw2013 *chip)
 	for (i = 0; i < chip->num_leds; i++) {
 		ret = regmap_update_bits(chip->regmap,
 					 AW2013_LCFG(chip->leds[i].num),
-					 AW2013_LCFG_IMAX_MASK,
+					 AW2027_LOCAL_IMAX_MASK,
 					 chip->leds[i].imax);
 		if (ret) {
 			dev_err(&chip->client->dev,
@@ -97,6 +173,38 @@ static int aw2013_chip_init(struct aw2013 *chip)
 	return ret;
 }
 
+static int aw2013_chip_init(struct aw2013 *chip)
+{
+	int i, ret;
+
+	ret = regmap_write(chip->regmap, AW2013_GCR, AW2013_GCR_ENABLE);
+	if (ret) {
+		dev_err(&chip->client->dev, "Failed to enable the chip: %d\n",
+			ret);
+		return ret;
+	}
+
+	if (chip->cdef->gcr2_imax) {
+		ret = aw2027_set_imax(chip);
+	} else {
+		/* AW2013 only supports 4-step individual current per LED */
+		for (i = 0; i < chip->num_leds; i++) {
+			ret = regmap_update_bits(chip->regmap,
+						 AW2013_LCFG(chip->leds[i].num),
+						 AW2013_IMAX_MASK,
+						 chip->leds[i].imax);
+			if (ret) {
+				dev_err(&chip->client->dev,
+					"Failed to set maximum current for led %d: %d\n",
+					chip->leds[i].num, ret);
+				return ret;
+			}
+		}
+	}
+
+	return ret;
+}
+
 static void aw2013_chip_disable(struct aw2013 *chip)
 {
 	int ret;
@@ -292,9 +400,9 @@ static int aw2013_probe_dt(struct aw2013 *chip)
 		init_data.fwnode = of_fwnode_handle(child);
 
 		if (!of_property_read_u32(child, "led-max-microamp", &imax)) {
-			led->imax = min_t(u32, imax / 5000, 3);
+			led->imax = aw2013_microamp_to_imax(imax, chip->cdef);
 		} else {
-			led->imax = 1; // 5mA
+			led->imax = chip->cdef->default_imax;
 			dev_info(&chip->client->dev,
 				 "DT property led-max-microamp is missing\n");
 		}
@@ -323,18 +431,18 @@ static void aw2013_chip_disable_action(void *data)
 	aw2013_chip_disable(data);
 }
 
-static const struct regmap_config aw2013_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 8,
-	.max_register = AW2013_REG_MAX,
-};
 
 static int aw2013_probe(struct i2c_client *client)
 {
 	struct aw2013 *chip;
+	const struct aw2013_chipdef *cdef;
 	int ret;
 	unsigned int chipid;
 
+	cdef = device_get_match_data(&client->dev);
+	if (!cdef)
+		return -ENODEV;
+
 	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
@@ -346,9 +454,10 @@ static int aw2013_probe(struct i2c_client *client)
 	mutex_lock(&chip->mutex);
 
 	chip->client = client;
+	chip->cdef = cdef;
 	i2c_set_clientdata(client, chip);
 
-	chip->regmap = devm_regmap_init_i2c(client, &aw2013_regmap_config);
+	chip->regmap = devm_regmap_init_i2c(client, chip->cdef->regmap_cfg);
 	if (IS_ERR(chip->regmap)) {
 		ret = PTR_ERR(chip->regmap);
 		dev_err(&client->dev, "Failed to allocate register map: %d\n",
@@ -382,8 +491,7 @@ static int aw2013_probe(struct i2c_client *client)
 			ret);
 		goto error_reg;
 	}
-
-	if (chipid != AW2013_RSTR_CHIP_ID) {
+	if (chipid != chip->cdef->chip_id) {
 		dev_err(&client->dev, "Chip reported wrong ID: %x\n",
 			chipid);
 		ret = -ENODEV;
@@ -420,7 +528,8 @@ static int aw2013_probe(struct i2c_client *client)
 }
 
 static const struct of_device_id aw2013_match_table[] = {
-	{ .compatible = "awinic,aw2013", },
+	{ .compatible = "awinic,aw2013", .data = &aw2013_chipdef },
+	{ .compatible = "awinic,aw2027", .data = &aw2027_chipdef },
 	{ /* sentinel */ },
 };
 
@@ -437,5 +546,5 @@ static struct i2c_driver aw2013_driver = {
 module_i2c_driver(aw2013_driver);
 
 MODULE_AUTHOR("Nikita Travkin <nikitos.tr@gmail.com>");
-MODULE_DESCRIPTION("AW2013 LED driver");
+MODULE_DESCRIPTION("AW2013/AW2027 LED driver");
 MODULE_LICENSE("GPL v2");
-- 
2.56.0

[PATCH v4 3/4] leds: aw2013: Use dev_err_probe

Message details and related content
Failure to use dev_err_probe() for regulator requests violates
LED subsystem guidelines. (At least according to sashiko [1])

The original driver didn't use it, but it's worth cleaning up
while I'm here.

[1]: https://github.com/sashiko-dev/sashiko/blob/main/third_party/prompts/kernel/subsystem/leds.md?plain=1#L71

Reviewed-by: Griffin Kroah-Hartman <griffin@kroah.com>
Reviewed-by: David Heidelberg <david@ixit.cz>

Signed-off-by: Paul Sajna <redacted>
---
 drivers/leds/leds-aw2013.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
index a51b72de21ac..cf8e4b0a4572 100644
--- a/drivers/leds/leds-aw2013.c
+++ b/drivers/leds/leds-aw2013.c
@@ -459,9 +459,8 @@ static int aw2013_probe(struct i2c_client *client)
 
 	chip->regmap = devm_regmap_init_i2c(client, chip->cdef->regmap_cfg);
 	if (IS_ERR(chip->regmap)) {
-		ret = PTR_ERR(chip->regmap);
-		dev_err(&client->dev, "Failed to allocate register map: %d\n",
-			ret);
+		ret = dev_err_probe(&client->dev, PTR_ERR(chip->regmap),
+				    "Failed to allocate register map\n");
 		goto error;
 	}
 
@@ -471,30 +470,29 @@ static int aw2013_probe(struct i2c_client *client)
 				      ARRAY_SIZE(chip->regulators),
 				      chip->regulators);
 	if (ret < 0) {
-		if (ret != -EPROBE_DEFER)
-			dev_err(&client->dev,
-				"Failed to request regulators: %d\n", ret);
+		ret = dev_err_probe(&client->dev, ret,
+				    "Failed to request regulators\n");
 		goto error;
 	}
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(chip->regulators),
 				    chip->regulators);
 	if (ret) {
-		dev_err(&client->dev,
-			"Failed to enable regulators: %d\n", ret);
+		ret = dev_err_probe(&client->dev, ret,
+				    "Failed to enable regulators\n");
 		goto error;
 	}
 
 	ret = regmap_read(chip->regmap, AW2013_RSTR, &chipid);
 	if (ret) {
-		dev_err(&client->dev, "Failed to read chip ID: %d\n",
-			ret);
+		ret = dev_err_probe(&client->dev, ret,
+				    "Failed to read chip ID\n");
 		goto error_reg;
 	}
 	if (chipid != chip->cdef->chip_id) {
-		dev_err(&client->dev, "Chip reported wrong ID: %x\n",
-			chipid);
-		ret = -ENODEV;
+		ret = dev_err_probe(&client->dev, -ENODEV,
+				    "Chip reported wrong ID: %x\n",
+				     chipid);
 		goto error_reg;
 	}
 
@@ -509,8 +507,8 @@ static int aw2013_probe(struct i2c_client *client)
 	ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
 				     chip->regulators);
 	if (ret) {
-		dev_err(&client->dev,
-			"Failed to disable regulators: %d\n", ret);
+		ret = dev_err_probe(&client->dev, ret,
+				    "Failed to disable regulators\n");
 		goto error;
 	}
 
-- 
2.56.0

[PATCH v4 4/4] leds: aw2013: Move assignment of chip->num_leds

Message details and related content
devm_led_classdev_register_ext() executes default triggers
synchronously, which call aw20xx_brightness_set().
aw20xx_chip_in_use() silently returns false, skipping chip enablement and
causing unbalanced regulator disables while probe is still running.

Increment chip->num_leds inside the loop, so that each call gets the
correct value for it's default trigger.

Signed-off-by: Paul Sajna <redacted>
---
 drivers/leds/leds-aw2013.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
index cf8e4b0a4572..c5df652ad8c4 100644
--- a/drivers/leds/leds-aw2013.c
+++ b/drivers/leds/leds-aw2013.c
@@ -410,6 +410,8 @@ static int aw2013_probe_dt(struct aw2013 *chip)
 		led->cdev.brightness_set_blocking = aw2013_brightness_set;
 		led->cdev.blink_set = aw2013_blink_set;
 
+		chip->num_leds = i + 1;
+
 		ret = devm_led_classdev_register_ext(&chip->client->dev,
 						     &led->cdev, &init_data);
 		if (ret < 0)
@@ -421,8 +423,6 @@ static int aw2013_probe_dt(struct aw2013 *chip)
 	if (!count)
 		return -EINVAL;
 
-	chip->num_leds = i;
-
 	return 0;
 }
 
-- 
2.56.0

Re: [PATCH v4 2/4] leds: aw2013: Add AW2027 support

Message details and related content
On 2026-11-08 2:49 p.m., Paul Sajna wrote:
+#define AW2016_23_27_33_CHIPID 0x09 /* vendor shared same chipid between several chips */
I'm not sure if I should add compatibles for all of these variants.
I would only be able to test 2027, but they should in theory all be software compatible if they have the same chipid.

-- 
Hal: They're going for the kernel!
Margo: Colonel Who?

Re: [PATCH v4 4/4] leds: aw2013: Move assignment of chip->num_leds

Message details and related content
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] leds: aw2013: commit message rationale does not match driver
  locking or code
--

commit 1ce6b90ecf6b0a7b5d8097b414e8e137a3fe0609
Author: Paul Sajna [off-list ref]
leds: aw2013: Move assignment of chip->num_leds

devm_led_classdev_register_ext() executes default triggers
synchronously, which call aw20xx_brightness_set().
aw20xx_chip_in_use() silently returns false, skipping chip enablement and
causing unbalanced regulator disables while probe is still running.

Increment chip->num_leds inside the loop, so that each call gets the
correct value for it's default trigger.

Signed-off-by: Paul Sajna <redacted>
[Severity: Medium]
Does this commit message match the locking and execution flow in
leds-aw2013?

aw2013_probe_dt() sets brightness_set_blocking to aw2013_brightness_set()
and leaves brightness_set NULL, so default triggers activated during
devm_led_classdev_register_ext() queue set_brightness_work instead of
calling aw2013_brightness_set() synchronously.

Furthermore, aw2013_probe() holds chip->mutex across aw2013_probe_dt(), and
aw2013_brightness_set() locks chip->mutex before calling
aw2013_chip_in_use().  Any queued work therefore waits until
aw2013_probe_dt() has finished and chip->num_leds is already set.

Also, aw2013_chip_disable() returns early when !chip->enabled, preventing
an unbalanced regulator_bulk_disable() if aw2013_chip_enable() was skipped,
and the driver functions are named aw2013_brightness_set() and
aw2013_chip_in_use() rather than aw20xx_*.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108-aw2013-aw20xx-rename-v4-0-826529beeb52@postmarketos.org?part=4

Re: [PATCH v4 4/4] leds: aw2013: Move assignment of chip->num_leds

Message details and related content
On 2026-11-08 3:00 p.m., sashiko-bot@kernel.org wrote:
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] leds: aw2013: commit message rationale does not match driver
   locking or code
--

commit 1ce6b90ecf6b0a7b5d8097b414e8e137a3fe0609
Author: Paul Sajna [off-list ref]
quoted
leds: aw2013: Move assignment of chip->num_leds

devm_led_classdev_register_ext() executes default triggers
synchronously, which call aw20xx_brightness_set().
aw20xx_chip_in_use() silently returns false, skipping chip enablement and
causing unbalanced regulator disables while probe is still running.

Increment chip->num_leds inside the loop, so that each call gets the
correct value for it's default trigger.

Signed-off-by: Paul Sajna <redacted>
[Severity: Medium]
Does this commit message match the locking and execution flow in
leds-aw2013?

aw2013_probe_dt() sets brightness_set_blocking to aw2013_brightness_set()
and leaves brightness_set NULL, so default triggers activated during
devm_led_classdev_register_ext() queue set_brightness_work instead of
calling aw2013_brightness_set() synchronously.

Furthermore, aw2013_probe() holds chip->mutex across aw2013_probe_dt(), and
aw2013_brightness_set() locks chip->mutex before calling
aw2013_chip_in_use().  Any queued work therefore waits until
aw2013_probe_dt() has finished and chip->num_leds is already set.

Also, aw2013_chip_disable() returns early when !chip->enabled, preventing
an unbalanced regulator_bulk_disable() if aw2013_chip_enable() was skipped,
and the driver functions are named aw2013_brightness_set() and
aw2013_chip_in_use() rather than aw20xx_*.
The robot is internally inconsistent and incoherent.
Feel free to drop this patch if you don't find it necessary.
See the robot's prior review here:
https://lore.kernel.org/linux-leds/20260908000128.B92201F00A3A@smtp.kernel.org/
[ ... ]
quoted hunk
-		led->cdev.brightness_set_blocking = aw2013_brightness_set;
-		led->cdev.blink_set = aw2013_blink_set;
+		led->cdev.brightness_set_blocking = aw20xx_brightness_set;
+		led->cdev.blink_set = aw20xx_blink_set;

 		ret = devm_led_classdev_register_ext(&chip->client->dev,
 						     &led->cdev, &init_data);
[Severity: High]
This is a pre-existing issue, but does assigning chip->num_leds after
registering the LED class devices cause initialization problems?
Looking at aw20xx_probe_dt(), chip->num_leds is set to i after the loop
completes. However, devm_led_classdev_register_ext() executes default triggers
synchronously, which call aw20xx_brightness_set(). If num_leds is still 0,
aw20xx_chip_in_use() silently returns false, skipping chip enablement and causing unbalanced regulator disables while probe is still
running.

-- 
Hal: They're going for the kernel!
Margo: Colonel Who?
Message details and related content
On 2026-11-08 2:49 p.m., Paul Sajna wrote:
I'll do my best to address all of Sashiko's comments, but it might take
me some time to circle back, don't bother reviewing until I've done
this.
Hello Lee, I believe I have addressed all the robot's comments, and this is now ready for a proper review.

-- 
Hal: They're going for the kernel!
Margo: Colonel Who?
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help