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

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 configu=
ring 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 ne=
ver applied to the hardware.
--

--- Patch [1]: [PATCH v3 1/3] backlight: aw99706: Fix DT property names to =
match binding ---
commit 0780fe78c9307963271e82c84a46a86db0bcde59
Author: Junjie Cao <[email protected]>

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_prop=
s[] =3D {
>  		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 =3D 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 =3D aw99706_chip_id_read(aw);
	if (ret !=3D AW99706_ID)
		return dev_err_probe(dev, -ENODEV,
				     "Unknown chip id 0x%02x\n", ret);

	aw99706_dt_parse(aw, &props);

	aw->hwen_gpio =3D 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 =3D 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 =3D 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 res=
et
state, the display remains off upon boot until userspace manually sets the
brightness.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804030255.1934=
[email protected]?part=3D1