Re: [PATCH v1 5/6] video: backlight: lm3533_bl: Set initial mapping mode from DT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The driver unconditionally clears the hardware linear mapping mode if the new DT property is absent, breaking existing configurations.
- [High] The `linear` sysfs attribute is exposed to userspace before hardware initialization completes, allowing userspace configurations to be silently discarded.
--

commit cebf8bbd6f9d389030ebbf6dbf1725031e2f46a7
Author: Svyatoslav Ryhel <clamor95@gmail.com>

video: backlight: lm3533_bl: Set initial mapping mode from DT

This commit adds support to configure the initial backlight mapping mode
(linear or exponential) from the device tree instead of leaving it
unconfigured. It also updates the linear sysfs attribute handling to use
similar patterns.

> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> index 42da652df58d9..be9114b7e0ad7 100644
> --- a/drivers/video/backlight/lm3533_bl.c
> +++ b/drivers/video/backlight/lm3533_bl.c

[ ... ]

> @@ -253,8 +247,15 @@ static struct attribute_group lm3533_bl_attribute_group = {
>  
>  static int lm3533_bl_setup(struct lm3533_bl *bl)
>  {
> +	int id = lm3533_bl_get_ctrlbank_id(bl);
>  	int ret;
>  
> +	ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
> +			    bl->linear ? CTRLBANK_AB_BCONF_MODE(id) : 0,
> +			    CTRLBANK_AB_BCONF_MODE(id));

Could this break existing hardware configurations where the DT property is
absent but the bootloader already configured linear mode?

Previously, this register was left unconfigured. Because
device_property_read_bool() returns false when the property is missing, this
unconditionally forces the mapping mode to exponential (0), silently reverting
any existing setups lacking the new ti,linear-mapping-mode property.

[ ... ]

> @@ -333,6 +334,9 @@ static int lm3533_bl_probe(struct platform_device *pdev)
>  	bl->pwm = 0;
>  	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
>  
> +	bl->linear = device_property_read_bool(&pdev->dev,
> +					       "ti,linear-mapping-mode");
> +
>  	ret = lm3533_bl_setup(bl);

Does this sequence silently discard userspace configuration?

Earlier in lm3533_bl_probe(), sysfs_create_group() creates the linear sysfs
attribute before we read the device properties and call lm3533_bl_setup().

If a udev rule writes to the linear attribute, it will update the hardware
register via store_linear(), but the setup function here will subsequently
overwrite the register with the bl->linear value cached from DT, discarding
the userspace setting.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517074306.30937-1-clamor95@gmail.com?part=5