From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eduardo Valentin Subject: Re: [RFC PATCHv2 1/2] Export SoC info through sysfs Date: Fri, 11 Mar 2011 15:50:43 +0200 Message-ID: <20110311135043.GA9320@besouro.research.nokia.com> References: <1299846911-15782-1-git-send-email-maxime.coquelin-nonst@stericsson.com> <1299846911-15782-2-git-send-email-maxime.coquelin-nonst@stericsson.com> Reply-To: eduardo.valentin@nokia.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1299846911-15782-2-git-send-email-maxime.coquelin-nonst@stericsson.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: ext Maxime Coquelin Cc: ext Nishanth Menon , ext Tony Lindgren , Peter De-Schrijver , Linus Walleij , Ambresh , Saravana Kannan , Andrei Warkentin , Lee Jones , Rabin VINCENT , Russell King , Jonas ABERG , ext Kevin Hilman , David Brown , "linux-arm-msm@vger.kernel.org" , Loic PALLARDY , "eduardo.valentin@nokia.com" , maxime_coquelin@yahoo.fr, Ryan Mallon , Linux-OMAP , "linux-arm-kernel@lists.infradead.org" , Daniel List-Id: linux-arm-msm@vger.kernel.org Hello, On Fri, Mar 11, 2011 at 01:35:10PM +0100, ext Maxime Coquelin wrote: > Common base to export System-on-Chip related informations through sysfs. > > Creation of a "socinfo" directory under /sys/. > Creation of SoC information entries. > > Signed-off-by: Maxime COQUELIN > --- > Documentation/ABI/testing/sysfs-socinfo | 16 ++++++ > drivers/base/Kconfig | 3 + > drivers/base/Makefile | 1 + > drivers/base/soc.c | 79 +++++++++++++++++++++++++++++++ > include/linux/sys_soc.h | 50 +++++++++++++++++++ > 5 files changed, 149 insertions(+), 0 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-socinfo > create mode 100644 drivers/base/soc.c > create mode 100644 include/linux/sys_soc.h > > diff --git a/Documentation/ABI/testing/sysfs-socinfo b/Documentation/ABI/testing/sysfs-socinfo > new file mode 100644 > index 0000000..afd9da2 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-socinfo > @@ -0,0 +1,16 @@ > +What: /sys/socinfo > +Date: March 2011 > +contact: Maxime Coquelin > +Description: > + The /sys/socinfo directory contains information about the > + System-on-Chip. It is only available if platform implements it. > + This directory contains two kind of attributes : > + - common attributes: > + * machine: the name of the machine. > + * family: the family name of the SoC > + - SoC-specific attributes: The SoC vendor can declare attributes > + to export some strings to user-space, like the serial-number for > + example. > + > +Users: > + User-space applications which needs these kind of attributes. > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > index d57e8d0..372ef3a 100644 > --- a/drivers/base/Kconfig > +++ b/drivers/base/Kconfig > @@ -168,4 +168,7 @@ config SYS_HYPERVISOR > bool > default n > > +config SYS_SOC > + bool > + > endmenu > diff --git a/drivers/base/Makefile b/drivers/base/Makefile > index 5f51c3b..f3bcfb3 100644 > --- a/drivers/base/Makefile > +++ b/drivers/base/Makefile > @@ -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 > > diff --git a/drivers/base/soc.c b/drivers/base/soc.c > new file mode 100644 > index 0000000..046b43b > --- /dev/null > +++ b/drivers/base/soc.c > @@ -0,0 +1,79 @@ > +/* > + * Copyright (C) ST-Ericsson SA 2011 > + * Author: Maxime Coquelin for ST-Ericsson. > + * License terms: GNU General Public License (GPL), version 2 > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +struct kobject *soc_object; > + > +ssize_t show_soc_info(struct kobject *kobj, > + struct kobj_attribute *attr, char *buf) > +{ > + struct sysfs_soc_info *si = container_of(attr, > + struct sysfs_soc_info, attr); > + > + if (si->info) > + return sprintf(buf, "%s\n", si->info); > + > + return si->get_info(buf, si); > +} > + > +int __init register_sysfs_soc_info(struct sysfs_soc_info *info, int nb_info) > +{ > + int i, ret; > + > + for (i = 0; i < nb_info; i++) { > + ret = sysfs_create_file(soc_object, &info[i].attr.attr); > + if (ret) { > + for (i -= 1; i >= 0; i--) > + sysfs_remove_file(soc_object, &info[i].attr.attr); > + break; > + } > + } > + > + return ret; > +} >>From functional perspective, this looks like a sysfs_create_group. Now it makes me wonder if this thing would make sense. Maybe it's better to create a node under platform and then add attributes to it, as suggested in V1 thread. I don't know, this could still be done by the socinfo code. I mean, location is still an issue it seams :-) Another thing, what could be done is, instead of creating new data structures to hold the attributes, a struct attribute_group could be pass instead during registration time. What do you think?? > + > +static struct attribute *soc_attrs[] = { > + NULL, > +}; > + > +static struct attribute_group soc_attr_group = { > + .attrs = soc_attrs, > +}; What is the point of the above two ? > + > +int __init register_sysfs_soc(struct sysfs_soc_info *info, size_t num) > +{ > + int ret; > + > + soc_object = kobject_create_and_add("socinfo", NULL); > + if (!soc_object) { > + ret = -ENOMEM; > + goto exit; > + } > + > + ret = sysfs_create_group(soc_object, &soc_attr_group); > + if (ret) > + goto kset_exit; You add an empty group here. > + > + ret = register_sysfs_soc_info(info, num); > + if (ret) > + goto group_exit; But the real thing happens here. > + > + return 0; > + > +group_exit: > + sysfs_remove_group(soc_object, &soc_attr_group); > +kset_exit: > + kobject_put(soc_object); > +exit: > + return ret; > +} > + > diff --git a/include/linux/sys_soc.h b/include/linux/sys_soc.h > new file mode 100644 > index 0000000..05e5529 > --- /dev/null > +++ b/include/linux/sys_soc.h > @@ -0,0 +1,50 @@ > +/* > + * Copyright (C) ST-Ericsson SA 2011 > + * Author: Maxime Coquelin for ST-Ericsson. > + * License terms: GNU General Public License (GPL), version 2 > + */ > +#ifndef __SYS_SOC_H > +#define __SYS_SOC_H > + > +#include > + > +/** > + * struct sys_soc_info - SoC exports related informations > + * @name: name of the export > + * @info: pointer on the key to export > + * @get_info: callback to retrieve key if info field is NULL > + * @attr: export's sysdev class attribute > + */ > +struct sysfs_soc_info { > + const char *info; > + ssize_t (*get_info)(char *buf, struct sysfs_soc_info *); > + struct kobj_attribute attr; > +}; > + > +ssize_t show_soc_info(struct kobject *, struct kobj_attribute *, char *); > + > +#define SYSFS_SOC_ATTR_VALUE(_name, _value) { \ > + .attr.attr.name = _name, \ > + .attr.attr.mode = S_IRUGO, \ > + .attr.show = show_soc_info, \ > + .info = _value, \ > +} > + > +#define SYSFS_SOC_ATTR_CALLBACK(_name, _callback) { \ > + .attr.attr.name = _name, \ > + .attr.attr.mode = S_IRUGO, \ > + .attr.show = show_soc_info, \ > + .get_info = _callback, \ > +} > + > +/** > + * register_sys_soc - register the soc information > + * @name: name of the machine > + * @info: pointer on the info table to export > + * @num: number of info to export > + * > + * NOTE: This function must only be called once > + */ > +int register_sysfs_soc(struct sysfs_soc_info *info, size_t num); > + > +#endif /* __SYS_SOC_H */ > -- > 1.7.1 -- Eduardo Valentin