Re: [PATCH v3 3/3] backlight: aw99706: Honor the core blank state in update_status()
[email protected] Tue, 04 Aug 2026 03:15:33 +0000
| Newsgroups | org.freedesktop.lists.dri-devel,org.kernel.vger.linux-leds |
|---|---|
| Message-ID | <[email protected]> |
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 appli= ed to the hardware during probe. -- --- Patch [3]: [PATCH v3 3/3] backlight: aw99706: Honor the core blank stat= e in update_status() --- commit 4fd7fd023a2e220d60fcbcb9c519d9b49b430467 Author: Junjie Cao <[email protected]> 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 =3D bl_get_data(bl); > =20 > - return aw99706_update_brightness(aw, bl->props.brightness); > + return aw99706_update_brightness(aw, backlight_get_brightness(bl)); > } > =20 > static const struct backlight_ops aw99706_bl_ops =3D { [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 =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); ret =3D 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 f= rom device tree is never applied to the hardware during probe. In 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; 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, a= nd the backlight core does not automatically sync the initial software state upon registration. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804030255.1934= [email protected]?part=3D3