Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `default-brightness` property is read from the Device Tree as a `u32` without being clamped to `LM3533_BL_MAX_BRIGHTNESS` (255). --
commit ea75baf449ff637e79d3393f0fdd68b3fcf947de Author: Svyatoslav Ryhel <[email protected]> mfd: lm3533: Convert to use OF bindings This commit removes platform data support and transitions the driver to use Device Tree bindings for its configuration. > diff --git a/drivers/video/backlight/lm3533_bl.c > b/drivers/video/backlight/lm3533_bl.c > index 9ef171d3aaeab..2c24647fc17a4 100644 > --- a/drivers/video/backlight/lm3533_bl.c > +++ b/drivers/video/backlight/lm3533_bl.c [ ... ] > @@ -292,13 +291,21 @@ static int lm3533_bl_probe(struct platform_device *pdev) > bl->cb.id = lm3533_bl_get_ctrlbank_id(bl); > bl->cb.dev = NULL; /* until registered */ > > + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d", > + pdev->name, pdev->id); > + if (!name) > + return -ENOMEM; > + > + device_property_read_u32(&pdev->dev, "default-brightness", > + &default_brightness); > + > memset(&props, 0, sizeof(props)); > props.type = BACKLIGHT_RAW; > props.max_brightness = LM3533_BL_MAX_BRIGHTNESS; > - props.brightness = pdata->default_brightness; > + props.brightness = default_brightness; [Severity: Medium] Should default_brightness be clamped to LM3533_BL_MAX_BRIGHTNESS before being assigned to props.brightness? The transition to Device Tree bindings introduces reading default-brightness as a u32. In the previous platform data implementation, default_brightness was a u8, which inherently restricted it to a 0-255 range. If a Device Tree provides a value greater than 255, it will be implicitly truncated to an 8-bit unsigned integer when passed to the hardware writing functions (for instance, 256 would wrap around to 0). This would cause the backlight to turn off instead of setting it to the expected maximum brightness. Does it make sense to clamp default_brightness similar to how led-max-microamp is clamped later in this function? > - bd = devm_backlight_device_register(&pdev->dev, pdata->name, > - pdev->dev.parent, bl, &lm3533_bl_ops, > - &props); > + > + bd = devm_backlight_device_register(&pdev->dev, name, &pdev->dev, > + bl, &lm3533_bl_ops, &props); [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
