Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes

Hi Greg,

On Oct 21, 2015, at 00:08 , Greg Kroah-Hartman [off-list ref] wrote:

On Tue, Oct 20, 2015 at 10:13:16PM +0300, Pantelis Antoniou wrote:

quoted

* A per overlay can_remove sysfs attribute that reports whether
the overlay can be removed or not due to another overlapping overlay.

* A target sysfs attribute listing the target of each fragment,
in a group named after the name of the fragment.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
drivers/of/overlay.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 99 insertions(+), 4 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 067404e..2d51d9e2 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c

@@ -25,8 +25,23 @@

#include "of_private.h"

+/* fwd. decl */
+struct of_overlay;
+struct of_overlay_info;
+
+/* an attribute for each fragment */
+struct fragment_attribute {
+	struct attribute attr;
+	ssize_t (*show)(struct kobject *kobj, struct fragment_attribute *fattr,
+			char *buf);
+	ssize_t (*store)(struct kobject *kobj, struct fragment_attribute *fattr,
+			 const char *buf, size_t count);
+	struct of_overlay_info *ovinfo;
+};
+
/**
 * struct of_overlay_info - Holds a single overlay info
+ * @info:	info node that contains the target and overlay
 * @target:	target of the overlay operation
 * @overlay:	pointer to the overlay contents node
 *

@@ -34,8 +49,13 @@
 * records.
 */

struct of_overlay_info {
+	struct of_overlay *ov;
+	struct device_node *info;
	struct device_node *target;
	struct device_node *overlay;
+	struct attribute_group attr_group;
+	struct attribute *attrs[2];

Why both 2 attributes _and_ an attribute group?  Why not put the
attributes in the attribute group?

And why just one attribute group?  Why not an array of them like the
rest of the kernel is used to handle?

Because this makes it easier to add all the attributes for all the fragments in a single
sysfs_create_groups() call, once for each overlay, instead of having a call to 
sysfs_create_group() for each fragment of the overlay.

Reusing driver core helpers is good, no?

thanks,

greg k-h

Regards

— Pantelis