On Sun, 25 May 2025, Pengyu Luo <mitltlat...@gmail.com> wrote:
> When registering a backlight device, if a device with the same name
> already exists, append "-secondary" to the new device's name. This is
> useful for platforms with dual backlight drivers (e.g. some panels use
> dual ktz8866), where both instances need to coexist.
>
> For now, only one secondary instance is supported. If more instances
> are needed, this logic can be extended with auto-increment or a more
> flexible naming scheme.
I think for both patches you should consider adding a new interface for
creating dual backlight scenarios.
For example, this patch turns a driver error (registering two or more
backlights with the same name) into a special use case, patch 2
magically connecting the two, and hiding the problem.
With i915, you could have multiple devices, each with multiple
independent panels with independent backlights. I think accidentally
trying to register more than one backlight with the same name should
remain an error, *unless* you want the special case of combined
backlights.
Similarly, what if you encounter a device with two panels, and two
*independent* ktz8866?
Please be explicit rather than implicit.
BR,
Jani.
>
> Suggested-by: Daniel Thompson <dani...@kernel.org>
> Signed-off-by: Pengyu Luo <mitltlat...@gmail.com>
> ---
> drivers/video/backlight/backlight.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index 9dc93c5e4..991702f5d 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -365,7 +365,8 @@ struct backlight_device *backlight_device_register(const
> char *name,
> struct device *parent, void *devdata, const struct backlight_ops *ops,
> const struct backlight_properties *props)
> {
> - struct backlight_device *new_bd;
> + struct backlight_device *new_bd, *prev_bd;
> + const char *new_name = NULL;
> int rc;
>
> pr_debug("backlight_device_register: name=%s\n", name);
> @@ -377,10 +378,23 @@ struct backlight_device
> *backlight_device_register(const char *name,
> mutex_init(&new_bd->update_lock);
> mutex_init(&new_bd->ops_lock);
>
> + /*
> + * If there is an instance with the same name already, then rename it.
> + * We also can use an auto-increment field, but it seems that there is
> + * no triple or quad case.
> + */
> + prev_bd = backlight_device_get_by_name(name);
> + if (!IS_ERR_OR_NULL(prev_bd)) {
> + new_name = kasprintf(GFP_KERNEL, "%s-secondary", name);
> + if (!new_name)
> + return ERR_PTR(-ENOMEM);
> + put_device(&prev_bd->dev);
> + }
> +
> new_bd->dev.class = &backlight_class;
> new_bd->dev.parent = parent;
> new_bd->dev.release = bl_device_release;
> - dev_set_name(&new_bd->dev, "%s", name);
> + dev_set_name(&new_bd->dev, "%s", new_name ? new_name : name);
> dev_set_drvdata(&new_bd->dev, devdata);
>
> /* Set default properties */
> @@ -414,6 +428,8 @@ struct backlight_device *backlight_device_register(const
> char *name,
> list_add(&new_bd->entry, &backlight_dev_list);
> mutex_unlock(&backlight_dev_list_mutex);
>
> + kfree(new_name);
> +
> return new_bd;
> }
> EXPORT_SYMBOL(backlight_device_register);
--
Jani Nikula, Intel
