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

[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 read the chip ID via I2C before the hardware enable GPIO is acquired and asserted.
- [High] Negative error codes from `aw99706_chip_id_read` are masked and improperly formatted, breaking probe deferral.
--

--- Patch [1]: [PATCH v2 1/3] backlight: aw99706: Fix DT property names to match binding ---
commit 9d2f2e8b6d2445c40c8f872abafa26c81384bf67
Author: Junjie Cao <junjie.cao@linux.dev>

backlight: aw99706: Fix DT property names to match binding

The driver reads four tuning properties without the unit suffixes that the
binding mandates. This commit fixes the issue 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_props[] = {
>  		AW99706_CFG0_REG, AW99706_DIM_MODE_MASK, 1,
>  	},
>  	{
> -		"awinic,sw-freq", aw99706_dt_property_lookup,
> +		"awinic,sw-freq-hz", aw99706_dt_property_lookup,
>  		aw99706_sw_freq_tbl, ARRAY_SIZE(aw99706_sw_freq_tbl),
>  		AW99706_CFG1_REG, AW99706_SW_FREQ_MASK, 750000,
>  	},
>  	{
> -		"awinic,sw-ilmt", aw99706_dt_property_lookup,
> +		"awinic,sw-ilmt-microamp", aw99706_dt_property_lookup,
>  		aw99706_sw_ilmt_tbl, ARRAY_SIZE(aw99706_sw_ilmt_tbl),
>  		AW99706_CFG1_REG, AW99706_SW_ILMT_MASK, 3000000,
>  	},
>  	{
> -		"awinic,iled-max", aw99706_dt_property_iled_max_convert,
> +		"awinic,iled-max-microamp", aw99706_dt_property_iled_max_convert,
>  		NULL, 0,
>  		AW99706_CFG2_REG, AW99706_ILED_MAX_MASK, 20000,
>  
>  	},
>  	{
> -		"awinic,uvlo-thres", aw99706_dt_property_lookup,
> +		"awinic,uvlo-thres-microvolt", aw99706_dt_property_lookup,
>  		aw99706_ulvo_thres_tbl, ARRAY_SIZE(aw99706_ulvo_thres_tbl),
>  		AW99706_CFG2_REG, AW99706_UVLOSEL_MASK, 2200000,
>  	},

[Severity: High]
This isn't a bug introduced by this patch, but does the probe function attempt
to read the chip ID via I2C before the hardware enable GPIO is acquired and
asserted?

Looking at aw99706_probe() in drivers/video/backlight/aw99706.c:

	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);

Since the hardware enable GPIO is only requested and asserted later in the
probe sequence, if the bootloader hasn't pre-enabled the hardware, the chip
will remain powered off or in reset during the earlier I2C read. This would
cause the probe function to fail to read the chip ID and abort.

[Severity: High]
This is a pre-existing issue, but does this sequence mask negative error codes
from aw99706_chip_id_read()?

Looking at the chip ID verification in 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);

If the I2C controller or bus is not yet ready, it could return -EPROBE_DEFER,
which is then replaced with -ENODEV. This would break the kernel's probe
deferral mechanism and cause a permanent probe failure. It also prints the
negative integer using %02x, which emits a two's complement hexadecimal value
instead of a clear error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803141310.1379194-1-junjie.cao@linux.dev?part=1