[PATCH 03/36] cpufreq: move to use bus_get_dev_root()

[PATCH 03/36] cpufreq: move to use bus_get_dev_root()

[Greg Kroah-Hartman]

Direct access to the struct bus_type dev_root pointer is going away soon
so replace that with a call to bus_get_dev_root() instead, which is what
it is there for.

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Note, this is a patch that is a prepatory cleanup as part of a larger
series of patches that is working on resolving some old driver core
design mistakes.  It will build and apply cleanly on top of 6.3-rc2 on
its own, but I'd prefer if I could take it through my driver-core tree
so that the driver core changes can be taken through there for 6.4-rc1.

 drivers/cpufreq/cpufreq.c      | 7 ++++++-
 drivers/cpufreq/intel_pstate.c | 7 +++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6d8fd3b8dcb5..6ad3119b8e15 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2932,11 +2932,16 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
 static int __init cpufreq_core_init(void)
 {
 	struct cpufreq_governor *gov = cpufreq_default_governor();
+	struct device *dev_root;
 
 	if (cpufreq_disabled())
 		return -ENODEV;
 
-	cpufreq_global_kobject = kobject_create_and_add("cpufreq", &cpu_subsys.dev_root->kobj);
+	dev_root = bus_get_dev_root(&cpu_subsys);
+	if (dev_root) {
+		cpufreq_global_kobject = kobject_create_and_add("cpufreq", &dev_root->kobj);
+		put_device(dev_root);
+	}
 	BUG_ON(!cpufreq_global_kobject);
 
 	if (!strlen(default_governor))
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 48a4613cef1e..102cf7f0ac63 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1473,10 +1473,13 @@ static struct kobject *intel_pstate_kobject;
 
 static void __init intel_pstate_sysfs_expose_params(void)
 {
+	struct device *dev_root = bus_get_dev_root(&cpu_subsys);
 	int rc;
 
-	intel_pstate_kobject = kobject_create_and_add("intel_pstate",
-						&cpu_subsys.dev_root->kobj);
+	if (dev_root) {
+		intel_pstate_kobject = kobject_create_and_add("intel_pstate", &dev_root->kobj);
+		put_device(dev_root);
+	}
 	if (WARN_ON(!intel_pstate_kobject))
 		return;
 
-- 
2.39.2

[PATCH 06/36] cpuidle: move to use bus_get_dev_root()

[Greg Kroah-Hartman]

Direct access to the struct bus_type dev_root pointer is going away soon
so replace that with a call to bus_get_dev_root() instead, which is what
it is there for.

This allows us to clean up the cpuidle_add_interface() call a bit as it
was only called in one place, with the same argument so just put that
into the function itself.  Note that cpuidle_remove_interface() should
also probably be removed in the future as there are no callers of it for
some reason.

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Note, this is a patch that is a prepatory cleanup as part of a larger
series of patches that is working on resolving some old driver core
design mistakes.  It will build and apply cleanly on top of 6.3-rc2 on
its own, but I'd prefer if I could take it through my driver-core tree
so that the driver core changes can be taken through there for 6.4-rc1.

 drivers/cpuidle/cpuidle.c |  2 +-
 drivers/cpuidle/cpuidle.h |  2 +-
 drivers/cpuidle/sysfs.c   | 12 +++++++++---
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 0b00f21cefe3..8e929f6602ce 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -808,7 +808,7 @@ static int __init cpuidle_init(void)
 	if (cpuidle_disabled())
 		return -ENODEV;
 
-	return cpuidle_add_interface(cpu_subsys.dev_root);
+	return cpuidle_add_interface();
 }
 
 module_param(off, int, 0444);
diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
index 9f336af17fa6..52701d9588f1 100644
--- a/drivers/cpuidle/cpuidle.h
+++ b/drivers/cpuidle/cpuidle.h
@@ -30,7 +30,7 @@ extern int cpuidle_switch_governor(struct cpuidle_governor *gov);
 
 struct device;
 
-extern int cpuidle_add_interface(struct device *dev);
+extern int cpuidle_add_interface(void);
 extern void cpuidle_remove_interface(struct device *dev);
 extern int cpuidle_add_device_sysfs(struct cpuidle_device *device);
 extern void cpuidle_remove_device_sysfs(struct cpuidle_device *device);
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 48948b171749..84e4946f1072 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -119,11 +119,17 @@ static struct attribute_group cpuidle_attr_group = {
 
 /**
  * cpuidle_add_interface - add CPU global sysfs attributes
- * @dev: the target device
  */
-int cpuidle_add_interface(struct device *dev)
+int cpuidle_add_interface(void)
 {
-	return sysfs_create_group(&dev->kobj, &cpuidle_attr_group);
+	struct device *dev_root = bus_get_dev_root(&cpu_subsys);
+	int retval = -EINVAL;
+
+	if (dev_root) {
+		retval = sysfs_create_group(&dev_root->kobj, &cpuidle_attr_group);
+		put_device(dev_root);
+	}
+	return retval;
 }
 
 /**
-- 
2.39.2

[PATCH 20/36] cpufreq: amd-pstate: move to use bus_get_dev_root()

[Greg Kroah-Hartman]

Direct access to the struct bus_type dev_root pointer is going away soon
so replace that with a call to bus_get_dev_root() instead, which is what
it is there for.

In doing so, remove the unneded kobject structure that was only being
created to cause a subdirectory for the attributes.  The name of the
attribute group is the correct way to do this, saving code and
complexity as well as allowing the attributes to properly show up to
userspace tools (the raw kobject would not allow that.)

Cc: Huang Rui <ray.huang@amd.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Note, this is a patch that is a prepatory cleanup as part of a larger
series of patches that is working on resolving some old driver core
design mistakes.  It will build and apply cleanly on top of 6.3-rc2 on
its own, but I'd prefer if I could take it through my driver-core tree
so that the driver core changes can be taken through there for 6.4-rc1.

 drivers/cpufreq/amd-pstate.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 73c7643b2697..b92454c50118 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -63,7 +63,6 @@ static struct cpufreq_driver *current_pstate_driver;
 static struct cpufreq_driver amd_pstate_driver;
 static struct cpufreq_driver amd_pstate_epp_driver;
 static int cppc_state = AMD_PSTATE_DISABLE;
-struct kobject *amd_pstate_kobj;
 
 /*
  * AMD Energy Preference Performance (EPP)
@@ -932,6 +931,7 @@ static struct attribute *pstate_global_attributes[] = {
 };
 
 static const struct attribute_group amd_pstate_global_attr_group = {
+	.name = "amd_pstate",
 	.attrs = pstate_global_attributes,
 };
 
@@ -1253,6 +1253,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
 
 static int __init amd_pstate_init(void)
 {
+	struct device *dev_root;
 	int ret;
 
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
@@ -1299,24 +1300,19 @@ static int __init amd_pstate_init(void)
 	if (ret)
 		pr_err("failed to register with return %d\n", ret);
 
-	amd_pstate_kobj = kobject_create_and_add("amd_pstate", &cpu_subsys.dev_root->kobj);
-	if (!amd_pstate_kobj) {
-		ret = -EINVAL;
-		pr_err("global sysfs registration failed.\n");
-		goto kobject_free;
-	}
-
-	ret = sysfs_create_group(amd_pstate_kobj, &amd_pstate_global_attr_group);
-	if (ret) {
-		pr_err("sysfs attribute export failed with error %d.\n", ret);
-		goto global_attr_free;
+	dev_root = bus_get_dev_root(&cpu_subsys);
+	if (dev_root) {
+		ret = sysfs_create_group(&dev_root->kobj, &amd_pstate_global_attr_group);
+		put_device(dev_root);
+		if (ret) {
+			pr_err("sysfs attribute export failed with error %d.\n", ret);
+			goto global_attr_free;
+		}
 	}
 
 	return ret;
 
 global_attr_free:
-	kobject_put(amd_pstate_kobj);
-kobject_free:
 	cpufreq_unregister_driver(current_pstate_driver);
 	return ret;
 }
-- 
2.39.2

Re: [PATCH 03/36] cpufreq: move to use bus_get_dev_root()

[Rafael J. Wysocki]

On Mon, Mar 13, 2023 at 7:29 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> Direct access to the struct bus_type dev_root pointer is going away soon
> so replace that with a call to bus_get_dev_root() instead, which is what
> it is there for.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Note, this is a patch that is a prepatory cleanup as part of a larger
> series of patches that is working on resolving some old driver core
> design mistakes.  It will build and apply cleanly on top of 6.3-rc2 on
> its own, but I'd prefer if I could take it through my driver-core tree
> so that the driver core changes can be taken through there for 6.4-rc1.
>
>  drivers/cpufreq/cpufreq.c      | 7 ++++++-
>  drivers/cpufreq/intel_pstate.c | 7 +++++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 6d8fd3b8dcb5..6ad3119b8e15 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2932,11 +2932,16 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
>  static int __init cpufreq_core_init(void)
>  {
>         struct cpufreq_governor *gov = cpufreq_default_governor();
> +       struct device *dev_root;
>
>         if (cpufreq_disabled())
>                 return -ENODEV;
>
> -       cpufreq_global_kobject = kobject_create_and_add("cpufreq", &cpu_subsys.dev_root->kobj);
> +       dev_root = bus_get_dev_root(&cpu_subsys);
> +       if (dev_root) {
> +               cpufreq_global_kobject = kobject_create_and_add("cpufreq", &dev_root->kobj);
> +               put_device(dev_root);
> +       }
>         BUG_ON(!cpufreq_global_kobject);
>
>         if (!strlen(default_governor))
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 48a4613cef1e..102cf7f0ac63 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1473,10 +1473,13 @@ static struct kobject *intel_pstate_kobject;
>
>  static void __init intel_pstate_sysfs_expose_params(void)
>  {
> +       struct device *dev_root = bus_get_dev_root(&cpu_subsys);
>         int rc;
>
> -       intel_pstate_kobject = kobject_create_and_add("intel_pstate",
> -                                               &cpu_subsys.dev_root->kobj);
> +       if (dev_root) {
> +               intel_pstate_kobject = kobject_create_and_add("intel_pstate", &dev_root->kobj);
> +               put_device(dev_root);
> +       }
>         if (WARN_ON(!intel_pstate_kobject))
>                 return;
>
> --
> 2.39.2
>

Re: [PATCH 06/36] cpuidle: move to use bus_get_dev_root()

[Rafael J. Wysocki]

On Mon, Mar 13, 2023 at 7:30 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> Direct access to the struct bus_type dev_root pointer is going away soon
> so replace that with a call to bus_get_dev_root() instead, which is what
> it is there for.
>
> This allows us to clean up the cpuidle_add_interface() call a bit as it
> was only called in one place, with the same argument so just put that
> into the function itself.  Note that cpuidle_remove_interface() should
> also probably be removed in the future as there are no callers of it for
> some reason.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Note, this is a patch that is a prepatory cleanup as part of a larger
> series of patches that is working on resolving some old driver core
> design mistakes.  It will build and apply cleanly on top of 6.3-rc2 on
> its own, but I'd prefer if I could take it through my driver-core tree
> so that the driver core changes can be taken through there for 6.4-rc1.
>
>  drivers/cpuidle/cpuidle.c |  2 +-
>  drivers/cpuidle/cpuidle.h |  2 +-
>  drivers/cpuidle/sysfs.c   | 12 +++++++++---
>  3 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 0b00f21cefe3..8e929f6602ce 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -808,7 +808,7 @@ static int __init cpuidle_init(void)
>         if (cpuidle_disabled())
>                 return -ENODEV;
>
> -       return cpuidle_add_interface(cpu_subsys.dev_root);
> +       return cpuidle_add_interface();
>  }
>
>  module_param(off, int, 0444);
> diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
> index 9f336af17fa6..52701d9588f1 100644
> --- a/drivers/cpuidle/cpuidle.h
> +++ b/drivers/cpuidle/cpuidle.h
> @@ -30,7 +30,7 @@ extern int cpuidle_switch_governor(struct cpuidle_governor *gov);
>
>  struct device;
>
> -extern int cpuidle_add_interface(struct device *dev);
> +extern int cpuidle_add_interface(void);
>  extern void cpuidle_remove_interface(struct device *dev);
>  extern int cpuidle_add_device_sysfs(struct cpuidle_device *device);
>  extern void cpuidle_remove_device_sysfs(struct cpuidle_device *device);
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index 48948b171749..84e4946f1072 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -119,11 +119,17 @@ static struct attribute_group cpuidle_attr_group = {
>
>  /**
>   * cpuidle_add_interface - add CPU global sysfs attributes
> - * @dev: the target device
>   */
> -int cpuidle_add_interface(struct device *dev)
> +int cpuidle_add_interface(void)
>  {
> -       return sysfs_create_group(&dev->kobj, &cpuidle_attr_group);
> +       struct device *dev_root = bus_get_dev_root(&cpu_subsys);
> +       int retval = -EINVAL;
> +
> +       if (dev_root) {
> +               retval = sysfs_create_group(&dev_root->kobj, &cpuidle_attr_group);
> +               put_device(dev_root);
> +       }
> +       return retval;

I would prefer

  if (!dev_root)
          return -EINVAL;

  retval = sysfs_create_group(&dev_root->kobj, &cpuidle_attr_group);
  put_device(dev_root);
  return retval;

assuming that successful group creation will bump up the reference
counter of dev_root.

Re: [PATCH 20/36] cpufreq: amd-pstate: move to use bus_get_dev_root()

[Huang Rui]

On Tue, Mar 14, 2023 at 02:29:02AM +0800, Greg Kroah-Hartman wrote:
> Direct access to the struct bus_type dev_root pointer is going away soon
> so replace that with a call to bus_get_dev_root() instead, which is what
> it is there for.
> 
> In doing so, remove the unneded kobject structure that was only being
> created to cause a subdirectory for the attributes.  The name of the
> attribute group is the correct way to do this, saving code and
> complexity as well as allowing the attributes to properly show up to
> userspace tools (the raw kobject would not allow that.)
> 
> Cc: Huang Rui <ray.huang@amd.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Note, this is a patch that is a prepatory cleanup as part of a larger
> series of patches that is working on resolving some old driver core
> design mistakes.  It will build and apply cleanly on top of 6.3-rc2 on
> its own, but I'd prefer if I could take it through my driver-core tree
> so that the driver core changes can be taken through there for 6.4-rc1.

Thanks Greg.

Acked-by: Huang Rui <ray.huang@.amd.com>

> 
>  drivers/cpufreq/amd-pstate.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 73c7643b2697..b92454c50118 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -63,7 +63,6 @@ static struct cpufreq_driver *current_pstate_driver;
>  static struct cpufreq_driver amd_pstate_driver;
>  static struct cpufreq_driver amd_pstate_epp_driver;
>  static int cppc_state = AMD_PSTATE_DISABLE;
> -struct kobject *amd_pstate_kobj;
>  
>  /*
>   * AMD Energy Preference Performance (EPP)
> @@ -932,6 +931,7 @@ static struct attribute *pstate_global_attributes[] = {
>  };
>  
>  static const struct attribute_group amd_pstate_global_attr_group = {
> +	.name = "amd_pstate",
>  	.attrs = pstate_global_attributes,
>  };
>  
> @@ -1253,6 +1253,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
>  
>  static int __init amd_pstate_init(void)
>  {
> +	struct device *dev_root;
>  	int ret;
>  
>  	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> @@ -1299,24 +1300,19 @@ static int __init amd_pstate_init(void)
>  	if (ret)
>  		pr_err("failed to register with return %d\n", ret);
>  
> -	amd_pstate_kobj = kobject_create_and_add("amd_pstate", &cpu_subsys.dev_root->kobj);
> -	if (!amd_pstate_kobj) {
> -		ret = -EINVAL;
> -		pr_err("global sysfs registration failed.\n");
> -		goto kobject_free;
> -	}
> -
> -	ret = sysfs_create_group(amd_pstate_kobj, &amd_pstate_global_attr_group);
> -	if (ret) {
> -		pr_err("sysfs attribute export failed with error %d.\n", ret);
> -		goto global_attr_free;
> +	dev_root = bus_get_dev_root(&cpu_subsys);
> +	if (dev_root) {
> +		ret = sysfs_create_group(&dev_root->kobj, &amd_pstate_global_attr_group);
> +		put_device(dev_root);
> +		if (ret) {
> +			pr_err("sysfs attribute export failed with error %d.\n", ret);
> +			goto global_attr_free;
> +		}
>  	}
>  
>  	return ret;
>  
>  global_attr_free:
> -	kobject_put(amd_pstate_kobj);
> -kobject_free:
>  	cpufreq_unregister_driver(current_pstate_driver);
>  	return ret;
>  }
> -- 
> 2.39.2
> 

Re: [PATCH 06/36] cpuidle: move to use bus_get_dev_root()

[Greg Kroah-Hartman]

On Mon, Mar 13, 2023 at 07:58:02PM +0100, Rafael J. Wysocki wrote:
> On Mon, Mar 13, 2023 at 7:30 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > Direct access to the struct bus_type dev_root pointer is going away soon
> > so replace that with a call to bus_get_dev_root() instead, which is what
> > it is there for.
> >
> > This allows us to clean up the cpuidle_add_interface() call a bit as it
> > was only called in one place, with the same argument so just put that
> > into the function itself.  Note that cpuidle_remove_interface() should
> > also probably be removed in the future as there are no callers of it for
> > some reason.
> >
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: linux-pm@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > Note, this is a patch that is a prepatory cleanup as part of a larger
> > series of patches that is working on resolving some old driver core
> > design mistakes.  It will build and apply cleanly on top of 6.3-rc2 on
> > its own, but I'd prefer if I could take it through my driver-core tree
> > so that the driver core changes can be taken through there for 6.4-rc1.
> >
> >  drivers/cpuidle/cpuidle.c |  2 +-
> >  drivers/cpuidle/cpuidle.h |  2 +-
> >  drivers/cpuidle/sysfs.c   | 12 +++++++++---
> >  3 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> > index 0b00f21cefe3..8e929f6602ce 100644
> > --- a/drivers/cpuidle/cpuidle.c
> > +++ b/drivers/cpuidle/cpuidle.c
> > @@ -808,7 +808,7 @@ static int __init cpuidle_init(void)
> >         if (cpuidle_disabled())
> >                 return -ENODEV;
> >
> > -       return cpuidle_add_interface(cpu_subsys.dev_root);
> > +       return cpuidle_add_interface();
> >  }
> >
> >  module_param(off, int, 0444);
> > diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
> > index 9f336af17fa6..52701d9588f1 100644
> > --- a/drivers/cpuidle/cpuidle.h
> > +++ b/drivers/cpuidle/cpuidle.h
> > @@ -30,7 +30,7 @@ extern int cpuidle_switch_governor(struct cpuidle_governor *gov);
> >
> >  struct device;
> >
> > -extern int cpuidle_add_interface(struct device *dev);
> > +extern int cpuidle_add_interface(void);
> >  extern void cpuidle_remove_interface(struct device *dev);
> >  extern int cpuidle_add_device_sysfs(struct cpuidle_device *device);
> >  extern void cpuidle_remove_device_sysfs(struct cpuidle_device *device);
> > diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> > index 48948b171749..84e4946f1072 100644
> > --- a/drivers/cpuidle/sysfs.c
> > +++ b/drivers/cpuidle/sysfs.c
> > @@ -119,11 +119,17 @@ static struct attribute_group cpuidle_attr_group = {
> >
> >  /**
> >   * cpuidle_add_interface - add CPU global sysfs attributes
> > - * @dev: the target device
> >   */
> > -int cpuidle_add_interface(struct device *dev)
> > +int cpuidle_add_interface(void)
> >  {
> > -       return sysfs_create_group(&dev->kobj, &cpuidle_attr_group);
> > +       struct device *dev_root = bus_get_dev_root(&cpu_subsys);
> > +       int retval = -EINVAL;
> > +
> > +       if (dev_root) {
> > +               retval = sysfs_create_group(&dev_root->kobj, &cpuidle_attr_group);
> > +               put_device(dev_root);
> > +       }
> > +       return retval;
> 
> I would prefer
> 
>   if (!dev_root)
>           return -EINVAL;
> 
>   retval = sysfs_create_group(&dev_root->kobj, &cpuidle_attr_group);
>   put_device(dev_root);
>   return retval;
> 
> assuming that successful group creation will bump up the reference
> counter of dev_root.

That is correct.  I'll respin this with this change in it now, thanks
for the review!

greg k-h

[PATCH v2 03/19] cpuidle: move to use bus_get_dev_root()

[Greg Kroah-Hartman]

Direct access to the struct bus_type dev_root pointer is going away soon
so replace that with a call to bus_get_dev_root() instead, which is what
it is there for.

This allows us to clean up the cpuidle_add_interface() call a bit as it
was only called in one place, with the same argument so just put that
into the function itself.  Note that cpuidle_remove_interface() should
also probably be removed in the future as there are no callers of it for
some reason.

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: - reworked cpuidle_add_interface() to exit early if dev_root is not
      valid based on review from Rafael.

 drivers/cpuidle/cpuidle.c |  2 +-
 drivers/cpuidle/cpuidle.h |  2 +-
 drivers/cpuidle/sysfs.c   | 13 ++++++++++---
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 0b00f21cefe3..8e929f6602ce 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -808,7 +808,7 @@ static int __init cpuidle_init(void)
 	if (cpuidle_disabled())
 		return -ENODEV;
 
-	return cpuidle_add_interface(cpu_subsys.dev_root);
+	return cpuidle_add_interface();
 }
 
 module_param(off, int, 0444);
diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
index 9f336af17fa6..52701d9588f1 100644
--- a/drivers/cpuidle/cpuidle.h
+++ b/drivers/cpuidle/cpuidle.h
@@ -30,7 +30,7 @@ extern int cpuidle_switch_governor(struct cpuidle_governor *gov);
 
 struct device;
 
-extern int cpuidle_add_interface(struct device *dev);
+extern int cpuidle_add_interface(void);
 extern void cpuidle_remove_interface(struct device *dev);
 extern int cpuidle_add_device_sysfs(struct cpuidle_device *device);
 extern void cpuidle_remove_device_sysfs(struct cpuidle_device *device);
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 48948b171749..d6f5da61cb7d 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -119,11 +119,18 @@ static struct attribute_group cpuidle_attr_group = {
 
 /**
  * cpuidle_add_interface - add CPU global sysfs attributes
- * @dev: the target device
  */
-int cpuidle_add_interface(struct device *dev)
+int cpuidle_add_interface(void)
 {
-	return sysfs_create_group(&dev->kobj, &cpuidle_attr_group);
+	struct device *dev_root = bus_get_dev_root(&cpu_subsys);
+	int retval;
+
+	if (!dev_root)
+		return -EINVAL;
+
+	retval = sysfs_create_group(&dev_root->kobj, &cpuidle_attr_group);
+	put_device(dev_root);
+	return retval;
 }
 
 /**
-- 
2.40.0

Re: [PATCH v2 03/19] cpuidle: move to use bus_get_dev_root()

[Rafael J. Wysocki]

On Wed, Mar 22, 2023 at 10:06 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> Direct access to the struct bus_type dev_root pointer is going away soon
> so replace that with a call to bus_get_dev_root() instead, which is what
> it is there for.
>
> This allows us to clean up the cpuidle_add_interface() call a bit as it
> was only called in one place, with the same argument so just put that
> into the function itself.  Note that cpuidle_remove_interface() should
> also probably be removed in the future as there are no callers of it for
> some reason.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

> ---
> v2: - reworked cpuidle_add_interface() to exit early if dev_root is not
>       valid based on review from Rafael.
>
>  drivers/cpuidle/cpuidle.c |  2 +-
>  drivers/cpuidle/cpuidle.h |  2 +-
>  drivers/cpuidle/sysfs.c   | 13 ++++++++++---
>  3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 0b00f21cefe3..8e929f6602ce 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -808,7 +808,7 @@ static int __init cpuidle_init(void)
>         if (cpuidle_disabled())
>                 return -ENODEV;
>
> -       return cpuidle_add_interface(cpu_subsys.dev_root);
> +       return cpuidle_add_interface();
>  }
>
>  module_param(off, int, 0444);
> diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
> index 9f336af17fa6..52701d9588f1 100644
> --- a/drivers/cpuidle/cpuidle.h
> +++ b/drivers/cpuidle/cpuidle.h
> @@ -30,7 +30,7 @@ extern int cpuidle_switch_governor(struct cpuidle_governor *gov);
>
>  struct device;
>
> -extern int cpuidle_add_interface(struct device *dev);
> +extern int cpuidle_add_interface(void);
>  extern void cpuidle_remove_interface(struct device *dev);
>  extern int cpuidle_add_device_sysfs(struct cpuidle_device *device);
>  extern void cpuidle_remove_device_sysfs(struct cpuidle_device *device);
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index 48948b171749..d6f5da61cb7d 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -119,11 +119,18 @@ static struct attribute_group cpuidle_attr_group = {
>
>  /**
>   * cpuidle_add_interface - add CPU global sysfs attributes
> - * @dev: the target device
>   */
> -int cpuidle_add_interface(struct device *dev)
> +int cpuidle_add_interface(void)
>  {
> -       return sysfs_create_group(&dev->kobj, &cpuidle_attr_group);
> +       struct device *dev_root = bus_get_dev_root(&cpu_subsys);
> +       int retval;
> +
> +       if (!dev_root)
> +               return -EINVAL;
> +
> +       retval = sysfs_create_group(&dev_root->kobj, &cpuidle_attr_group);
> +       put_device(dev_root);
> +       return retval;
>  }
>
>  /**
> --
> 2.40.0
>

Re: [PATCH v2 03/19] cpuidle: move to use bus_get_dev_root()

[Greg Kroah-Hartman]

On Wed, Mar 22, 2023 at 03:27:45PM +0100, Rafael J. Wysocki wrote:
> On Wed, Mar 22, 2023 at 10:06 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > Direct access to the struct bus_type dev_root pointer is going away soon
> > so replace that with a call to bus_get_dev_root() instead, which is what
> > it is there for.
> >
> > This allows us to clean up the cpuidle_add_interface() call a bit as it
> > was only called in one place, with the same argument so just put that
> > into the function itself.  Note that cpuidle_remove_interface() should
> > also probably be removed in the future as there are no callers of it for
> > some reason.
> >
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: linux-pm@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Acked-by: Rafael J. Wysocki <rafael@kernel.org>

Great, thanks for the quick reviews!

greg k-h