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

[sashiko-bot@kernel.org]

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