Hi On Thu, Aug 21, 2014 at 12:59 PM, Sudeep Holla <sudeep.ho...@arm.com> wrote: > From: Sudeep Holla <sudeep.ho...@arm.com> > > This patch creates a new class called "cpu" and assigns it to all the > cpu devices. This helps in grouping all the cpu devices and associated > child devices under the same class. > > This patch also: > 1. modifies the get_parent_device to return the legacy path > (/sys/devices/system/cpu/..) for the cpu class devices to support > existing sysfs ABI > 2. avoids creating link in the class directory pointing to the device as > there would be per-cpu instance of these devices with the same name > 3. makes sure subsystem symlink continues pointing to cpu bus instead of > cpu class for cpu devices
This patch lacks any explanation _why_ you add a class for CPUs. With this patch applied, these directories are effectively the same: /sys/bus/cpu/devices/ /sys/class/cpu/ Why do we need a cpu-class if the same set of information is already available on the cpu-bus? Furthermore, classes are deprecated anyway. Everything you can do with a class can be solved with a bus. And we already have a bus for cpus. Thanks David > Signed-off-by: Sudeep Holla <sudeep.ho...@arm.com> > Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org> > --- > drivers/base/core.c | 39 +++++++++++++++++++++++++++++++++------ > drivers/base/cpu.c | 7 +++++++ > include/linux/cpu.h | 2 ++ > 3 files changed, 42 insertions(+), 6 deletions(-) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 20da3ad1696b..fe622e2a48d0 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -10,6 +10,7 @@ > * > */ > > +#include <linux/cpu.h> > #include <linux/device.h> > #include <linux/err.h> > #include <linux/init.h> > @@ -742,6 +743,12 @@ static struct kobject *get_device_parent(struct device > *dev, > return &block_class.p->subsys.kobj; > } > #endif > + /* > + * if the device is in cpu class, then use the default/legacy > + * /sys/devices/system/cpu/.. path > + */ > + if (dev->class == cpu_class) > + return &parent->kobj; > > /* > * If we have no parent, we live in "virtual". > @@ -808,11 +815,17 @@ static int device_add_class_symlinks(struct device *dev) > if (!dev->class) > return 0; > > - error = sysfs_create_link(&dev->kobj, > - &dev->class->p->subsys.kobj, > - "subsystem"); > - if (error) > - goto out; > + /* > + * the subsystem symlink in each cpu device needs to continue > + * pointing to cpu bus > + */ > + if (dev->bus != &cpu_subsys) { > + error = sysfs_create_link(&dev->kobj, > + &dev->class->p->subsys.kobj, > + "subsystem"); > + if (error) > + goto out; > + } > > if (dev->parent && device_is_not_partition(dev)) { > error = sysfs_create_link(&dev->kobj, &dev->parent->kobj, > @@ -826,6 +839,13 @@ static int device_add_class_symlinks(struct device *dev) > if (sysfs_deprecated && dev->class == &block_class) > return 0; > #endif > + /* > + * don't create a link in the cpu class directory pointing to the > + * device as there would be per-cpu instance of these devices with > + * the same name > + */ > + if (dev->class == cpu_class) > + return 0; > > /* link in the class directory pointing to the device */ > error = sysfs_create_link(&dev->class->p->subsys.kobj, > @@ -851,11 +871,18 @@ static void device_remove_class_symlinks(struct device > *dev) > > if (dev->parent && device_is_not_partition(dev)) > sysfs_remove_link(&dev->kobj, "device"); > - sysfs_remove_link(&dev->kobj, "subsystem"); > + > + /* if subsystem points to cpu bus, bus_remove_device will remove it */ > + if (dev->bus != &cpu_subsys) > + sysfs_remove_link(&dev->kobj, "subsystem"); > #ifdef CONFIG_BLOCK > if (sysfs_deprecated && dev->class == &block_class) > return; > #endif > + /* symlinks are not created for cpu class devices, nothing to remove > */ > + if (dev->class == cpu_class) > + return; > + > sysfs_delete_link(&dev->class->p->subsys.kobj, &dev->kobj, > dev_name(dev)); > } > > diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c > index 277a9cfa9040..8e380214625d 100644 > --- a/drivers/base/cpu.c > +++ b/drivers/base/cpu.c > @@ -319,6 +319,7 @@ static int cpu_uevent(struct device *dev, struct > kobj_uevent_env *env) > } > #endif > > +struct class *cpu_class; > /* > * register_cpu - Setup a sysfs device for a CPU. > * @cpu - cpu->hotpluggable field set to 1 will generate a control file in > @@ -335,6 +336,8 @@ int register_cpu(struct cpu *cpu, int num) > memset(&cpu->dev, 0x00, sizeof(struct device)); > cpu->dev.id = num; > cpu->dev.bus = &cpu_subsys; > + cpu->dev.parent = cpu_subsys.dev_root; > + cpu->dev.class = cpu_class; > cpu->dev.release = cpu_device_release; > cpu->dev.offline_disabled = !cpu->hotpluggable; > cpu->dev.offline = !cpu_online(num); > @@ -420,5 +423,9 @@ void __init cpu_dev_init(void) > if (subsys_system_register(&cpu_subsys, cpu_root_attr_groups)) > panic("Failed to register CPU subsystem"); > > + cpu_class = class_create(THIS_MODULE, "cpu"); > + if (IS_ERR(cpu_class)) > + panic("Failed to register CPU class"); > + > cpu_dev_register_generic(); > } > diff --git a/include/linux/cpu.h b/include/linux/cpu.h > index 95978ad7fcdd..8c0fc9b0acad 100644 > --- a/include/linux/cpu.h > +++ b/include/linux/cpu.h > @@ -39,6 +39,8 @@ extern void cpu_remove_dev_attr(struct device_attribute > *attr); > extern int cpu_add_dev_attr_group(struct attribute_group *attrs); > extern void cpu_remove_dev_attr_group(struct attribute_group *attrs); > > +extern struct class *cpu_class; > + > #ifdef CONFIG_HOTPLUG_CPU > extern void unregister_cpu(struct cpu *cpu); > extern ssize_t arch_cpu_probe(const char *, size_t); > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/