Re: [PATCH 3/3] platform/x86: dell-smbios-wmi: Replace global list with single item

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 16 Jul 2026, Armin Wolf wrote:

> There can only exist a single instance of the dell-smbios-wmi driver
> at the same time because of naming conflicts with the character
> device ("wmi/dell-smbios"). Having a global list for all instances
> thus makes no sense.
> 
> Replace the global list with a single item used by the character
> device. This simplifies the driver and allows us to mark it as
> being multi-instance safe.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/dell/dell-smbios-wmi.c | 67 ++++++++++-----------
>  1 file changed, 31 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
> index 552b80429688..f9443ddfff55 100644
> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
> @@ -10,7 +10,6 @@
>  #include <linux/device.h>
>  #include <linux/dmi.h>
>  #include <linux/fs.h>
> -#include <linux/list.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> @@ -21,7 +20,6 @@
>  #include "dell-smbios.h"
>  #include "dell-wmi-descriptor.h"
>  
> -static DECLARE_RWSEM(list_lock);
>  static int wmi_supported;
>  
>  struct misc_bios_flags_structure {
> @@ -35,20 +33,14 @@ struct misc_bios_flags_structure {
>  struct wmi_smbios_priv {
>  	struct mutex call_lock; /* Protects the content of the SMBIOS buffer */
>  	struct dell_wmi_smbios_buffer *buf;
> -	struct list_head list;
>  	struct wmi_device *wdev;
>  	struct device *child;
>  	u64 req_buf_size;
>  	struct miscdevice char_dev;
>  };
> -static LIST_HEAD(wmi_list);

Okay, nevermind the comments on patch 1.

--
 i.

>  
> -static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
> -{
> -	return list_first_entry_or_null(&wmi_list,
> -					struct wmi_smbios_priv,
> -					list);
> -}
> +static DECLARE_RWSEM(chardev_lock);	/* Protects chardev_priv */
> +static struct wmi_smbios_priv *chardev_priv;
>  
>  static int run_smbios_call(struct wmi_device *wdev)
>  {
> @@ -105,16 +97,13 @@ static int dell_smbios_wmi_call(struct device *dev, struct calling_interface_buf
>  static ssize_t dell_smbios_wmi_read(struct file *filp, char __user *buffer, size_t length,
>  				    loff_t *offset)
>  {
> -	struct wmi_smbios_priv *priv;
> -
> -	guard(rwsem_read)(&list_lock);
> +	guard(rwsem_read)(&chardev_lock);
>  
> -	priv = get_first_smbios_priv();
> -	if (!priv)
> +	if (!chardev_priv)
>  		return -ENODEV;
>  
> -	return simple_read_from_buffer(buffer, length, offset, &priv->req_buf_size,
> -				       sizeof(priv->req_buf_size));
> +	return simple_read_from_buffer(buffer, length, offset, &chardev_priv->req_buf_size,
> +				       sizeof(chardev_priv->req_buf_size));
>  }
>  
>  static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
> @@ -158,20 +147,18 @@ static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
>  static long dell_smbios_wmi_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
>  	struct dell_wmi_smbios_buffer __user *input = (struct dell_wmi_smbios_buffer __user *)arg;
> -	struct wmi_smbios_priv *priv;
>  
>  	if (cmd != DELL_WMI_SMBIOS_CMD)
>  		return -ENOIOCTLCMD;
>  
> -	guard(rwsem_read)(&list_lock);
> +	guard(rwsem_read)(&chardev_lock);
>  
> -	priv = get_first_smbios_priv();
> -	if (!priv)
> +	if (!chardev_priv)
>  		return -ENODEV;
>  
> -	guard(mutex)(&priv->call_lock);
> +	guard(mutex)(&chardev_priv->call_lock);
>  
> -	return dell_smbios_wmi_do_ioctl(priv, input);
> +	return dell_smbios_wmi_do_ioctl(chardev_priv, input);
>  }
>  
>  static const struct file_operations dell_smbios_wmi_fops = {
> @@ -188,10 +175,29 @@ static void dell_smbios_wmi_unregister_chardev(void *data)
>  	misc_deregister(char_dev);
>  }
>  
> +static void dell_smbios_wmi_clear_chardev(void *data)
> +{
> +	guard(rwsem_write)(&chardev_lock);
> +
> +	chardev_priv = NULL;
> +}
> +
>  static int dell_smbios_wmi_register_chardev(struct wmi_smbios_priv *priv)
>  {
>  	int ret;
>  
> +	scoped_guard(rwsem_write, &chardev_lock) {
> +		/* We can only have a single chardev at a time */
> +		if (chardev_priv)
> +			return -EBUSY;
> +
> +		chardev_priv = priv;
> +	}
> +
> +	ret = devm_add_action_or_reset(&priv->wdev->dev, dell_smbios_wmi_clear_chardev, NULL);
> +	if (ret < 0)
> +		return ret;
> +
>  	priv->char_dev.minor = MISC_DYNAMIC_MINOR;
>  	priv->char_dev.name = "wmi/dell-smbios";
>  	priv->char_dev.fops = &dell_smbios_wmi_fops;
> @@ -255,24 +261,12 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
>  	if (ret)
>  		return ret;
>  
> -	ret = dell_smbios_register_device(&wdev->dev, 1, &dell_smbios_wmi_call);
> -	if (ret)
> -		return ret;
> -
> -	guard(rwsem_write)(&list_lock);
> -	list_add_tail(&priv->list, &wmi_list);
> -
> -	return 0;
> +	return dell_smbios_register_device(&wdev->dev, 1, &dell_smbios_wmi_call);
>  }
>  
>  static void dell_smbios_wmi_remove(struct wmi_device *wdev)
>  {
> -	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
> -
>  	dell_smbios_unregister_device(&wdev->dev);
> -
> -	guard(rwsem_write)(&list_lock);
> -	list_del(&priv->list);
>  }
>  
>  static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
> @@ -310,6 +304,7 @@ static struct wmi_driver dell_smbios_wmi_driver = {
>  	.probe = dell_smbios_wmi_probe,
>  	.remove = dell_smbios_wmi_remove,
>  	.id_table = dell_smbios_wmi_id_table,
> +	.no_singleton = true,
>  };
>  
>  int init_dell_smbios_wmi(void)
>