Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1251972 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-10-20 21:20 +0200
    Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Rob Herring <robherring2@gmail.com> - 2015-10-20 23:10 +0200
      Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-10-20 23:20 +0200
        Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Rob Herring <robherring2@gmail.com> - 2015-10-21 00:00 +0200
          Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-10-21 21:40 +0200
            Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Rob Herring <robherring2@gmail.com> - 2015-10-22 00:00 +0200
              Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-10-22 18:20 +0200
    Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-20 23:10 +0200
      Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-10-20 23:20 +0200
        Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-21 01:30 +0200

#1251972 — [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes

* 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@konsulko.com>
---
 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];
+	struct fragment_attribute target_attr;
 };
 
 /**
@@ -52,6 +72,7 @@ struct of_overlay {
 	struct list_head node;
 	int count;
 	struct of_overlay_info *ovinfo_tab;
+	const struct attribute_group **attr_groups;
 	struct of_changeset cset;
 	struct kobject kobj;
 };
@@ -245,6 +266,8 @@ static int of_fill_overlay_info(struct of_overlay *ov,
 	if (ovinfo->target == NULL)
 		goto err_fail;
 
+	ovinfo->info = of_node_get(info_node);
+
 	return 0;
 
 err_fail:
@@ -255,6 +278,17 @@ err_fail:
 	return -EINVAL;
 }
 
+static ssize_t target_show(struct kobject *kobj,
+		struct fragment_attribute *fattr, char *buf)
+{
+	struct of_overlay_info *ovinfo = fattr->ovinfo;
+
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+			of_node_full_name(ovinfo->target));
+}
+
+static const struct fragment_attribute target_template_attr = __ATTR_RO(target);
+
 /**
  * of_build_overlay_info() - Build an overlay info array
  * @ov		Overlay to build
@@ -272,7 +306,7 @@ static int of_build_overlay_info(struct of_overlay *ov,
 {
 	struct device_node *node;
 	struct of_overlay_info *ovinfo;
-	int cnt, err;
+	int i, cnt, err;
 
 	/* worst case; every child is a node */
 	cnt = 0;
@@ -293,14 +327,45 @@ static int of_build_overlay_info(struct of_overlay *ov,
 
 	/* if nothing filled, return error */
 	if (cnt == 0) {
-		kfree(ovinfo);
-		return -ENODEV;
+		err = -ENODEV;
+		goto err_free_ovinfo;
 	}
 
 	ov->count = cnt;
 	ov->ovinfo_tab = ovinfo;
 
+	ov->attr_groups = kcalloc(cnt + 1,
+			sizeof(struct attribute_group *), GFP_KERNEL);
+	if (ov->attr_groups == NULL) {
+		err = -ENOMEM;
+		goto err_free_ovinfo;
+	}
+
+	for (i = 0; i < cnt; i++) {
+		ovinfo = &ov->ovinfo_tab[i];
+
+		ov->attr_groups[i] = &ovinfo->attr_group;
+
+		ovinfo->target_attr = target_template_attr;
+		/* make lockdep happy */
+		sysfs_attr_init(&ovinfo->target_attr.attr);
+		ovinfo->target_attr.ovinfo = ovinfo;
+
+		ovinfo->attrs[0] = &ovinfo->target_attr.attr;
+		ovinfo->attrs[1] = NULL;
+
+		/* NOTE: direct reference to the full_name */
+		ovinfo->attr_group.name = kbasename(ovinfo->info->full_name);
+		ovinfo->attr_group.attrs = ovinfo->attrs;
+
+	}
+	ov->attr_groups[i] = NULL;
+
 	return 0;
+
+err_free_ovinfo:
+	kfree(ovinfo);
+	return err;
 }
 
 /**
@@ -317,12 +382,16 @@ static int of_free_overlay_info(struct of_overlay *ov)
 	struct of_overlay_info *ovinfo;
 	int i;
 
+	/* free attribute groups space */
+	kfree(ov->attr_groups);
+
 	/* do it in reverse */
 	for (i = ov->count - 1; i >= 0; i--) {
 		ovinfo = &ov->ovinfo_tab[i];
 
 		of_node_put(ovinfo->target);
 		of_node_put(ovinfo->overlay);
+		of_node_put(ovinfo->info);
 	}
 	kfree(ov->ovinfo_tab);
 
@@ -373,8 +442,25 @@ static const struct attribute *overlay_global_attrs[] = {
 	NULL
 };
 
+static ssize_t can_remove_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct of_overlay *ov = kobj_to_overlay(kobj);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", overlay_removal_is_ok(ov));
+}
+
+static struct kobj_attribute can_remove_attr = __ATTR_RO(can_remove);
+
+static struct attribute *overlay_attrs[] = {
+	&can_remove_attr.attr,
+	NULL
+};
+
 static struct kobj_type of_overlay_ktype = {
 	.release = of_overlay_release,
+	.sysfs_ops = &kobj_sysfs_ops,	/* default kobj sysfs ops */
+	.default_attrs = overlay_attrs,
 };
 
 static struct kset *ov_kset;
@@ -454,13 +540,21 @@ int of_overlay_create(struct device_node *tree)
 		goto err_cancel_overlay;
 	}
 
+	err = sysfs_create_groups(&ov->kobj, ov->attr_groups);
+	if (err != 0) {
+		pr_err("%s: sysfs_create_groups() failed for tree@%s\n",
+				__func__, tree->full_name);
+		goto err_remove_kobj;
+	}
+
 	/* add to the tail of the overlay list */
 	list_add_tail(&ov->node, &ov_list);
 
 	mutex_unlock(&of_mutex);
 
 	return id;
-
+err_remove_kobj:
+	kobject_put(&ov->kobj);
 err_cancel_overlay:
 	of_changeset_revert(&ov->cset);
 err_revert_overlay:
@@ -579,6 +673,7 @@ int of_overlay_destroy(int id)
 
 
 	list_del(&ov->node);
+	sysfs_remove_groups(&ov->kobj, ov->attr_groups);
 	of_changeset_revert(&ov->cset);
 	of_free_overlay_info(ov);
 	idr_remove(&ov_idr, id);
-- 
1.7.12

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1252040

On Tue, Oct 20, 2015 at 2:13 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> * 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@konsulko.com>
> ---
>  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];
> +       struct fragment_attribute target_attr;
>  };
>
>  /**
> @@ -52,6 +72,7 @@ struct of_overlay {
>         struct list_head node;
>         int count;
>         struct of_overlay_info *ovinfo_tab;
> +       const struct attribute_group **attr_groups;
>         struct of_changeset cset;
>         struct kobject kobj;
>  };
> @@ -245,6 +266,8 @@ static int of_fill_overlay_info(struct of_overlay *ov,
>         if (ovinfo->target == NULL)
>                 goto err_fail;
>
> +       ovinfo->info = of_node_get(info_node);
> +
>         return 0;
>
>  err_fail:
> @@ -255,6 +278,17 @@ err_fail:
>         return -EINVAL;
>  }
>
> +static ssize_t target_show(struct kobject *kobj,
> +               struct fragment_attribute *fattr, char *buf)
> +{
> +       struct of_overlay_info *ovinfo = fattr->ovinfo;
> +
> +       return snprintf(buf, PAGE_SIZE, "%s\n",
> +                       of_node_full_name(ovinfo->target));

This can be a link to the node itself, can't it?

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252049

Hi Rob,

> On Oct 21, 2015, at 00:04 , Rob Herring <robherring2@gmail.com> wrote:
> 
> On Tue, Oct 20, 2015 at 2:13 PM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>> * 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@konsulko.com>
>> ---
>> 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];
>> +       struct fragment_attribute target_attr;
>> };
>> 
>> /**
>> @@ -52,6 +72,7 @@ struct of_overlay {
>>        struct list_head node;
>>        int count;
>>        struct of_overlay_info *ovinfo_tab;
>> +       const struct attribute_group **attr_groups;
>>        struct of_changeset cset;
>>        struct kobject kobj;
>> };
>> @@ -245,6 +266,8 @@ static int of_fill_overlay_info(struct of_overlay *ov,
>>        if (ovinfo->target == NULL)
>>                goto err_fail;
>> 
>> +       ovinfo->info = of_node_get(info_node);
>> +
>>        return 0;
>> 
>> err_fail:
>> @@ -255,6 +278,17 @@ err_fail:
>>        return -EINVAL;
>> }
>> 
>> +static ssize_t target_show(struct kobject *kobj,
>> +               struct fragment_attribute *fattr, char *buf)
>> +{
>> +       struct of_overlay_info *ovinfo = fattr->ovinfo;
>> +
>> +       return snprintf(buf, PAGE_SIZE, "%s\n",
>> +                       of_node_full_name(ovinfo->target));
> 
> This can be a link to the node itself, can't it?
> 

Yes. Do you want it like this?

> Rob

Regards

— Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252141

On Tue, Oct 20, 2015 at 4:11 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> Hi Rob,
>
>> On Oct 21, 2015, at 00:04 , Rob Herring <robherring2@gmail.com> wrote:
>>
>> On Tue, Oct 20, 2015 at 2:13 PM, Pantelis Antoniou
>> <pantelis.antoniou@konsulko.com> wrote:
>>> * 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.

[...]

>>> @@ -255,6 +278,17 @@ err_fail:
>>>        return -EINVAL;
>>> }
>>>
>>> +static ssize_t target_show(struct kobject *kobj,
>>> +               struct fragment_attribute *fattr, char *buf)
>>> +{
>>> +       struct of_overlay_info *ovinfo = fattr->ovinfo;
>>> +
>>> +       return snprintf(buf, PAGE_SIZE, "%s\n",
>>> +                       of_node_full_name(ovinfo->target));
>>
>> This can be a link to the node itself, can't it?
>>
>
> Yes. Do you want it like this?

Yes, hence the suggestion. Unless you see some reason why not.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1253151

Hi Rob,

> On Oct 21, 2015, at 00:54 , Rob Herring <robherring2@gmail.com> wrote:
> 
> On Tue, Oct 20, 2015 at 4:11 PM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>> Hi Rob,
>> 
>>> On Oct 21, 2015, at 00:04 , Rob Herring <robherring2@gmail.com> wrote:
>>> 
>>> On Tue, Oct 20, 2015 at 2:13 PM, Pantelis Antoniou
>>> <pantelis.antoniou@konsulko.com> wrote:
>>>> * 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.
> 
> [...]
> 
>>>> @@ -255,6 +278,17 @@ err_fail:
>>>>       return -EINVAL;
>>>> }
>>>> 
>>>> +static ssize_t target_show(struct kobject *kobj,
>>>> +               struct fragment_attribute *fattr, char *buf)
>>>> +{
>>>> +       struct of_overlay_info *ovinfo = fattr->ovinfo;
>>>> +
>>>> +       return snprintf(buf, PAGE_SIZE, "%s\n",
>>>> +                       of_node_full_name(ovinfo->target));
>>> 
>>> This can be a link to the node itself, can't it?
>>> 
>> 
>> Yes. Do you want it like this?
> 
> Yes, hence the suggestion. Unless you see some reason why not.
> 

Nope, can’t be done. The sysfs API only allows linking one kobj to another.
The kobj is the overlay but the target is in the fragment attribute group.

Sorry, I really tried, but can’t be done without hacking in a new link sysfs API. 

> Rob

Regards

— Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1253293

On Wed, Oct 21, 2015 at 2:37 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> Hi Rob,
>
>> On Oct 21, 2015, at 00:54 , Rob Herring <robherring2@gmail.com> wrote:
>>
>> On Tue, Oct 20, 2015 at 4:11 PM, Pantelis Antoniou
>> <pantelis.antoniou@konsulko.com> wrote:
>>> Hi Rob,
>>>
>>>> On Oct 21, 2015, at 00:04 , Rob Herring <robherring2@gmail.com> wrote:
>>>>
>>>> On Tue, Oct 20, 2015 at 2:13 PM, Pantelis Antoniou
>>>> <pantelis.antoniou@konsulko.com> wrote:
>>>>> * 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.
>>
>> [...]
>>
>>>>> @@ -255,6 +278,17 @@ err_fail:
>>>>>       return -EINVAL;
>>>>> }
>>>>>
>>>>> +static ssize_t target_show(struct kobject *kobj,
>>>>> +               struct fragment_attribute *fattr, char *buf)
>>>>> +{
>>>>> +       struct of_overlay_info *ovinfo = fattr->ovinfo;
>>>>> +
>>>>> +       return snprintf(buf, PAGE_SIZE, "%s\n",
>>>>> +                       of_node_full_name(ovinfo->target));
>>>>
>>>> This can be a link to the node itself, can't it?
>>>>
>>>
>>> Yes. Do you want it like this?
>>
>> Yes, hence the suggestion. Unless you see some reason why not.
>>
>
> Nope, can’t be done. The sysfs API only allows linking one kobj to another.
> The kobj is the overlay but the target is in the fragment attribute group.

Can't we make the fragments kobj's as well?

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1253958

Hi Rob,

> On Oct 22, 2015, at 00:52 , Rob Herring <robherring2@gmail.com> wrote:
> 
> On Wed, Oct 21, 2015 at 2:37 PM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>> Hi Rob,
>> 
>>> On Oct 21, 2015, at 00:54 , Rob Herring <robherring2@gmail.com> wrote:
>>> 
>>> On Tue, Oct 20, 2015 at 4:11 PM, Pantelis Antoniou
>>> <pantelis.antoniou@konsulko.com> wrote:
>>>> Hi Rob,
>>>> 
>>>>> On Oct 21, 2015, at 00:04 , Rob Herring <robherring2@gmail.com> wrote:
>>>>> 
>>>>> On Tue, Oct 20, 2015 at 2:13 PM, Pantelis Antoniou
>>>>> <pantelis.antoniou@konsulko.com> wrote:
>>>>>> * 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.
>>> 
>>> [...]
>>> 
>>>>>> @@ -255,6 +278,17 @@ err_fail:
>>>>>>      return -EINVAL;
>>>>>> }
>>>>>> 
>>>>>> +static ssize_t target_show(struct kobject *kobj,
>>>>>> +               struct fragment_attribute *fattr, char *buf)
>>>>>> +{
>>>>>> +       struct of_overlay_info *ovinfo = fattr->ovinfo;
>>>>>> +
>>>>>> +       return snprintf(buf, PAGE_SIZE, "%s\n",
>>>>>> +                       of_node_full_name(ovinfo->target));
>>>>> 
>>>>> This can be a link to the node itself, can't it?
>>>>> 
>>>> 
>>>> Yes. Do you want it like this?
>>> 
>>> Yes, hence the suggestion. Unless you see some reason why not.
>>> 
>> 
>> Nope, can’t be done. The sysfs API only allows linking one kobj to another.
>> The kobj is the overlay but the target is in the fragment attribute group.
> 
> Can't we make the fragments kobj's as well?
> 

We could, but it break the mental model of what a kobj should represent.
An overlay is an object which can be address, a fragment is never directly
exposed.

TBH a link attribute is indeed better than a path attribute, but marginally so.
It’s not worth the trouble IMO.
 
> Rob

Regards

— Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252041

On Tue, Oct 20, 2015 at 10:13:16PM +0300, Pantelis Antoniou wrote:
> * 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@konsulko.com>
> ---
>  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?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252047

Hi Greg,

> On Oct 21, 2015, at 00:08 , Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> On Tue, Oct 20, 2015 at 10:13:16PM +0300, Pantelis Antoniou wrote:
>> * 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@konsulko.com>
>> ---
>> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252366

On Wed, Oct 21, 2015 at 12:15:00AM +0300, Pantelis Antoniou wrote:
> Hi Greg,
> 
> > On Oct 21, 2015, at 00:08 , Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > 
> > On Tue, Oct 20, 2015 at 10:13:16PM +0300, Pantelis Antoniou wrote:
> >> * 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@konsulko.com>
> >> ---
> >> 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?

Yes it is, sorry, I missed how you used these later on, and the
attribute_groups usage there, nice job, sorry for the noise.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web