[RFC PATCHv2 0/2] Export SoC info through sysfs

[RFC PATCHv2 0/2] Export SoC info through sysfs

[Maxime Coquelin]

Here is the second version of the proposal to export SoC related information to user-space through sysFS interface.

The first patch introduces the common part, which provides an interface to the platform to export platform-defined IDs to user-space.
The IDs strings can be provided in two ways: either with a pointer to the string, or by a callback fullfilling the sysfs's "show" callback buffer.

The second patch is given as example for the ux500 architecture. It registers the socinfo interface, and exports 5 values (Family name, machine name, silicon process, silicon revision, and serial number).

Here is the output for DB8500:
root at ME:/sys/socinfo ls -l
-r--r--r-- 1 root root 4096 Jan  2 03:35 family
-r--r--r-- 1 root root 4096 Jan  2 03:35 machine
-r--r--r-- 1 root root 4096 Jan  2 03:35 process
-r--r--r-- 1 root root 4096 Jan  2 03:35 revision
-r--r--r-- 1 root root 4096 Jan  2 03:35 soc_id
root at ME:/sys/socinfo cat machine
DB8500
root at ME:/sys/socinfo cat family
Ux500
root at ME:/sys/socinfo cat process
Standard
root at ME:/sys/socinfo cat revision
2.0
root at ME:/sys/socinfo cat soc_id
2ba07ce9e5835d6185321e9577462ef2ea2129cf

-------------------------------------------------------------------------------
v1:
	Conclusion about first version :
		* /sys/devices/system is not right place for this interface.
		* "mach_name" should be declared as other attributes to simplify code.
		* The get_info callback should use directly the "show" callback buffer.
		* Documentation was missing (ABI description required)
		* Errors were not reported at registration time.
		* Macros should be used to create attributes.
		* Some kernel coding rules not followed.

-------------------------------------------------------------------------------

Any comments are welcome.

Note that I will be off from 13th to 21th of March.

Regards,
Maxime

-------------------------------------------------------------------------------

Maxime Coquelin (2):
  Export SoC info through sysfs
  ux500: Export U8500 SoC info through sysfs

 Documentation/ABI/testing/sysfs-socinfo |   16 ++++++
 arch/arm/mach-ux500/Kconfig             |    1 +
 arch/arm/mach-ux500/id.c                |   70 +++++++++++++++++++++++++++
 drivers/base/Kconfig                    |    3 +
 drivers/base/Makefile                   |    1 +
 drivers/base/soc.c                      |   79 +++++++++++++++++++++++++++++++
 include/linux/sys_soc.h                 |   50 +++++++++++++++++++
 7 files changed, 220 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

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Maxime Coquelin]

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 <maxime.coquelin-nonst@stericsson.com>
---
 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 <maxime.coquelin-nonst@stericsson.com>
+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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#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>
+
+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;
+}
+
+static struct attribute *soc_attrs[] = {
+	NULL,
+};
+
+static struct attribute_group soc_attr_group = {
+	.attrs = soc_attrs,
+};
+
+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;
+
+	ret = register_sysfs_soc_info(info, num);
+	if (ret)
+		goto group_exit;
+
+	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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+#ifndef __SYS_SOC_H
+#define __SYS_SOC_H
+
+#include <linux/kobject.h>
+
+/**
+ * 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

[RFC PATCHv2 2/2] ux500: Export U8500 SoC info through sysfs

[Maxime Coquelin]

ST-Ericsson's U8500 implementation.
Register sysfs SoC interface, and exports:
	- Machine name
	- Family name
	- SoC ID
	- Silicon process
	- Silicon revision number.

Signed-off-by: Maxime COQUELIN <maxime.coquelin-nonst@stericsson.com>

socinfo : ux500 part 2
---
 arch/arm/mach-ux500/Kconfig |    1 +
 arch/arm/mach-ux500/id.c    |   70 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-ux500/Kconfig b/arch/arm/mach-ux500/Kconfig
index 7f8620f..08f9859 100644
--- a/arch/arm/mach-ux500/Kconfig
+++ b/arch/arm/mach-ux500/Kconfig
@@ -23,6 +23,7 @@ config MACH_U8500
 	bool "U8500 Development platform"
 	depends on UX500_SOC_DB8500
 	select TPS6105X
+	select SYS_SOC
 	help
 	  Include support for the mop500 development platform.
 
diff --git a/arch/arm/mach-ux500/id.c b/arch/arm/mach-ux500/id.c
index d35122e..457b48e 100644
--- a/arch/arm/mach-ux500/id.c
+++ b/arch/arm/mach-ux500/id.c
@@ -8,6 +8,9 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/sys_soc.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
 
 #include <asm/cputype.h>
 #include <asm/tlbflush.h>
@@ -105,3 +108,70 @@ void __init ux500_map_io(void)
 
 	ux500_print_soc_info(asicid);
 }
+
+
+#ifdef CONFIG_SYS_SOC
+#define U8500_BB_UID_BASE (U8500_BACKUPRAM1_BASE + 0xFC0)
+#define U8500_BB_UID_LENGTH 5
+
+static ssize_t ux500_get_machine(char *buf, struct sysfs_soc_info *si)
+{
+	return sprintf(buf, "DB%4x\n", dbx500_partnumber());
+}
+
+static ssize_t ux500_get_soc_id(char *buf, struct sysfs_soc_info *si)
+{
+	void __iomem *uid_base;
+	int i;
+	ssize_t sz = 0;
+
+	if (dbx500_partnumber() == 0x8500) {
+		uid_base = __io_address(U8500_BB_UID_BASE);
+		for (i = 0; i < U8500_BB_UID_LENGTH; i++)
+			sz += sprintf(buf + sz, "%08x", readl(uid_base + i * sizeof(u32)));
+		sz += sprintf(buf + sz, "\n");
+	}
+	else {
+		/* Don't know where it is located for U5500 */
+		sz = sprintf(buf, "N/A\n");
+	}
+
+	return sz;
+}
+
+static ssize_t ux500_get_revision(char *buf, struct sysfs_soc_info *si)
+{
+	unsigned int rev = dbx500_revision();
+
+	if (rev == 0x01)
+		return sprintf(buf, "%s\n", "ED");
+	else if (rev >= 0xA0)
+		return sprintf(buf, "%d.%d\n" , (rev >> 4) - 0xA + 1, rev & 0xf);
+
+	return sprintf(buf, "%s", "Unknown\n");
+}
+
+static ssize_t ux500_get_process(char *buf, struct sysfs_soc_info *si)
+{
+	if (dbx500_id.process == 0x00)
+		return sprintf(buf, "Standard\n");
+
+	return sprintf(buf, "%02xnm\n", dbx500_id.process);
+}
+
+static struct sysfs_soc_info soc_info[] = {
+	SYSFS_SOC_ATTR_CALLBACK("machine", ux500_get_machine),
+	SYSFS_SOC_ATTR_VALUE("family", "Ux500"),
+	SYSFS_SOC_ATTR_CALLBACK("soc_id", ux500_get_soc_id),
+	SYSFS_SOC_ATTR_CALLBACK("revision", ux500_get_revision),
+	SYSFS_SOC_ATTR_CALLBACK("process", ux500_get_process),
+};
+
+static int __init ux500_sys_soc_init(void)
+{
+	return register_sysfs_soc(soc_info, ARRAY_SIZE(soc_info));
+}
+
+module_init(ux500_sys_soc_init);
+#endif
+
-- 
1.7.1

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Eduardo Valentin]

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 <maxime.coquelin-nonst@stericsson.com>
> ---
>  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 <maxime.coquelin-nonst@stericsson.com>
> +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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#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>
> +
> +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;
> +}

[RFC PATCHv2 2/2] ux500: Export U8500 SoC info through sysfs

[Jean-Christophe PLAGNIOL-VILLARD]

On 13:35 Fri 11 Mar     , Maxime Coquelin wrote:
> ST-Ericsson's U8500 implementation.
> Register sysfs SoC interface, and exports:
> 	- Machine name
> 	- Family name
> 	- SoC ID
> 	- Silicon process
> 	- Silicon revision number.
I propose some weeks ago exactly the same kind of patch

and Greg point that we need to create a bus

Best Regards,
J.

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Eduardo Valentin]

Hello,

On Fri, Mar 11, 2011 at 03:32:51PM +0100, ext Arnd Bergmann wrote:
> On Friday 11 March 2011, 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 <maxime.coquelin-nonst@stericsson.com>
> 
> I think it's better than the previous patch to create an
> artificial device in /sys/devices/system/socinfo, but I'd
> still prefer the information to be attached to a real device
> that represents the SOC, as I explained in the discussion with
> Linus Walleij.

True.

> 
> You should definitely add Greg on Cc, as he's maintaining sysfs
> and certainly has an opininion here.
> 
> 	Arnd
> 
> > ---
> >  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 <maxime.coquelin-nonst@stericsson.com>
> > +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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> > + * License terms:  GNU General Public License (GPL), version 2
> > + */
> > +
> > +#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>
> > +
> > +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);
> > +}
> 
> I think this can be simplified by just letting the soc
> register its own show method instead of the get_info function.

Indeed. One way to do it is to use attribute_group, as I mentioned
on other email.

> 
> > +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;
> > +}
> > +
> > +static struct attribute *soc_attrs[] = {
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group soc_attr_group = {
> > +	.attrs = soc_attrs,
> > +};
> > +
> > +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;
> > +
> > +	ret = register_sysfs_soc_info(info, num);
> > +	if (ret)
> > +		goto group_exit;
> > +
> > +	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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> > + * License terms:  GNU General Public License (GPL), version 2
> > + */
> > +#ifndef __SYS_SOC_H
> > +#define __SYS_SOC_H
> > +
> > +#include <linux/kobject.h>
> > +
> > +/**
> > + * 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 */
> > -- 

-- 
Eduardo Valentin

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Arnd Bergmann]

On Friday 11 March 2011, 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 <maxime.coquelin-nonst@stericsson.com>

I think it's better than the previous patch to create an
artificial device in /sys/devices/system/socinfo, but I'd
still prefer the information to be attached to a real device
that represents the SOC, as I explained in the discussion with
Linus Walleij.

You should definitely add Greg on Cc, as he's maintaining sysfs
and certainly has an opininion here.

	Arnd

> ---
>  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 <maxime.coquelin-nonst@stericsson.com>
> +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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#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>
> +
> +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);
> +}

I think this can be simplified by just letting the soc
register its own show method instead of the get_info function.

> +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;
> +}
> +
> +static struct attribute *soc_attrs[] = {
> +	NULL,
> +};
> +
> +static struct attribute_group soc_attr_group = {
> +	.attrs = soc_attrs,
> +};
> +
> +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;
> +
> +	ret = register_sysfs_soc_info(info, num);
> +	if (ret)
> +		goto group_exit;
> +
> +	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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +#ifndef __SYS_SOC_H
> +#define __SYS_SOC_H
> +
> +#include <linux/kobject.h>
> +
> +/**
> + * 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 */
> -- 

[RFC PATCHv2 2/2] ux500: Export U8500 SoC info through sysfs

[Linus Walleij]

On Fri, Mar 11, 2011 at 3:11 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 13:35 Fri 11 Mar ? ? , Maxime Coquelin wrote:
>> ST-Ericsson's U8500 implementation.
>> Register sysfs SoC interface, and exports:
>> ? ? ? - Machine name
>> ? ? ? - Family name
>> ? ? ? - SoC ID
>> ? ? ? - Silicon process
>> ? ? ? - Silicon revision number.
> I propose some weeks ago exactly the same kind of patch

Could you tell which Subject: and mailing list so I can find it?
If it was private can you forward it to the list?

Yours,
Linus Walleij

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Maxime Coquelin]

>> +
>> +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??

It would be cleaner indeed.

>> +
>> +static struct attribute *soc_attrs[] = {
>> +	NULL,
>> +};
>> +
>> +static struct attribute_group soc_attr_group = {
>> +	.attrs = soc_attrs,
>> +};
>
> What is the point of the above two ?

sysfs_create_group() requires attributes.

>> +
>> +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.
>

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Ben Dooks]

On Fri, Mar 11, 2011 at 03:32:51PM +0100, Arnd Bergmann wrote:
> On Friday 11 March 2011, 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 <maxime.coquelin-nonst@stericsson.com>
> 
> I think it's better than the previous patch to create an
> artificial device in /sys/devices/system/socinfo, but I'd
> still prefer the information to be attached to a real device
> that represents the SOC, as I explained in the discussion with
> Linus Walleij.

Why not create it under the cpu directory?
 
> You should definitely add Greg on Cc, as he's maintaining sysfs
> and certainly has an opininion here.
> 
> 	Arnd
> 
> > ---
> >  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 <maxime.coquelin-nonst@stericsson.com>
> > +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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> > + * License terms:  GNU General Public License (GPL), version 2
> > + */
> > +
> > +#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>
> > +
> > +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);
> > +}
> 
> I think this can be simplified by just letting the soc
> register its own show method instead of the get_info function.
> 
> > +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;
> > +}
> > +
> > +static struct attribute *soc_attrs[] = {
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group soc_attr_group = {
> > +	.attrs = soc_attrs,
> > +};
> > +
> > +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;
> > +
> > +	ret = register_sysfs_soc_info(info, num);
> > +	if (ret)
> > +		goto group_exit;
> > +
> > +	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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> > + * License terms:  GNU General Public License (GPL), version 2
> > + */
> > +#ifndef __SYS_SOC_H
> > +#define __SYS_SOC_H
> > +
> > +#include <linux/kobject.h>
> > +
> > +/**
> > + * 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 */
> > -- 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Ben Dooks, ben at fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Greg KH]

On Fri, Mar 11, 2011 at 03:32:51PM +0100, Arnd Bergmann wrote:
> On Friday 11 March 2011, 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 <maxime.coquelin-nonst@stericsson.com>
> 
> I think it's better than the previous patch to create an
> artificial device in /sys/devices/system/socinfo, but I'd
> still prefer the information to be attached to a real device
> that represents the SOC, as I explained in the discussion with
> Linus Walleij.
> 
> You should definitely add Greg on Cc, as he's maintaining sysfs
> and certainly has an opininion here.
> 
> 	Arnd
> 
> > ---
> >  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 <maxime.coquelin-nonst@stericsson.com>
> > +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.

I thought I rejected this the last time it came around?

I still fail to understand why this is needed, please provide more
information about why you feel this is something that the kernel needs.

thanks,

greg k-h

[RFC PATCHv2 2/2] ux500: Export U8500 SoC info through sysfs

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:20 Fri 11 Mar     , Linus Walleij wrote:
> On Fri, Mar 11, 2011 at 3:11 PM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 13:35 Fri 11 Mar ? ? , Maxime Coquelin wrote:
> >> ST-Ericsson's U8500 implementation.
> >> Register sysfs SoC interface, and exports:
> >> ? ? ? - Machine name
> >> ? ? ? - Family name
> >> ? ? ? - SoC ID
> >> ? ? ? - Silicon process
> >> ? ? ? - Silicon revision number.
> > I propose some weeks ago exactly the same kind of patch
> 
> Could you tell which Subject: and mailing list so I can find it?
> If it was private can you forward it to the list?
 892   F Dec14 To linux-kernel [PATCH] base: add sysfs socs info

Best Regards,
J.

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Eduardo Valentin]

On Fri, Mar 11, 2011 at 04:40:05PM +0100, ext Maxime Coquelin wrote:
> 
> 
> 
> >>+
> >>+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??
> 
> It would be cleaner indeed.
> 
> >>+
> >>+static struct attribute *soc_attrs[] = {
> >>+	NULL,
> >>+};
> >>+
> >>+static struct attribute_group soc_attr_group = {
> >>+	.attrs = soc_attrs,
> >>+};
> >
> >What is the point of the above two ?
> 
> sysfs_create_group() requires attributes.

Indeed, it does. but you call it with an empty set of attributes.
So, what's the point?

> 
> >>+
> >>+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.
> >
> 

-- 
Eduardo Valentin

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Eduardo Valentin]

On Fri, Mar 11, 2011 at 03:40:17PM +0000, ext Ben Dooks wrote:
> On Fri, Mar 11, 2011 at 03:32:51PM +0100, Arnd Bergmann wrote:
> > On Friday 11 March 2011, 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 <maxime.coquelin-nonst@stericsson.com>
> > 
> > I think it's better than the previous patch to create an
> > artificial device in /sys/devices/system/socinfo, but I'd
> > still prefer the information to be attached to a real device
> > that represents the SOC, as I explained in the discussion with
> > Linus Walleij.
> 
> Why not create it under the cpu directory?

I think it is mainly because people have strong opinion against
merging/confusing soc data with cpu data.

>  
> > You should definitely add Greg on Cc, as he's maintaining sysfs
> > and certainly has an opininion here.
> > 
> > 	Arnd
> > 
> > > ---
> > >  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 <maxime.coquelin-nonst@stericsson.com>
> > > +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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> > > + * License terms:  GNU General Public License (GPL), version 2
> > > + */
> > > +
> > > +#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>
> > > +
> > > +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);
> > > +}
> > 
> > I think this can be simplified by just letting the soc
> > register its own show method instead of the get_info function.
> > 
> > > +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;
> > > +}
> > > +
> > > +static struct attribute *soc_attrs[] = {
> > > +	NULL,
> > > +};
> > > +
> > > +static struct attribute_group soc_attr_group = {
> > > +	.attrs = soc_attrs,
> > > +};
> > > +
> > > +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;
> > > +
> > > +	ret = register_sysfs_soc_info(info, num);
> > > +	if (ret)
> > > +		goto group_exit;
> > > +
> > > +	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 <maxime.coquelin-nonst@stericsson.com> for ST-Ericsson.
> > > + * License terms:  GNU General Public License (GPL), version 2
> > > + */
> > > +#ifndef __SYS_SOC_H
> > > +#define __SYS_SOC_H
> > > +
> > > +#include <linux/kobject.h>
> > > +
> > > +/**
> > > + * 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 */
> > > -- 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> -- 
> Ben Dooks, ben at fluff.org, http://www.fluff.org/ben/
> 
> Large Hadron Colada: A large Pina Colada that makes the universe disappear.

-- 
Eduardo Valentin

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Eduardo Valentin]

Hello,

On Fri, Mar 11, 2011 at 07:52:57AM -0800, ext Greg KH wrote:
> On Fri, Mar 11, 2011 at 03:32:51PM +0100, Arnd Bergmann wrote:
> > On Friday 11 March 2011, 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 <maxime.coquelin-nonst@stericsson.com>
> > 
> > I think it's better than the previous patch to create an
> > artificial device in /sys/devices/system/socinfo, but I'd
> > still prefer the information to be attached to a real device
> > that represents the SOC, as I explained in the discussion with
> > Linus Walleij.
> > 
> > You should definitely add Greg on Cc, as he's maintaining sysfs
> > and certainly has an opininion here.
> > 
> > 	Arnd
> > 
> > > ---
> > >  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 <maxime.coquelin-nonst@stericsson.com>
> > > +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.
> 
> I thought I rejected this the last time it came around?
> 

Greg,

Do you mind sharing the link of the thread where you have reject this?

> I still fail to understand why this is needed, please provide more
> information about why you feel this is something that the kernel needs.

In case of OMAP, there are few ids that we would expose to user space,
like serial, revision and die id to quote some from top of my mind.
I must emphasize  that these are not data from CPU at all, they belong
to SoC itself.

This info can be useful to track buggy silicon misbehaving on products
which are already on the market. For instance, one possible usage would
be to collect from products which arrives on customer care centers.

That's obviously the view I have from OMAP perspective.

But looks like if we had some common way to expose this data, not only
OMAP socs would benefit of it, as you can see interest from other soc vendors
in this thread (also on V1 of this patch set or on my original patch set over /proc)


> 
> thanks,
> 
> greg k-h

-- 
Eduardo Valentin

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Greg KH]

On Fri, Mar 11, 2011 at 07:58:16PM +0200, Eduardo Valentin wrote:
> Hello,
> 
> On Fri, Mar 11, 2011 at 07:52:57AM -0800, ext Greg KH wrote:
> > On Fri, Mar 11, 2011 at 03:32:51PM +0100, Arnd Bergmann wrote:
> > > On Friday 11 March 2011, 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 <maxime.coquelin-nonst@stericsson.com>
> > > 
> > > I think it's better than the previous patch to create an
> > > artificial device in /sys/devices/system/socinfo, but I'd
> > > still prefer the information to be attached to a real device
> > > that represents the SOC, as I explained in the discussion with
> > > Linus Walleij.
> > > 
> > > You should definitely add Greg on Cc, as he's maintaining sysfs
> > > and certainly has an opininion here.
> > > 
> > > 	Arnd
> > > 
> > > > ---
> > > >  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 <maxime.coquelin-nonst@stericsson.com>
> > > > +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.
> > 
> > I thought I rejected this the last time it came around?
> > 
> 
> Greg,
> 
> Do you mind sharing the link of the thread where you have reject this?

I don't remember it, you can search as well as I can :)

> > I still fail to understand why this is needed, please provide more
> > information about why you feel this is something that the kernel needs.
> 
> In case of OMAP, there are few ids that we would expose to user space,
> like serial, revision and die id to quote some from top of my mind.
> I must emphasize  that these are not data from CPU at all, they belong
> to SoC itself.

I understand, but note that there could be multiple SoC devices, right?
How will this code support that?

Make this a "real" device not /sys/socinfo please.  It can be a platform
device that export the needed information, that way you can have
multiple ones.

thanks,

greg k-h

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Arnd Bergmann]

On Friday 11 March 2011 20:33:30 Greg KH wrote:
> Make this a "real" device not /sys/socinfo please.  It can be a platform
> device that export the needed information, that way you can have
> multiple ones.

Note that the version 1 of this patch had a device, and I argued against
that patch on the basis that anything under /sys/devices/ should
reflect an actual part of the hardware, which socinfo by itself
does not.

I believe the best way to represent this really is to have
a device (platform or other, I don't care) that:

* represents the SOC in its entirety
* has the subdevices that are part of the SOC as direct
  or indirect children
* can be identified easily as a SOC (through one of
  its name, its place in the hierarchy or its bus_type)
* has the standard device attributes proposed in Maxime's
  patch.

	Arnd

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Greg KH]

On Fri, Mar 11, 2011 at 10:42:43PM +0100, Arnd Bergmann wrote:
> On Friday 11 March 2011 20:33:30 Greg KH wrote:
> > Make this a "real" device not /sys/socinfo please.  It can be a platform
> > device that export the needed information, that way you can have
> > multiple ones.
> 
> Note that the version 1 of this patch had a device, and I argued against
> that patch on the basis that anything under /sys/devices/ should
> reflect an actual part of the hardware, which socinfo by itself
> does not.

Why is the overall SoC not a device?  cpus are (well, they will be in a
few kernel versions in the future), so what makes the other bits somehow
"special"?

> I believe the best way to represent this really is to have
> a device (platform or other, I don't care) that:
> 
> * represents the SOC in its entirety
> * has the subdevices that are part of the SOC as direct
>   or indirect children
> * can be identified easily as a SOC (through one of
>   its name, its place in the hierarchy or its bus_type)
> * has the standard device attributes proposed in Maxime's
>   patch.

That's fine, as it would be a real "struct device" :)

thanks,

greg k-h

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Arnd Bergmann]

On Friday 11 March 2011 23:03:33 Greg KH wrote:
> On Fri, Mar 11, 2011 at 10:42:43PM +0100, Arnd Bergmann wrote:
> > On Friday 11 March 2011 20:33:30 Greg KH wrote:
> > > Make this a "real" device not /sys/socinfo please.  It can be a platform
> > > device that export the needed information, that way you can have
> > > multiple ones.
> > 
> > Note that the version 1 of this patch had a device, and I argued against
> > that patch on the basis that anything under /sys/devices/ should
> > reflect an actual part of the hardware, which socinfo by itself
> > does not.
> 
> Why is the overall SoC not a device?  cpus are (well, they will be in a
> few kernel versions in the future), so what makes the other bits somehow
> "special"?

The suggestion was to have a single disconnected device stuck 
in /sys/devices/system/socinfo, and only have it there to
contain device attributes that can be collected from random
places in the system, such hardcoded board specific data,
or read from registers that belong to another device.

A real device IMHO would be one that has specific hardware
properties, such as its own set of device registers or other
devices that are attached to it and that are represented
as children of this device.

	Arnd

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Lee Jones]

On 11/03/11 22:13, Arnd Bergmann wrote:
> On Friday 11 March 2011 23:03:33 Greg KH wrote:
>> On Fri, Mar 11, 2011 at 10:42:43PM +0100, Arnd Bergmann wrote:
>>> On Friday 11 March 2011 20:33:30 Greg KH wrote:
>>>> Make this a "real" device not /sys/socinfo please.  It can be a platform
>>>> device that export the needed information, that way you can have
>>>> multiple ones.
>>>
>>> Note that the version 1 of this patch had a device, and I argued against
>>> that patch on the basis that anything under /sys/devices/ should
>>> reflect an actual part of the hardware, which socinfo by itself
>>> does not.
>>
>> Why is the overall SoC not a device?  cpus are (well, they will be in a
>> few kernel versions in the future), so what makes the other bits somehow
>> "special"?
> 
> The suggestion was to have a single disconnected device stuck 
> in /sys/devices/system/socinfo, and only have it there to
> contain device attributes that can be collected from random
> places in the system, such hardcoded board specific data,
> or read from registers that belong to another device.
> 
> A real device IMHO would be one that has specific hardware
> properties, such as its own set of device registers or other
> devices that are attached to it and that are represented
> as children of this device.

Unfortunately, Maxime is extremely busy at the moment, so I have taken
over to finally get this thing upstream. Just for clarification what's
the _final_ verdict on the location of this entry? Something that we can
all agree on.

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Arnd Bergmann]

On Thursday 07 April 2011, Lee Jones wrote:
> On 11/03/11 22:13, Arnd Bergmann wrote:
> > On Friday 11 March 2011 23:03:33 Greg KH wrote:
> >> On Fri, Mar 11, 2011 at 10:42:43PM +0100, Arnd Bergmann wrote:
> >>> On Friday 11 March 2011 20:33:30 Greg KH wrote:
> >>>> Make this a "real" device not /sys/socinfo please.  It can be a platform
> >>>> device that export the needed information, that way you can have
> >>>> multiple ones.
> >>>
> >>> Note that the version 1 of this patch had a device, and I argued against
> >>> that patch on the basis that anything under /sys/devices/ should
> >>> reflect an actual part of the hardware, which socinfo by itself
> >>> does not.
> >>
> >> Why is the overall SoC not a device?  cpus are (well, they will be in a
> >> few kernel versions in the future), so what makes the other bits somehow
> >> "special"?
> > 
> > The suggestion was to have a single disconnected device stuck 
> > in /sys/devices/system/socinfo, and only have it there to
> > contain device attributes that can be collected from random
> > places in the system, such hardcoded board specific data,
> > or read from registers that belong to another device.
> > 
> > A real device IMHO would be one that has specific hardware
> > properties, such as its own set of device registers or other
> > devices that are attached to it and that are represented
> > as children of this device.
> 
> Unfortunately, Maxime is extremely busy at the moment, so I have taken
> over to finally get this thing upstream. Just for clarification what's
> the final verdict on the location of this entry? Something that we can
> all agree on.

I'll try to capture the opinions from the other people as well,
hopefully this represents a consensus. Please correct me otherwise.

I believe the uncontroversial parts are:

* Make it not an entry but a device with certain properties.
* Make the naming so that you can have more than one of them.
* Put all devices on an SoC that uses this scheme underneath that
  device by setting their parents to the SoC device.

For the location of the device, I have not seen a clear consensus yet,
but I'm fine with eihter of these:

/sys/devices/soc/${NAME}/
/sys/devices/platform/soc${NUMBER}/

For the implementation, I'd suggest adding an soc_device_register()
function that gets passed the necessary data and returns the struct device
that can then be used as the parent in all platform_device_register calls.

	Arnd

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Ryan Mallon]

On 04/08/2011 09:29 AM, Arnd Bergmann wrote:
> On Thursday 07 April 2011, Lee Jones wrote:
>> On 11/03/11 22:13, Arnd Bergmann wrote:
>>> On Friday 11 March 2011 23:03:33 Greg KH wrote:
>>>> On Fri, Mar 11, 2011 at 10:42:43PM +0100, Arnd Bergmann wrote:
>>>>> On Friday 11 March 2011 20:33:30 Greg KH wrote:
>>>>>> Make this a "real" device not /sys/socinfo please.  It can be a platform
>>>>>> device that export the needed information, that way you can have
>>>>>> multiple ones.
>>>>>
>>>>> Note that the version 1 of this patch had a device, and I argued against
>>>>> that patch on the basis that anything under /sys/devices/ should
>>>>> reflect an actual part of the hardware, which socinfo by itself
>>>>> does not.
>>>>
>>>> Why is the overall SoC not a device?  cpus are (well, they will be in a
>>>> few kernel versions in the future), so what makes the other bits somehow
>>>> "special"?
>>>
>>> The suggestion was to have a single disconnected device stuck 
>>> in /sys/devices/system/socinfo, and only have it there to
>>> contain device attributes that can be collected from random
>>> places in the system, such hardcoded board specific data,
>>> or read from registers that belong to another device.
>>>
>>> A real device IMHO would be one that has specific hardware
>>> properties, such as its own set of device registers or other
>>> devices that are attached to it and that are represented
>>> as children of this device.
>>
>> Unfortunately, Maxime is extremely busy at the moment, so I have taken
>> over to finally get this thing upstream. Just for clarification what's
>> the final verdict on the location of this entry? Something that we can
>> all agree on.
> 
> I'll try to capture the opinions from the other people as well,
> hopefully this represents a consensus. Please correct me otherwise.
> 
> I believe the uncontroversial parts are:
> 
> * Make it not an entry but a device with certain properties.
> * Make the naming so that you can have more than one of them.
> * Put all devices on an SoC that uses this scheme underneath that
>   device by setting their parents to the SoC device.
> 
> For the location of the device, I have not seen a clear consensus yet,
> but I'm fine with eihter of these:
> 
> /sys/devices/soc/${NAME}/
> /sys/devices/platform/soc${NUMBER}/

I prefer the second format here since the path is always the same which
makes it easier to write parsing tools. The name should be an entry in
the directory rather than the name of the directory itself.

Probably SoC number should match CPU number for SMP machines right?
Possibly the SoC directory should even contain a symlink to the relevant
CPU sysfs directory? If CPU's are set to become devices under sysfs then
the SoC should also probably be the parent of the CPU?

> For the implementation, I'd suggest adding an soc_device_register()
> function that gets passed the necessary data and returns the struct device
> that can then be used as the parent in all platform_device_register calls.

Agreed.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Nicolas Pitre]

On Fri, 8 Apr 2011, Ryan Mallon wrote:

> On 04/08/2011 09:29 AM, Arnd Bergmann wrote:
> > I'll try to capture the opinions from the other people as well,
> > hopefully this represents a consensus. Please correct me otherwise.
> > 
> > I believe the uncontroversial parts are:
> > 
> > * Make it not an entry but a device with certain properties.
> > * Make the naming so that you can have more than one of them.
> > * Put all devices on an SoC that uses this scheme underneath that
> >   device by setting their parents to the SoC device.
> > 
> > For the location of the device, I have not seen a clear consensus yet,
> > but I'm fine with eihter of these:
> > 
> > /sys/devices/soc/${NAME}/
> > /sys/devices/platform/soc${NUMBER}/
> 
> I prefer the second format here since the path is always the same which
> makes it easier to write parsing tools. The name should be an entry in
> the directory rather than the name of the directory itself.
> 
> Probably SoC number should match CPU number for SMP machines right?

Not really.  SOC means System On Chip.  you usually have only one SOC on 
a board, and within that SOC you may have one or more CPUs.  SMP 
machines are still likely to have only one SOC.


Nicolas

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Ryan Mallon]

On 04/08/2011 10:01 AM, Nicolas Pitre wrote:
> On Fri, 8 Apr 2011, Ryan Mallon wrote:
> 
>> On 04/08/2011 09:29 AM, Arnd Bergmann wrote:
>>> I'll try to capture the opinions from the other people as well,
>>> hopefully this represents a consensus. Please correct me otherwise.
>>>
>>> I believe the uncontroversial parts are:
>>>
>>> * Make it not an entry but a device with certain properties.
>>> * Make the naming so that you can have more than one of them.
>>> * Put all devices on an SoC that uses this scheme underneath that
>>>   device by setting their parents to the SoC device.
>>>
>>> For the location of the device, I have not seen a clear consensus yet,
>>> but I'm fine with eihter of these:
>>>
>>> /sys/devices/soc/${NAME}/
>>> /sys/devices/platform/soc${NUMBER}/
>>
>> I prefer the second format here since the path is always the same which
>> makes it easier to write parsing tools. The name should be an entry in
>> the directory rather than the name of the directory itself.
>>
>> Probably SoC number should match CPU number for SMP machines right?
> 
> Not really.  SOC means System On Chip.  you usually have only one SOC on 
> a board, and within that SOC you may have one or more CPUs.  SMP 
> machines are still likely to have only one SOC.

Ok. What is the situation where we have more than one SoC?

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Nicolas Pitre]

On Fri, 8 Apr 2011, Ryan Mallon wrote:

> On 04/08/2011 10:01 AM, Nicolas Pitre wrote:
> > On Fri, 8 Apr 2011, Ryan Mallon wrote:
> > 
> >> On 04/08/2011 09:29 AM, Arnd Bergmann wrote:
> >>> I'll try to capture the opinions from the other people as well,
> >>> hopefully this represents a consensus. Please correct me otherwise.
> >>>
> >>> I believe the uncontroversial parts are:
> >>>
> >>> * Make it not an entry but a device with certain properties.
> >>> * Make the naming so that you can have more than one of them.
> >>> * Put all devices on an SoC that uses this scheme underneath that
> >>>   device by setting their parents to the SoC device.
> >>>
> >>> For the location of the device, I have not seen a clear consensus yet,
> >>> but I'm fine with eihter of these:
> >>>
> >>> /sys/devices/soc/${NAME}/
> >>> /sys/devices/platform/soc${NUMBER}/
> >>
> >> I prefer the second format here since the path is always the same which
> >> makes it easier to write parsing tools. The name should be an entry in
> >> the directory rather than the name of the directory itself.
> >>
> >> Probably SoC number should match CPU number for SMP machines right?
> > 
> > Not really.  SOC means System On Chip.  you usually have only one SOC on 
> > a board, and within that SOC you may have one or more CPUs.  SMP 
> > machines are still likely to have only one SOC.
> 
> Ok. What is the situation where we have more than one SoC?

That would mean two separate instances of Linux, just like two systems.  
SMP across multiple SOCs doesn't make much sense, and I don't think they 
are likely to be designed for that ability either.


Nicolas

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Ryan Mallon]

On 04/08/2011 10:45 AM, Nicolas Pitre wrote:
> On Fri, 8 Apr 2011, Ryan Mallon wrote:
> 
>> On 04/08/2011 10:01 AM, Nicolas Pitre wrote:
>>> On Fri, 8 Apr 2011, Ryan Mallon wrote:
>>>
>>>> On 04/08/2011 09:29 AM, Arnd Bergmann wrote:
>>>>> I'll try to capture the opinions from the other people as well,
>>>>> hopefully this represents a consensus. Please correct me otherwise.
>>>>>
>>>>> I believe the uncontroversial parts are:
>>>>>
>>>>> * Make it not an entry but a device with certain properties.
>>>>> * Make the naming so that you can have more than one of them.
>>>>> * Put all devices on an SoC that uses this scheme underneath that
>>>>>   device by setting their parents to the SoC device.
>>>>>
>>>>> For the location of the device, I have not seen a clear consensus yet,
>>>>> but I'm fine with eihter of these:
>>>>>
>>>>> /sys/devices/soc/${NAME}/
>>>>> /sys/devices/platform/soc${NUMBER}/
>>>>
>>>> I prefer the second format here since the path is always the same which
>>>> makes it easier to write parsing tools. The name should be an entry in
>>>> the directory rather than the name of the directory itself.
>>>>
>>>> Probably SoC number should match CPU number for SMP machines right?
>>>
>>> Not really.  SOC means System On Chip.  you usually have only one SOC on 
>>> a board, and within that SOC you may have one or more CPUs.  SMP 
>>> machines are still likely to have only one SOC.
>>
>> Ok. What is the situation where we have more than one SoC?
> 
> That would mean two separate instances of Linux, just like two systems.  
> SMP across multiple SOCs doesn't make much sense, and I don't think they 
> are likely to be designed for that ability either.

So we probably don't need the ability to have multiple SoC directories
under sysfs then?

For the other part of the question, do you think it makes sense for the
CPU's to be child devices of the SoC (and for the CPU devices to be
potentially symlinked from the SoC directory)?

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Arnd Bergmann]

On Friday 08 April 2011, Ryan Mallon wrote:
>>>>> /sys/devices/soc/${NAME}/
>>>>> /sys/devices/platform/soc${NUMBER}/
>>>>
>>>> I prefer the second format here since the path is always the same which
>>>> makes it easier to write parsing tools. The name should be an entry in
>>>> the directory rather than the name of the directory itself.

In the first case, we would ensure that every directory under /sys/devices/soc
is an SOC device, so you would not need to parse the name at all, which is
even simpler. I'm open to other arguments either way, but I think this
one is not particularly important.

> > That would mean two separate instances of Linux, just like two systems.  
> > SMP across multiple SOCs doesn't make much sense, and I don't think they 
> > are likely to be designed for that ability either.
> 
> So we probably don't need the ability to have multiple SoC directories
> under sysfs then?

There are systems that have multiple ones, they are just not as common.

> For the other part of the question, do you think it makes sense for the
> CPU's to be child devices of the SoC (and for the CPU devices to be
> potentially symlinked from the SoC directory)?

No, the CPUs are children of /sys/devices/system, and we should not change
that. Symlinks sound like a good idea though.

	Arnd

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Greg KH]

On Fri, Apr 08, 2011 at 01:19:10AM +0200, Arnd Bergmann wrote:
> On Friday 08 April 2011, Ryan Mallon wrote:
> >>>>> /sys/devices/soc/${NAME}/
> >>>>> /sys/devices/platform/soc${NUMBER}/
> >>>>
> >>>> I prefer the second format here since the path is always the same which
> >>>> makes it easier to write parsing tools. The name should be an entry in
> >>>> the directory rather than the name of the directory itself.
> 
> In the first case, we would ensure that every directory under /sys/devices/soc
> is an SOC device, so you would not need to parse the name at all, which is
> even simpler. I'm open to other arguments either way, but I think this
> one is not particularly important.

I think your proposal is best.

> > > That would mean two separate instances of Linux, just like two systems.  
> > > SMP across multiple SOCs doesn't make much sense, and I don't think they 
> > > are likely to be designed for that ability either.
> > 
> > So we probably don't need the ability to have multiple SoC directories
> > under sysfs then?
> 
> There are systems that have multiple ones, they are just not as common.
> 
> > For the other part of the question, do you think it makes sense for the
> > CPU's to be child devices of the SoC (and for the CPU devices to be
> > potentially symlinked from the SoC directory)?
> 
> No, the CPUs are children of /sys/devices/system, and we should not change
> that. Symlinks sound like a good idea though.

Symlinks are a requirement as multiple cpus can be attached to a single
SoC.

What about multiple cpus that are attached to multiple SoCs?  Why even
try to describe this relationship, what would userspace get out of this
information?

thanks,

greg k-h

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Arnd Bergmann]

On Friday 08 April 2011, Greg KH wrote:
> Symlinks are a requirement as multiple cpus can be attached to a single
> SoC.
> 
> What about multiple cpus that are attached to multiple SoCs?  Why even
> try to describe this relationship, what would userspace get out of this
> information?

The only one I can think of is node affinity. I've worked with a system
(IBM QS2x Cell blade) that had two SoCs with multiple CPUs each. There was
a significant performance penalty when talking to devices on the remote
SoC. In that case, we used the NUMA node information in /sys/ to deal
with it, but we might not want to use the NUMA infrastructure on systems
that only have RAM on one node.

	Arnd

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Lee Jones]

On 08/04/11 04:35, Arnd Bergmann wrote:
> On Friday 08 April 2011, Greg KH wrote:
>> Symlinks are a requirement as multiple cpus can be attached to a single
>> SoC.
>>
>> What about multiple cpus that are attached to multiple SoCs?  Why even
>> try to describe this relationship, what would userspace get out of this
>> information?
> 
> The only one I can think of is node affinity. I've worked with a system
> (IBM QS2x Cell blade) that had two SoCs with multiple CPUs each. There was
> a significant performance penalty when talking to devices on the remote
> SoC. In that case, we used the NUMA node information in /sys/ to deal
> with it, but we might not want to use the NUMA infrastructure on systems
> that only have RAM on one node.

I'm struggling to see a scenario where we'd have multiple SoCs on a
single device and only one filesystem. ux500 sub-arch code is yet to
facilitate SoC counting functionality. If were are to implement per-SoC
directories would this need to be added?

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Arnd Bergmann]

On Friday 08 April 2011, Lee Jones wrote:
> On 08/04/11 04:35, Arnd Bergmann wrote:
> > On Friday 08 April 2011, Greg KH wrote:
> >> Symlinks are a requirement as multiple cpus can be attached to a single
> >> SoC.
> >>
> >> What about multiple cpus that are attached to multiple SoCs?  Why even
> >> try to describe this relationship, what would userspace get out of this
> >> information?
> > 
> > The only one I can think of is node affinity. I've worked with a system
> > (IBM QS2x Cell blade) that had two SoCs with multiple CPUs each. There was
> > a significant performance penalty when talking to devices on the remote
> > SoC. In that case, we used the NUMA node information in sys to deal
> > with it, but we might not want to use the NUMA infrastructure on systems
> > that only have RAM on one node.
> 
> I'm struggling to see a scenario where we'd have multiple SoCs on a
> single device and only one filesystem. ux500 sub-arch code is yet to
> facilitate SoC counting functionality. If were are to implement per-SoC
> directories would this need to be added?

How can you say we won't need it when I've just given you a specific
example of a widely shipping machine?

It's not important that the kernel internal code supports multiple SoCs as
long as there is no in-tree user for this. It can always be added at a later
point.

What is important is that the user interface accomodates for it. If you
name the device simply /sys/devices/soc/, any user program looking at
that will have to be modified when we have more than one. We cannot
allow that to happen.

	Arnd

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Lee Jones]

On 08/04/11 16:02, Arnd Bergmann wrote:
> On Friday 08 April 2011, Lee Jones wrote:
>> On 08/04/11 04:35, Arnd Bergmann wrote:
>>> On Friday 08 April 2011, Greg KH wrote:
>>>> Symlinks are a requirement as multiple cpus can be attached to a single
>>>> SoC.
>>>>
>>>> What about multiple cpus that are attached to multiple SoCs?  Why even
>>>> try to describe this relationship, what would userspace get out of this
>>>> information?
>>>
>>> The only one I can think of is node affinity. I've worked with a system
>>> (IBM QS2x Cell blade) that had two SoCs with multiple CPUs each. There was
>>> a significant performance penalty when talking to devices on the remote
>>> SoC. In that case, we used the NUMA node information in sys to deal
>>> with it, but we might not want to use the NUMA infrastructure on systems
>>> that only have RAM on one node.
>>
>> I'm struggling to see a scenario where we'd have multiple SoCs on a
>> single device and only one filesystem. ux500 sub-arch code is yet to
>> facilitate SoC counting functionality. If were are to implement per-SoC
>> directories would this need to be added?
> 
> How can you say we won't need it when I've just given you a specific
> example of a widely shipping machine?

I didn't say we won't need it. I'm just finding it difficult to picture
how it would work. I'm more than happy to implement it, but there are
more things to take into consideration, such as naming conventions for
multiple SoCs. Even if they start at '1' and increase sequentially,
there are still questions that need to be answered, such as; How will
the SoCs be ordered? First come first served? Priority order? Or we
could name them by soc_id?

> It's not important that the kernel internal code supports multiple SoCs as
> long as there is no in-tree user for this. It can always be added at a later
> point.
> 
> What is important is that the user interface accomodates for it. If you
> name the device simply /sys/devices/soc/, any user program looking at
> that will have to be modified when we have more than one. We cannot
> allow that to happen.

No problem. I can insure it's complete on first submission.

[RFC PATCHv2 1/2] Export SoC info through sysfs

[Arnd Bergmann]

On Friday 08 April 2011, Lee Jones wrote:
> I didn't say we won't need it. I'm just finding it difficult to picture
> how it would work. I'm more than happy to implement it, but there are
> more things to take into consideration, such as naming conventions for
> multiple SoCs. Even if they start at '1' and increase sequentially,
> there are still questions that need to be answered, such as; How will
> the SoCs be ordered? First come first served? Priority order? Or we
> could name them by soc_id?

Any of these would be fine IMHO. In most cases, the order is fixed,
either through a board file or through the order in the device tree,
so I would just use a sequential number.

	Arnd