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

[sashiko-bot@kernel.org]

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