Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] build: Incompatible pointer type passed to device_property_read_u32() - [Medium] gpio: Hardware left powered on upon early probe failure - [Low] backlight: TOCTOU race in aw99703_shutdown() missing update_lock --
commit b010e2c6526192785d6b65bcadf761f3928c7acd Author: Luka Panio <[email protected]> backlight: Add Awinic AW99703 backlight driver Add a driver for the Awinic AW99703 3-string WLED backlight controller. > diff --git a/drivers/video/backlight/aw99703.c > b/drivers/video/backlight/aw99703.c > new file mode 100644 > index 0000000000000..b0a46753d1765 > --- /dev/null > +++ b/drivers/video/backlight/aw99703.c [ ... ] > +static void aw99703_dt_parse(struct aw99703_device *aw, > + struct backlight_properties *props) > +{ [ ... ] > + props->max_brightness = AW99703_MAX_BRT_LVL; > + device_property_read_u32(aw->dev, "max-brightness", > + &props->max_brightness); [Severity: High] Does this code trigger a -Wincompatible-pointer-types build failure? device_property_read_u32() expects a u32 * as its third argument, but &props->max_brightness is a pointer to int. In recent kernel builds, this warning is promoted to a fatal error. > + if (props->max_brightness > AW99703_MAX_BRT_LVL) > + props->max_brightness = AW99703_MAX_BRT_LVL; > + > + props->brightness = props->max_brightness >> 1; > + device_property_read_u32(aw->dev, "default-brightness", > + &props->brightness); [Severity: High] Similarly, &props->brightness is also a pointer to int. [ ... ] > +static void aw99703_power_off(void *data) > +{ > + struct aw99703_device *aw = data; > + > + aw99703_update_brightness(aw, 0); > + gpiod_set_value_cansleep(aw->hwen_gpio, 0); > +} > + > +static int aw99703_probe(struct i2c_client *client) > +{ [ ... ] > + if (aw->hwen_gpio) { > + ret = gpiod_direction_output(aw->hwen_gpio, 1); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to drive enable gpio\n"); > + fsleep(AW99703_HWEN_ON_DELAY_US); > + } > + > + ret = aw99703_chip_id_read(aw); > + if (ret) > + return dev_err_probe(dev, ret, "chip id check failed\n"); [Severity: Medium] If the chip ID read fails, does this return without de-asserting the GPIO? The probe function sets the hwen_gpio output to 1 to power on the chip, but if aw99703_chip_id_read() or aw99703_hw_init() fail, it returns an error code immediately. Since the devm_add_action_or_reset() for aw99703_power_off is registered later in the function, the cleanup action won't execute on early probe failure, leaving the hardware permanently enabled. > + > + props.type = BACKLIGHT_RAW; > + aw99703_dt_parse(aw, &props); > + aw99703_read_boot_brightness(aw, &props); > + > + ret = aw99703_hw_init(aw); > + if (ret) > + return dev_err_probe(dev, ret, "failed to initialize chip\n"); > + > + ret = devm_add_action_or_reset(dev, aw99703_power_off, aw); [ ... ] > +static void aw99703_shutdown(struct i2c_client *client) > +{ > + aw99703_power_off(i2c_get_clientdata(client)); > +} [Severity: Low] Is there a data race between this shutdown callback and concurrent sysfs writes to the brightness attribute? aw99703_shutdown() invokes aw99703_power_off(), which modifies the aw->bl_enable flag and writes to I2C registers via aw99703_update_brightness() without acquiring the backlight bd->update_lock mutex. Since userspace tasks are not frozen during system shutdown, a concurrent sysfs operation could attempt to communicate with the chip while its registers are being written or after the power pin is de-asserted. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
