linux-kernel - Re: [RFC PATCH 1/2] backlight: Rename duplicated devices to support dual-backlight setups

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7dc6a9e5171bc70be23188ffd8c45168fa79aacb@intel.com>
Date: Mon, 26 May 2025 11:53:40 +0300
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Pengyu Luo <mitltlatltl@...il.com>, Lee Jones <lee@...nel.org>, Daniel
 Thompson <danielt@...nel.org>, Jingoo Han <jingoohan1@...il.com>, Helge
 Deller <deller@....de>
Cc: dri-devel@...ts.freedesktop.org, linux-fbdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, Pengyu Luo <mitltlatltl@...il.com>
Subject: Re: [RFC PATCH 1/2] backlight: Rename duplicated devices to support
 dual-backlight setups

On Sun, 25 May 2025, Pengyu Luo <mitltlatltl@...il.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 <danielt@...nel.org>
> Signed-off-by: Pengyu Luo <mitltlatltl@...il.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