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

[PATCH v3 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 validation paths accept values the binding forbids
(patch 2), and update_status() ignores the blank state, leaving bl_power
inoperative (patch 3).

v3:
- patch 2: reject 0 for awinic,sw-freq-hz (reserved-slot marker
  collided with it); add Fixes tag
- patch 3: add stable tag
v2: only patch 1/3 reached the lists (outbound mail failure):
    https://lore.kernel.org/all/20260803141310.1379194-2-junjie.cao@linux.dev/
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 | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

-- 
2.43.0

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

[Junjie Cao]

From: Junjie Cao <junjie.cao@linux.dev>

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 v3 2/3] backlight: aw99706: Validate all DT property values consistently

[Junjie Cao]

From: Junjie Cao <junjie.cao@linux.dev>

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.

The switching frequency table has a second hole: reserved slots use 0
as their marker, so "awinic,sw-freq-hz = <0>" matches slot 0 and
programs a reserved encoding. Make the reserved marker U32_MAX and
skip such slots during lookup.

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

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

diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
index e130f164303a..6ec49b6cb14c 100644
--- a/drivers/video/backlight/aw99706.c
+++ b/drivers/video/backlight/aw99706.c
@@ -60,7 +60,7 @@
 #define AW99706_MTPLDOSEL_REG			0x1E
 #define AW99706_MTPRUN_REG			0x1F
 
-#define RESV	0
+#define RESV	U32_MAX
 
 /* Boost switching frequency table, in Hz */
 static const u32 aw99706_sw_freq_tbl[] = {
@@ -94,17 +94,19 @@ 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;
 	}
 
 	for (i = 0; i < prop->tbl_size; i++)
-		if (prop->lookup_tbl[i] == dt_val)
+		if (prop->lookup_tbl[i] != RESV && prop->lookup_tbl[i] == dt_val)
 			break;
 
 	*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 v3 3/3] backlight: aw99706: Honor the core blank state in update_status()

[Junjie Cao]

From: Junjie Cao <junjie.cao@linux.dev>

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")
Cc: stable@vger.kernel.org
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 6ec49b6cb14c..c4a66b59ecf1 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 v3 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] I2C chip ID read is attempted before the hardware enable GPIO is asserted.
- [High] Initial backlight brightness state is not synchronized to the hardware during probe.
--

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

backlight: aw99706: Validate all DT property values consistently
    
The lookup helpers for dim-mode and ramp-ctl take a shortcut when
lookup_tbl is NULL, accepting any u32 value without range-checking.
This commit adds a field-width check for the NULL-table case so that
values exceeding the register field maximum are rejected. It also updates
the switching frequency table reserved marker to U32_MAX and changes
error returns to -EINVAL.

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

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but does aw99706_probe() attempt an
I2C read before the hardware enable GPIO is asserted?

In aw99706_probe(), the driver calls aw99706_chip_id_read() before fetching
hwen_gpio and calling aw99706_hw_init() to assert the enable pin:

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);
    ...
}

Could this cause the probe to fail with an I2C NACK or timeout if the
bootloader hasn't already left the GPIO asserted?


[Severity: High]
This is a pre-existing issue, but is the initial backlight brightness state
synchronized to the hardware during probe?

aw99706_probe() parses the default brightness and registers the backlight
device, but returns without flushing the initial state:

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 backlight_update_status() or aw99706_update_brightness() is never called
here, does this mean the parsed brightness and the backlight enable bit aren't
written to the hardware, potentially leaving a black screen during boot?

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] I2C read is performed before the hardware is powered up or enabled.
- [High] The initial backlight state parsed from device tree is never applied to the hardware during probe.
--

--- Patch [3]: [PATCH v3 3/3] backlight: aw99706: Honor the core blank state in update_status() ---
commit 4fd7fd023a2e220d60fcbcb9c519d9b49b430467
Author: Junjie Cao <junjie.cao@linux.dev>

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.

> diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
> index 6ec49b6cb14c6..c4a66b59ecf13 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 = {

[Severity: High]
This is a pre-existing issue, but I noticed an I2C read is performed before
the hardware is powered up or enabled.

In 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);

	ret = aw99706_hw_init(aw);

Will the driver probe fail with -ENODEV here if the hardware is disabled by
default, since the hardware enable GPIO (aw->hwen_gpio) is not requested and
asserted until later via devm_gpiod_get() and aw99706_hw_init()?


[Severity: High]
This is also a pre-existing issue, but the initial backlight state parsed from
device tree is never applied to the hardware during probe.

In 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;

Does the backlight remain off during the entire boot process until userspace
writes to the sysfs brightness attribute?

The probe function completes without calling backlight_update_status() to
synchronize the parsed device tree brightness properties to the hardware, and
the backlight core does not automatically sync the initial software state
upon registration.

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver attempts to read the I2C CHIPID register before configuring and asserting the hardware enable GPIO.
- [High] The `update_status` callback directly uses `bl->props.brightness` and fails to account for backlight blanking or suspend power states.
- [High] The initial `default-brightness` parsed from the device tree is never applied to the hardware.
--

--- Patch [1]: [PATCH v3 1/3] backlight: aw99706: Fix DT property names to match binding ---
commit 0780fe78c9307963271e82c84a46a86db0bcde59
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. Fix 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,

[ ... ]

[Severity: High]
This is a pre-existing issue, but does aw99706_bl_update_status() handle
backlight blanking or suspend power states?

drivers/video/backlight/aw99706.c:aw99706_bl_update_status() {
	struct aw99706_device *aw = bl_get_data(bl);

	return aw99706_update_brightness(aw, bl->props.brightness);
}

By directly accessing bl->props.brightness instead of using
backlight_get_brightness(bl), the driver ignores BL_CORE_FBBLANK and
BL_CORE_SUSPENDED flags, keeping the screen illuminated when it should be
blanked.

[Severity: High]
This is a pre-existing issue, but can the device probe successfully if the
I2C read occurs before the hardware enable GPIO is asserted?

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 the bootloader does not leave the enable GPIO asserted, the device will be
powered off or in reset, rendering I2C communication impossible.

[Severity: High]
This is a pre-existing issue, but is the initial default-brightness applied
to the hardware?

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;
}

The probe routine successfully parses default-brightness and registers the
backlight device, but doesn't call backlight_update_status(bl_dev) to sync
the parsed initial brightness to the hardware registers.

Without an explicit initial brightness update to overwrite the hardware reset
state, the display remains off upon boot until userspace manually sets the
brightness.

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