diff mbox series

[4/7] driver core: clean up the logic to determine which /sys/dev/ directory to use

Message ID 20230331093318.82288-4-gregkh@linuxfoundation.org
State New
Headers
Series [1/7] driver core: core: move to use class_to_subsys() |

Commit Message

Greg KH March 31, 2023, 9:33 a.m. UTC 
  When a dev_t is set in a struct device, an symlink in /sys/dev/ is
created for it either under /sys/dev/block/ or /sys/dev/char/ depending
on the device type.

The logic to determine this would trigger off of the class of the
object, and the kobj_type set in that location.  But it turns out that
this deep nesting isn't needed at all, as it's either a choice of block
or "everything else" which is a char device.  So make the logic a lot
more simple and obvious, and remove the incorrect comments in the code
that tried to document something that was not happening at all (it is
impossible to set class->dev_kobj to NULL as the class core prevented
that from happening.

This removes the only place that class->dev_kobj was being used, so
after this, it can be removed entirely.

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/base.h     | 10 ++++++++++
 drivers/base/core.c     | 22 ++++------------------
 drivers/base/devtmpfs.c |  9 ---------
 3 files changed, 14 insertions(+), 27 deletions(-)

Comments

Rafael J. Wysocki March 31, 2023, 2:49 p.m. UTC | #1 
On Fri, Mar 31, 2023 at 11:33 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> When a dev_t is set in a struct device, an symlink in /sys/dev/ is
> created for it either under /sys/dev/block/ or /sys/dev/char/ depending
> on the device type.
>
> The logic to determine this would trigger off of the class of the
> object, and the kobj_type set in that location.  But it turns out that
> this deep nesting isn't needed at all, as it's either a choice of block
> or "everything else" which is a char device.  So make the logic a lot
> more simple and obvious, and remove the incorrect comments in the code
> that tried to document something that was not happening at all (it is
> impossible to set class->dev_kobj to NULL as the class core prevented
> that from happening.
>
> This removes the only place that class->dev_kobj was being used, so
> after this, it can be removed entirely.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I like this.

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

> ---
>  drivers/base/base.h     | 10 ++++++++++
>  drivers/base/core.c     | 22 ++++------------------
>  drivers/base/devtmpfs.c |  9 ---------
>  3 files changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 6296164bb7f3..4660e1159ee0 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -209,6 +209,16 @@ int devtmpfs_init(void);
>  static inline int devtmpfs_init(void) { return 0; }
>  #endif
>
> +#ifdef CONFIG_BLOCK
> +extern struct class block_class;
> +static inline bool is_blockdev(struct device *dev)
> +{
> +       return dev->class == &block_class;
> +}
> +#else
> +static inline bool is_blockdev(struct device *dev) { return false; }
> +#endif
> +
>  /* Device links support */
>  int device_links_read_lock(void);
>  void device_links_read_unlock(int idx);
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index e3bc34fcf779..dbc2ba6dfffc 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3430,27 +3430,13 @@ int dev_set_name(struct device *dev, const char *fmt, ...)
>  }
>  EXPORT_SYMBOL_GPL(dev_set_name);
>
> -/**
> - * device_to_dev_kobj - select a /sys/dev/ directory for the device
> - * @dev: device
> - *
> - * By default we select char/ for new entries.  Setting class->dev_obj
> - * to NULL prevents an entry from being created.  class->dev_kobj must
> - * be set (or cleared) before any devices are registered to the class
> - * otherwise device_create_sys_dev_entry() and
> - * device_remove_sys_dev_entry() will disagree about the presence of
> - * the link.
> - */
> +/* select a /sys/dev/ directory for the device */
>  static struct kobject *device_to_dev_kobj(struct device *dev)
>  {
> -       struct kobject *kobj;
> -
> -       if (dev->class)
> -               kobj = dev->class->dev_kobj;
> +       if (is_blockdev(dev))
> +               return sysfs_dev_block_kobj;
>         else
> -               kobj = sysfs_dev_char_kobj;
> -
> -       return kobj;
> +               return sysfs_dev_char_kobj;
>  }
>
>  static int device_create_sys_dev_entry(struct device *dev)
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index ae72d4ba8547..b848764ef018 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -94,15 +94,6 @@ static struct file_system_type dev_fs_type = {
>         .mount = public_dev_mount,
>  };
>
> -#ifdef CONFIG_BLOCK
> -static inline int is_blockdev(struct device *dev)
> -{
> -       return dev->class == &block_class;
> -}
> -#else
> -static inline int is_blockdev(struct device *dev) { return 0; }
> -#endif
> -
>  static int devtmpfs_submit_req(struct req *req, const char *tmp)
>  {
>         init_completion(&req->done);
> --
> 2.40.0
>
diff mbox series

Patch

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 6296164bb7f3..4660e1159ee0 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -209,6 +209,16 @@  int devtmpfs_init(void);
 static inline int devtmpfs_init(void) { return 0; }
 #endif
 
+#ifdef CONFIG_BLOCK
+extern struct class block_class;
+static inline bool is_blockdev(struct device *dev)
+{
+	return dev->class == &block_class;
+}
+#else
+static inline bool is_blockdev(struct device *dev) { return false; }
+#endif
+
 /* Device links support */
 int device_links_read_lock(void);
 void device_links_read_unlock(int idx);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index e3bc34fcf779..dbc2ba6dfffc 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3430,27 +3430,13 @@  int dev_set_name(struct device *dev, const char *fmt, ...)
 }
 EXPORT_SYMBOL_GPL(dev_set_name);
 
-/**
- * device_to_dev_kobj - select a /sys/dev/ directory for the device
- * @dev: device
- *
- * By default we select char/ for new entries.  Setting class->dev_obj
- * to NULL prevents an entry from being created.  class->dev_kobj must
- * be set (or cleared) before any devices are registered to the class
- * otherwise device_create_sys_dev_entry() and
- * device_remove_sys_dev_entry() will disagree about the presence of
- * the link.
- */
+/* select a /sys/dev/ directory for the device */
 static struct kobject *device_to_dev_kobj(struct device *dev)
 {
-	struct kobject *kobj;
-
-	if (dev->class)
-		kobj = dev->class->dev_kobj;
+	if (is_blockdev(dev))
+		return sysfs_dev_block_kobj;
 	else
-		kobj = sysfs_dev_char_kobj;
-
-	return kobj;
+		return sysfs_dev_char_kobj;
 }
 
 static int device_create_sys_dev_entry(struct device *dev)
diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index ae72d4ba8547..b848764ef018 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -94,15 +94,6 @@  static struct file_system_type dev_fs_type = {
 	.mount = public_dev_mount,
 };
 
-#ifdef CONFIG_BLOCK
-static inline int is_blockdev(struct device *dev)
-{
-	return dev->class == &block_class;
-}
-#else
-static inline int is_blockdev(struct device *dev) { return 0; }
-#endif
-
 static int devtmpfs_submit_req(struct req *req, const char *tmp)
 {
 	init_completion(&req->done);