On Fri, Mar 11, 2011 at 04:40:05PM +0100, ext Maxime Coquelin wrote:

quoted

quoted

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

quoted

quoted

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

quoted

quoted

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

quoted

+
+	ret = register_sysfs_soc_info(info, num);
+	if (ret)
+		goto group_exit;

But the real thing happens here.

-- 
Eduardo Valentin