[PATCH v2 0/3] backlight: aw99706: DT parsing and blank state fixes

[PATCH v2 0/3] backlight: aw99706: DT parsing and blank state fixes

[Junjie Cao]

The driver reads its tuning properties under names the binding does not
define (patch 1), the NULL-table lookup path accepts any value without
range checking (patch 2), and update_status() ignores the blank state,
leaving bl_power inoperative (patch 3).

v2:
- add patch 3
- add stable tag to patch 1
v1: https://lore.kernel.org/all/20260701133918.33487-1-junjie.cao@linux.dev/

Junjie Cao (3):
  backlight: aw99706: Fix DT property names to match binding
  backlight: aw99706: Validate all DT property values consistently
  backlight: aw99706: Honor the core blank state in update_status()

 drivers/video/backlight/aw99706.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] backlight: aw99706: Fix DT property names to match binding

[Junjie Cao]

The driver reads four tuning properties without the unit suffixes that
the binding mandates: "awinic,sw-freq" instead of "awinic,sw-freq-hz",
"awinic,sw-ilmt" instead of "awinic,sw-ilmt-microamp", "awinic,iled-max"
instead of "awinic,iled-max-microamp", and "awinic,uvlo-thres" instead
of "awinic,uvlo-thres-microvolt".

As a result, device_property_read_u32() never finds these properties in
a binding-conformant device tree and silently falls back to the compiled-in
defaults for switching frequency, switching current limit, max LED current,
and UVLO threshold.

Fix by aligning the property name strings in aw99706_dt_props[] with the
binding. No value/range changes are needed since both sides already use
the same units and enumerations.

Fixes: 147b38a5ad06 ("backlight: aw99706: Add support for Awinic AW99706 backlight")
Cc: stable@vger.kernel.org
Signed-off-by: Junjie Cao <junjie.cao@linux.dev>
---
 drivers/video/backlight/aw99706.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
index 18299faf06ad..e130f164303a 100644
--- a/drivers/video/backlight/aw99706.c
+++ b/drivers/video/backlight/aw99706.c
@@ -130,23 +130,23 @@ static const struct aw99706_dt_prop aw99706_dt_props[] = {
 		AW99706_CFG0_REG, AW99706_DIM_MODE_MASK, 1,
 	},
 	{
-		"awinic,sw-freq", aw99706_dt_property_lookup,
+		"awinic,sw-freq-hz", aw99706_dt_property_lookup,
 		aw99706_sw_freq_tbl, ARRAY_SIZE(aw99706_sw_freq_tbl),
 		AW99706_CFG1_REG, AW99706_SW_FREQ_MASK, 750000,
 	},
 	{
-		"awinic,sw-ilmt", aw99706_dt_property_lookup,
+		"awinic,sw-ilmt-microamp", aw99706_dt_property_lookup,
 		aw99706_sw_ilmt_tbl, ARRAY_SIZE(aw99706_sw_ilmt_tbl),
 		AW99706_CFG1_REG, AW99706_SW_ILMT_MASK, 3000000,
 	},
 	{
-		"awinic,iled-max", aw99706_dt_property_iled_max_convert,
+		"awinic,iled-max-microamp", aw99706_dt_property_iled_max_convert,
 		NULL, 0,
 		AW99706_CFG2_REG, AW99706_ILED_MAX_MASK, 20000,
 
 	},
 	{
-		"awinic,uvlo-thres", aw99706_dt_property_lookup,
+		"awinic,uvlo-thres-microvolt", aw99706_dt_property_lookup,
 		aw99706_ulvo_thres_tbl, ARRAY_SIZE(aw99706_ulvo_thres_tbl),
 		AW99706_CFG2_REG, AW99706_UVLOSEL_MASK, 2200000,
 	},
-- 
2.43.0

[PATCH v2 2/3] backlight: aw99706: Validate all DT property values consistently

[Junjie Cao]

The lookup helpers for dim-mode and ramp-ctl take a shortcut when
lookup_tbl is NULL: they accept any u32 value without range-checking
and return success unconditionally. Out-of-range values get silently
truncated by regmap_update_bits instead of triggering the dev_warn +
default-fallback path that the other properties use.

Add a field-width check for the NULL-table case so that values
exceeding the register field maximum are rejected the same way a
table-lookup miss is.

While here, also switch the error returns to -EINVAL for consistency.

Signed-off-by: Junjie Cao <junjie.cao@linux.dev>
---
 drivers/video/backlight/aw99706.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
index e130f164303a..dfb4b38b5b4c 100644
--- a/drivers/video/backlight/aw99706.c
+++ b/drivers/video/backlight/aw99706.c
@@ -94,6 +94,8 @@ static int aw99706_dt_property_lookup(const struct aw99706_dt_prop *prop,
 	int i;
 
 	if (!prop->lookup_tbl) {
+		if (dt_val > (prop->mask >> __ffs(prop->mask)))
+			return -EINVAL;
 		*val = dt_val;
 		return 0;
 	}
@@ -104,7 +106,7 @@ static int aw99706_dt_property_lookup(const struct aw99706_dt_prop *prop,
 
 	*val = i;
 
-	return i == prop->tbl_size ? -1 : 0;
+	return i == prop->tbl_size ? -EINVAL : 0;
 }
 
 #define MIN_ILED_MAX	5000
@@ -116,11 +118,14 @@ aw99706_dt_property_iled_max_convert(const struct aw99706_dt_prop *prop,
 				     u32 dt_val, u8 *val)
 {
 	if (dt_val > MAX_ILED_MAX || dt_val < MIN_ILED_MAX)
-		return -1;
+		return -EINVAL;
+
+	if ((dt_val - MIN_ILED_MAX) % STEP_ILED_MAX)
+		return -EINVAL;
 
 	*val = (dt_val - MIN_ILED_MAX) / STEP_ILED_MAX;
 
-	return (dt_val - MIN_ILED_MAX) % STEP_ILED_MAX;
+	return 0;
 }
 
 static const struct aw99706_dt_prop aw99706_dt_props[] = {
-- 
2.43.0

[PATCH v2 3/3] backlight: aw99706: Honor the core blank state in update_status()

[Junjie Cao]

update_status() passes props.brightness straight to the hardware and
ignores the power/blank state tracked by the core. Writing 4 to the
bl_power sysfs attribute or blanking the framebuffer therefore leaves
the backlight lit.

Use backlight_get_brightness(), which returns 0 while the device is
blanked.

Fixes: 147b38a5ad06 ("backlight: aw99706: Add support for Awinic AW99706 backlight")
Signed-off-by: Junjie Cao <junjie.cao@linux.dev>
---
 drivers/video/backlight/aw99706.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
index dfb4b38b5b4c..d035ff5af52c 100644
--- a/drivers/video/backlight/aw99706.c
+++ b/drivers/video/backlight/aw99706.c
@@ -340,7 +340,7 @@ static int aw99706_bl_update_status(struct backlight_device *bl)
 {
 	struct aw99706_device *aw = bl_get_data(bl);
 
-	return aw99706_update_brightness(aw, bl->props.brightness);
+	return aw99706_update_brightness(aw, backlight_get_brightness(bl));
 }
 
 static const struct backlight_ops aw99706_bl_ops = {
-- 
2.43.0

Re: [PATCH v2 1/3] backlight: aw99706: Fix DT property names to match binding

[Junjie Cao]

The rest of this series was lost to a mail delivery failure; please
ignore this one and see v3 instead. Sorry for the noise.

Re: [PATCH v2 3/3] backlight: aw99706: Honor the core blank state in update_status()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] I2C chip ID read is performed before the hardware enable GPIO is requested and asserted.
- [High] Hardware is manually disabled in `remove()` while the backlight device remains registered via `devm_`, leading to a use-after-disable race.
- [Medium] Missing delay after asserting the enable GPIO before issuing I2C commands.
- [Medium] Incomplete suspend/resume logic leaves hardware enable GPIO asserted.
--

--- Patch [3]: [PATCH v2 3/3] backlight: aw99706: Honor the core blank state in update_status() ---
commit 0ee82864b515a444eccb8c16ea1fd1e52d9895f0
Author: Junjie Cao <junjie.cao@linux.dev>
Date:   Mon Aug 3 14:13:10 2026 +0000

    backlight: aw99706: Honor the core blank state in update_status()
    
    update_status() passes props.brightness straight to the hardware and
    ignores the power/blank state tracked by the core. Writing 4 to the
    bl_power sysfs attribute or blanking the framebuffer therefore leaves
    the backlight lit.
    
    Use backlight_get_brightness(), which returns 0 while the device is
    blanked.
    
    Fixes: 147b38a5ad06 ("backlight: aw99706: Add support for Awinic AW99706 backlight")
    Signed-off-by: Junjie Cao <junjie.cao@linux.dev>

> diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
> index dfb4b38b5b4c7..d035ff5af52c6 100644
> --- a/drivers/video/backlight/aw99706.c
> +++ b/drivers/video/backlight/aw99706.c
> @@ -340,7 +340,7 @@ static int aw99706_bl_update_status(struct backlight_device *bl)
>  {
>  	struct aw99706_device *aw = bl_get_data(bl);
>  
> -	return aw99706_update_brightness(aw, bl->props.brightness);
> +	return aw99706_update_brightness(aw, backlight_get_brightness(bl));
>  }

While reviewing this patch, I noticed a few pre-existing issues in the driver:

drivers/video/backlight/aw99706.c:aw99706_probe() {
    ...
	ret = aw99706_chip_id_read(aw);
	if (ret != AW99706_ID)
		return dev_err_probe(dev, -ENODEV,
				     "Unknown chip id 0x%02x\n", ret);

	aw99706_dt_parse(aw, &props);

	aw->hwen_gpio = devm_gpiod_get(aw->dev, "enable", GPIOD_OUT_LOW);
	if (IS_ERR(aw->hwen_gpio))
		return dev_err_probe(dev, PTR_ERR(aw->hwen_gpio),
				     "Failed to get enable gpio\n");
    ...
}

[Severity: High]
This is a pre-existing issue, but doesn't aw99706_chip_id_read() perform an
I2C read before the hardware enable GPIO is requested and asserted?

If the hardware is disabled by default or left in reset by the bootloader,
won't this I2C read fail and make the driver completely non-functional?


drivers/video/backlight/aw99706.c:aw99706_remove() {
	struct aw99706_device *aw = i2c_get_clientdata(client);

	aw99706_update_brightness(aw, 0);

	msleep(50);

	gpiod_set_value_cansleep(aw->hwen_gpio, 0);
}

[Severity: High]
This is a pre-existing issue, but does disabling the hardware here create a
use-after-disable race?

The backlight device is registered with devm_backlight_device_register(), so
devres will unregister it after aw99706_remove() returns.

If a userspace process accesses the sysfs backlight attributes in this window,
could aw99706_bl_update_status() issue I2C writes to a powered-down chip?


drivers/video/backlight/aw99706.c:aw99706_hw_init() {
	int ret, i;

	gpiod_set_value_cansleep(aw->hwen_gpio, 1);

	for (i = 0; i < ARRAY_SIZE(aw->init_tbl); i++) {
		ret = aw99706_i2c_update_bits(aw, aw->init_tbl[i].reg,
    ...
}

[Severity: Medium]
This is a pre-existing issue, but does this sequence need a stabilization
delay between asserting hwen_gpio and issuing I2C commands?

Could the initialization commands be dropped or NACKed if the controller
requires a few milliseconds to wake up before accepting I2C traffic?


drivers/video/backlight/aw99706.c:aw99706_suspend() {
	struct aw99706_device *aw = dev_get_drvdata(dev);

	return aw99706_update_brightness(aw, 0);
}

static int aw99706_resume(struct device *dev)
{
	struct aw99706_device *aw = dev_get_drvdata(dev);

	return aw99706_hw_init(aw);
}

[Severity: Medium]
This is a pre-existing issue, but do these suspend and resume operations
handle the hardware enable GPIO correctly?

aw99706_suspend() zeros out the brightness but leaves hwen_gpio asserted,
and aw99706_resume() blindly re-initializes all hardware registers without
the power ever being removed.

Should aw99706_suspend() pull hwen_gpio low to actually save power, or are
these operations redundant given BL_CORE_SUSPENDRESUME already handles setting
brightness to 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803141310.1379194-1-junjie.cao@linux.dev?part=3

Re: [PATCH v2 1/3] backlight: aw99706: Fix DT property names to match binding

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver attempts to read the chip ID via I2C before the hardware enable GPIO is acquired and asserted.
- [High] Negative error codes from `aw99706_chip_id_read` are masked and improperly formatted, breaking probe deferral.
--

--- Patch [1]: [PATCH v2 1/3] backlight: aw99706: Fix DT property names to match binding ---
commit 9d2f2e8b6d2445c40c8f872abafa26c81384bf67
Author: Junjie Cao <junjie.cao@linux.dev>

backlight: aw99706: Fix DT property names to match binding

The driver reads four tuning properties without the unit suffixes that the
binding mandates. This commit fixes the issue by aligning the property name
strings in aw99706_dt_props[] with the binding.

> diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
> index 18299faf06ad4..e130f164303a2 100644
> --- a/drivers/video/backlight/aw99706.c
> +++ b/drivers/video/backlight/aw99706.c
> @@ -130,23 +130,23 @@ static const struct aw99706_dt_prop aw99706_dt_props[] = {
>  		AW99706_CFG0_REG, AW99706_DIM_MODE_MASK, 1,
>  	},
>  	{
> -		"awinic,sw-freq", aw99706_dt_property_lookup,
> +		"awinic,sw-freq-hz", aw99706_dt_property_lookup,
>  		aw99706_sw_freq_tbl, ARRAY_SIZE(aw99706_sw_freq_tbl),
>  		AW99706_CFG1_REG, AW99706_SW_FREQ_MASK, 750000,
>  	},
>  	{
> -		"awinic,sw-ilmt", aw99706_dt_property_lookup,
> +		"awinic,sw-ilmt-microamp", aw99706_dt_property_lookup,
>  		aw99706_sw_ilmt_tbl, ARRAY_SIZE(aw99706_sw_ilmt_tbl),
>  		AW99706_CFG1_REG, AW99706_SW_ILMT_MASK, 3000000,
>  	},
>  	{
> -		"awinic,iled-max", aw99706_dt_property_iled_max_convert,
> +		"awinic,iled-max-microamp", aw99706_dt_property_iled_max_convert,
>  		NULL, 0,
>  		AW99706_CFG2_REG, AW99706_ILED_MAX_MASK, 20000,
>  
>  	},
>  	{
> -		"awinic,uvlo-thres", aw99706_dt_property_lookup,
> +		"awinic,uvlo-thres-microvolt", aw99706_dt_property_lookup,
>  		aw99706_ulvo_thres_tbl, ARRAY_SIZE(aw99706_ulvo_thres_tbl),
>  		AW99706_CFG2_REG, AW99706_UVLOSEL_MASK, 2200000,
>  	},

[Severity: High]
This isn't a bug introduced by this patch, but does the probe function attempt
to read the chip ID via I2C before the hardware enable GPIO is acquired and
asserted?

Looking at aw99706_probe() in drivers/video/backlight/aw99706.c:

	ret = aw99706_chip_id_read(aw);
	if (ret != AW99706_ID)
		return dev_err_probe(dev, -ENODEV,
				     "Unknown chip id 0x%02x\n", ret);

	aw99706_dt_parse(aw, &props);

	aw->hwen_gpio = devm_gpiod_get(aw->dev, "enable", GPIOD_OUT_LOW);

Since the hardware enable GPIO is only requested and asserted later in the
probe sequence, if the bootloader hasn't pre-enabled the hardware, the chip
will remain powered off or in reset during the earlier I2C read. This would
cause the probe function to fail to read the chip ID and abort.

[Severity: High]
This is a pre-existing issue, but does this sequence mask negative error codes
from aw99706_chip_id_read()?

Looking at the chip ID verification in aw99706_probe():

	ret = aw99706_chip_id_read(aw);
	if (ret != AW99706_ID)
		return dev_err_probe(dev, -ENODEV,
				     "Unknown chip id 0x%02x\n", ret);

If the I2C controller or bus is not yet ready, it could return -EPROBE_DEFER,
which is then replaced with -ENODEV. This would break the kernel's probe
deferral mechanism and cause a permanent probe failure. It also prints the
negative integer using %02x, which emits a two's complement hexadecimal value
instead of a clear error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803141310.1379194-1-junjie.cao@linux.dev?part=1

Re: [PATCH v2 2/3] backlight: aw99706: Validate all DT property values consistently

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver attempts to communicate with the hardware via I2C before obtaining and asserting the hardware enable GPIO, which will cause probe to fail if the device is not powered on by default.
- [High] The driver never applies the initial brightness state to the hardware during probe, leaving the backlight hardware off or in an undefined state until userspace explicitly updates it.
--

--- Patch [2]: [PATCH v2 2/3] backlight: aw99706: Validate all DT property values consistently ---
commit 7ade0b3839909ff086334534c2c53128a09d10da
Author: Junjie Cao <junjie.cao@linux.dev>

backlight: aw99706: Validate all DT property values consistently

The lookup helpers for dim-mode and ramp-ctl accept any u32 value without
range-checking when lookup_tbl is NULL. This adds a field-width check for
the NULL-table case to reject out-of-range values. It also switches the
error returns to -EINVAL for consistency.

> diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
> index e130f164303a2..dfb4b38b5b4c7 100644
> --- a/drivers/video/backlight/aw99706.c
> +++ b/drivers/video/backlight/aw99706.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed that probe might fail if the
hardware is not already powered on.

drivers/video/backlight/aw99706.c:aw99706_probe() {
    ...
	ret = aw99706_chip_id_read(aw);
	if (ret != AW99706_ID)
		return dev_err_probe(dev, -ENODEV,
				     "Unknown chip id 0x%02x\n", ret);

	aw99706_dt_parse(aw, &props);

	aw->hwen_gpio = devm_gpiod_get(aw->dev, "enable", GPIOD_OUT_LOW);
    ...
}

Will the chip ID read fail here if the bootloader left the hardware enable
GPIO deactivated? Since the enable GPIO isn't requested and asserted until
after the I2C read, the I2C communication could timeout or fail.

[Severity: High]
This is a pre-existing issue, but it looks like the initial brightness state
is never pushed to the hardware during probe.

drivers/video/backlight/aw99706.c:aw99706_probe() {
    ...
	bl_dev = devm_backlight_device_register(dev, "aw99706-backlight", dev,
						aw, &aw99706_bl_ops, &props);
	if (IS_ERR(bl_dev))
		return dev_err_probe(dev, PTR_ERR(bl_dev),
				     "Failed to register backlight!\n");

	aw->bl_dev = bl_dev;

	return 0;
}

Since devm_gpiod_get(..., GPIOD_OUT_LOW) actively toggles the hardware enable
pin (resetting the hardware) and backlight_update_status() is never called
after devm_backlight_device_register(), will this leave the backlight off
until userspace explicitly updates it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803141310.1379194-1-junjie.cao@linux.dev?part=2