Re: [PATCH v2 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] 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