Framework for exporting System-on-Chip information via sysfs
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
From: Lee Jones <[email protected]>
Signed-off-by Lee Jones <[email protected]>
--- drivers/base/Kconfig | 3 + drivers/base/Makefile | 1 + drivers/base/soc.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 29 ++++++++++++++ 4 files changed, 129 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.h
drivers/base/Kconfig +3 -0
@@ -168,6 +168,9 @@ config SYS_HYPERVISOR bool default n +config SYS_SOC + bool + config ARCH_NO_SYSDEV_OPS bool ---help---
drivers/base/Makefile +1 -0
@@ -18,6 +18,7 @@ ifeq ($(CONFIG_SYSFS),y) obj-$(CONFIG_MODULES) += module.o endif obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o +obj-$(CONFIG_SYS_SOC) += soc.o ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
drivers/base/soc.c new file +96 -0
96 lines not on this page · raw message
include/linux/sys_soc.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* Copyright (C) ST-Ericsson SA 2011 +* Author: Lee Jones <[email protected]> for ST-Ericsson. +* License terms: GNU General Public License (GPL), version 2 +*/ +#ifndef __SYS_SOC_H +#define __SYS_SOC_H + +#include <linux/kobject.h> +#include <linux/device.h> + +#define MAX_SOCS 8 + +/** +* soc_device_register - register SoC as a device +* @soc_attr: Array of sysfs file attributes +* @soc: Parent node '/sys/devices/soc' +*/ +int soc_device_register (struct device_attribute *soc_attrs[], +int num_socs); +/** +* soc_device_unregister - unregister SoC as a device +* @soc_attr: Array of sysfs file attributes +* @soc: Parent node '/sys/devices/soc' +*/ +void soc_device_unregister (struct device_attribute *soc_attrs[], +int num_socs); + +#endif /* __SYS_SOC_H */
Signature · 1 line
1.7.4.1
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
Hi Lee,
Works fine on my platform, but I have a couple of questions.
Quoted 49 lines · Lee Jones
From: Lee Jones <[email protected]>
Signed-off-by Lee Jones <[email protected]>--- drivers/base/Kconfig | 3 + drivers/base/Makefile | 1 + drivers/base/soc.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 29 ++++++++++++++ 4 files changed, 129 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.hdrivers/base/Kconfig +3
-0@@ -168,6 +168,9 @@ config SYS_HYPERVISOR bool default n +config SYS_SOC + bool + config ARCH_NO_SYSDEV_OPS bool ---help---drivers/base/Makefile +1
-0@@ -18,6 +18,7 @@ ifeq ($(CONFIG_SYSFS),y) obj-$(CONFIG_MODULES) += module.o endif obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o +obj-$(CONFIG_SYS_SOC) += soc.o ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUGdrivers/base/soc.c new file +5
-0@@ -0,0 +1,96 @@ +/* +* Copyright (C) ST-Ericsson SA 2011 +* +* Author: Lee Jones <[email protected]> for ST-Ericsson. +* License terms: GNU General Public License (GPL), version 2
drivers/base/soc.c line 15
Quoted 10 lines
drivers/base/soc.c new file +10
-0+*/ + +#include <linux/sysfs.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/stat.h> +#include <linux/slab.h> +#include <linux/sys_soc.h> + +static char *soc_names[] = {"1" ,"2" ,"3" ,"4" ,"5" ,"6" ,"7" ,"8" };
Could this be made const? Actually, could we just use dev_set_name() and do it at runtime?
drivers/base/soc.c line 16
Quoted 1 line
drivers/base/soc.c new file +1
-0+static struct device parent_soc = { .init_name ="soc" , };
I don't think this device can be statically allocated.
drivers/base/soc.c line 23
Quoted 7 lines
drivers/base/soc.c new file +7
-0+struct device *soc[MAX_SOCS ]; + +static int soc_device_create_files (struct device *soc, +struct device_attribute soc_attrs[]); + +static void soc_device_remove_files (struct device *soc, +struct device_attribute soc_attrs[]);
If you move soc_device_remove_files() above soc_device_create_files() then I don't think you need these prototypes.
Quoted 11 lines
drivers/base/soc.c new file +11
-0+ +static int __init soc_device_create_files (struct device *soc, +struct device_attribute soc_attrs[]) +{ +int ret =0 ; +int i =0 ; + +while (soc_attrs[i].attr.name !=NULL ) { + ret =device_create_file (soc, &soc_attrs[i++]); +if (ret) + goto out;
drivers/base/soc.c line 44
Quoted 10 lines
drivers/base/soc.c new file +10
-0+ } +return ret; + +out: +soc_device_remove_files (soc, soc_attrs); +return ret; +} + +static void __exit soc_device_remove_files(struct device *soc, +struct device_attribute soc_attrs[])
This is used in the cleanup path of soc_device_create_files() so can't be annotated with __exit:
soc_device_remove_files' referenced in section .init.text' of drivers/built-in.o: defined in discarded section `.exit.text' of drivers/built-in.o
drivers/base/soc.c line 55
Quoted 11 lines
drivers/base/soc.c new file +11
-0+{ +int i =0 ; + +while (soc_attrs[i++].attr.name !=NULL ) +device_remove_file (soc, &soc_attrs[i]); +} + +int __init soc_device_register (struct device_attribute *soc_attrs[], +int soc_count) +{ +int ret, i;
Shouldn't this check that soc_count < MAX_SOCS?
drivers/base/soc.c line 60
Quoted 5 lines
drivers/base/soc.c new file +5
-0+ +/* Register top-level SoC device '/sys/devices/soc' */ + ret =device_register (&parent_soc); +if (ret) +return ret;
I think that this device needs to be dynamically allocated and have a release function that does a kfree() on it.
drivers/base/soc.c line 68
Quoted 8 lines
drivers/base/soc.c new file +8
-0+ +/* Register each SoC and populate sysfs with requested attributes */ +for (i =0 ; i < soc_count -1 ; i++) { + soc[i] =kmalloc (sizeof (struct device ),GFP_KERNEL ); +if (!soc[i]) +return -ENOMEM ; + +memset (soc[i],0 ,sizeof (struct device ));
kzalloc() and remove the memset()?
drivers/base/soc.c line 69
Quoted 1 line
drivers/base/soc.c new file +1
-0+ soc[i]->init_name = soc_names[i];
It would be good to use dev_set_name() here, but I wonder if perhaps the devices should be called soc1, soc2 etc rather than 1, 2?
Quoted 12 lines
drivers/base/soc.c new file +12
-0+ soc[i]->parent = &parent_soc; + + ret =device_register (soc[i]); +if (ret) +return ret; + +soc_device_create_files (soc[i], soc_attrs[i]); + } + +return ret; +} +
drivers/base/soc.c line 91
Quoted 10 lines
drivers/base/soc.c new file +10
-0+void __exit soc_device_unregister(struct device_attribute *soc_attrs[], +int soc_count) +{ +int i; + +/* Unregister and free all SoC from sysfs */ +for (i =0 ; i < soc_count -1 ; i++) { +soc_device_remove_files (soc[i], soc_attrs[i]); +device_unregister (soc[i]); +kfree (soc[i]);
The kfree() should be done in the .release() method otherwise the device could be freed whilst someone still holds a reference.
Quoted 47 lines
drivers/base/soc.c new file +5
-0+ } + +/* Unregister top-level SoC device '/sys/devices/soc' */ +device_unregister (&parent_soc); +}include/linux/sys_soc.h new file +29
-0@@ -0,0 +1,29 @@ +/* +* Copyright (C) ST-Ericsson SA 2011 +* Author: Lee Jones <[email protected]> for ST-Ericsson. +* License terms: GNU General Public License (GPL), version 2 +*/ +#ifndef __SYS_SOC_H +#define __SYS_SOC_H + +#include <linux/kobject.h> +#include <linux/device.h> + +#define MAX_SOCS 8 + +/** +* soc_device_register - register SoC as a device +* @soc_attr: Array of sysfs file attributes +* @soc: Parent node '/sys/devices/soc' +*/ +int soc_device_register (struct device_attribute *soc_attrs[], +int num_socs); +/** +* soc_device_unregister - unregister SoC as a device +* @soc_attr: Array of sysfs file attributes +* @soc: Parent node '/sys/devices/soc' +*/ +void soc_device_unregister (struct device_attribute *soc_attrs[], +int num_socs); + +#endif /* __SYS_SOC_H */ Signature · 6 lines
1.7.4.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
Hi James,
Firstly, thank you for taking the time to review my patch.
I hope this is more to your liking:
--------------------------------------------------------------------------------
From: Lee Jones <[email protected]>
Signed-off-by Lee Jones <[email protected]>
--- drivers/base/Kconfig | 3 + drivers/base/Makefile | 1 + drivers/base/soc.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 29 +++++++++++ 4 files changed, 160 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.h
drivers/base/Kconfig +3 -0
@@ -168,6 +168,9 @@ config SYS_HYPERVISOR bool default n +config SYS_SOC + bool + config ARCH_NO_SYSDEV_OPS bool ---help---
drivers/base/Makefile +1 -0
@@ -18,6 +18,7 @@ ifeq ($(CONFIG_SYSFS),y) obj-$(CONFIG_MODULES) += module.o endif obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o +obj-$(CONFIG_SYS_SOC) += soc.o ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
drivers/base/soc.c new file +127 -0
127 lines not on this page · raw message
include/linux/sys_soc.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* Copyright (C) ST-Ericsson SA 2011 +* Author: Lee Jones <[email protected]> for ST-Ericsson. +* License terms: GNU General Public License (GPL), version 2 +*/ +#ifndef __SYS_SOC_H +#define __SYS_SOC_H + +#include <linux/kobject.h> +#include <linux/device.h> + +#define MAX_SOCS 8 + +/** +* soc_device_register - register SoC as a device +* @soc_attr: Array of sysfs file attributes +* @num_socs: Amount of SoCs we're attempting to register +*/ +int soc_device_register (struct device_attribute *soc_attrs[], +int num_socs); +/** +* soc_device_unregister - unregister SoC as a device +* @soc_attr: Array of sysfs file attributes +* @num_socs: Amount of SoCs we're attempting to register +*/ +void soc_device_unregister (struct device_attribute *soc_attrs[], +int num_socs); + +#endif /* __SYS_SOC_H */
Signature · 1 line
1.7.4.1