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

Groups > linux.kernel > #1226228 > unrolled thread

[PATCH v5 0/4] of: overlay: kobject & sysfs'ation

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 0/4] of: overlay: kobject & sysfs'ation Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-09-16 18:10 +0200
    [PATCH v5 2/4] of: overlay: global sysfs enable attribute Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-09-16 18:20 +0200
    [PATCH v5 3/4] of: overlay: add per overlay sysfs attributes Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-09-16 18:20 +0200
      Re: [PATCH v5 3/4] of: overlay: add per overlay sysfs attributes Rob Herring <robherring2@gmail.com> - 2015-09-16 18:50 +0200
        Re: [PATCH v5 3/4] of: overlay: add per overlay sysfs attributes Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2015-09-16 21:10 +0200

#1226228 — [PATCH v5 0/4] of: overlay: kobject & sysfs'ation

The first patch puts the overlays as objects in the sysfs in
/sys/firmware/devicetree/overlays.

The next adds a master overlay enable switch (that once is set to
disabled can't be re-enabled), while the one after that
introduces a number of default per overlay attributes.

The patchset is against linus's tree as of today.

The last patch updates the ABI docs for the sysfs entries.

Changes since v4:
* Rebased against latest mainline.

Changes since v3:
* Used strtobool instead of kstrtoul
* ABI Documentation includes a pointer to the discussion that
requested the sysfs property.

Changes since v2:
* Removed the unittest patch.
* Split the sysfs attribute patch to a global and a per-overlay
  patch.
* Dropped binary attributes using textual kobj_attributes instead.

Changes since v1:
* Maintainer requested changes.
* Documented the sysfs entries
* Per overlay sysfs attributes.

Pantelis Antoniou (4):
  of: overlay: kobjectify overlay objects
  of: overlay: global sysfs enable attribute
  of: overlay: add per overlay sysfs attributes
  Documentation: ABI: /sys/firmware/devicetree/overlays

 .../ABI/testing/sysfs-firmware-devicetree-overlays |  35 +++++
 drivers/of/base.c                                  |   7 +
 drivers/of/of_private.h                            |   9 ++
 drivers/of/overlay.c                               | 146 ++++++++++++++++++++-
 4 files changed, 195 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays

-- 
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]

#1226233 — [PATCH v5 2/4] of: overlay: global sysfs enable attribute

A throw once master enable switch to protect against any
further overlay applications if the administrator desires so.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/overlay.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index eeb3ec2..62cfb99 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -21,6 +21,7 @@
 #include <linux/err.h>
 #include <linux/idr.h>
 #include <linux/sysfs.h>
+#include <linux/atomic.h>
 
 #include "of_private.h"
 
@@ -55,8 +56,12 @@ struct of_overlay {
 	struct kobject kobj;
 };
 
+/* master enable switch; once set to 0 can't be re-enabled */
+static atomic_t ov_enable = ATOMIC_INIT(1);
+
 static int of_overlay_apply_one(struct of_overlay *ov,
 		struct device_node *target, const struct device_node *overlay);
+static int overlay_removal_is_ok(struct of_overlay *ov);
 
 static int of_overlay_apply_single_property(struct of_overlay *ov,
 		struct device_node *target, struct property *prop)
@@ -339,6 +344,35 @@ void of_overlay_release(struct kobject *kobj)
 	kfree(ov);
 }
 
+static ssize_t enable_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", atomic_read(&ov_enable));
+}
+
+static ssize_t enable_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int ret;
+	bool new_enable;
+
+	ret = strtobool(buf, &new_enable);
+	if (ret != 0)
+		return ret;
+	/* if we've disabled it, no going back */
+	if (atomic_read(&ov_enable) == 0)
+		return -EPERM;
+	atomic_set(&ov_enable, (int)new_enable);
+	return count;
+}
+
+static struct kobj_attribute enable_attr = __ATTR_RW(enable);
+
+static const struct attribute *overlay_global_attrs[] = {
+	&enable_attr.attr,
+	NULL
+};
+
 static struct kobj_type of_overlay_ktype = {
 	.release = of_overlay_release,
 };
@@ -360,6 +394,10 @@ int of_overlay_create(struct device_node *tree)
 	struct of_overlay *ov;
 	int err, id;
 
+	/* administratively disabled */
+	if (!atomic_read(&ov_enable))
+		return -EPERM;
+
 	/* allocate the overlay structure */
 	ov = kzalloc(sizeof(*ov), GFP_KERNEL);
 	if (ov == NULL)
@@ -596,5 +634,8 @@ int of_overlay_init(void)
 	if (!ov_kset)
 		return -ENOMEM;
 
-	return 0;
+	rc = sysfs_create_files(&ov_kset->kobj, overlay_global_attrs);
+	WARN(rc, "%s: error adding global attributes\n", __func__);
+
+	return rc;
 }
-- 
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] | [prev] | [next] | [standalone]

#1226234 — [PATCH v5 3/4] of: overlay: add per overlay sysfs attributes

The two default overlay attributes are:

* A targets sysfs attribute listing the targets of the installed
overlay. The targets list the path on the kernel's device tree
where each overlay fragment is applied to

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

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/overlay.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 62cfb99..cff3c05 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -373,8 +373,61 @@ 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 ssize_t targets_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct of_overlay *ov = kobj_to_overlay(kobj);
+	struct of_overlay_info *ovinfo;
+	char *s, *e;
+	ssize_t ret;
+	int i, len;
+
+	s = buf;
+	e = buf + PAGE_SIZE;
+
+	mutex_lock(&of_mutex);
+
+	/* targets */
+	for (i = 0; i < ov->count; i++) {
+		ovinfo = &ov->ovinfo_tab[i];
+
+		len = snprintf(s, e - s, "%s\n",
+				of_node_full_name(ovinfo->target));
+		if (len == 0) {
+			ret = -ENOSPC;
+			goto err;
+		}
+		s += len;
+	}
+
+	/* the buffer is zero terminated */
+	ret = s - buf;
+err:
+	mutex_unlock(&of_mutex);
+	return ret;
+}
+
+static struct kobj_attribute can_remove_attr = __ATTR_RO(can_remove);
+static struct kobj_attribute targets_attr = __ATTR_RO(targets);
+
+static struct attribute *overlay_attrs[] = {
+	&can_remove_attr.attr,
+	&targets_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;
-- 
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] | [prev] | [next] | [standalone]

#1226283 — Re: [PATCH v5 3/4] of: overlay: add per overlay sysfs attributes

On Wed, Sep 16, 2015 at 11:09 AM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> The two default overlay attributes are:
>
> * A targets sysfs attribute listing the targets of the installed
> overlay. The targets list the path on the kernel's device tree
> where each overlay fragment is applied to
>
> * A per overlay can_remove sysfs attribute that reports whether
> the overlay can be removed or not due to another overlapping overlay.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> ---

How about an answer to my v4 questions?

Rob

>  drivers/of/overlay.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 62cfb99..cff3c05 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -373,8 +373,61 @@ 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 ssize_t targets_show(struct kobject *kobj,
> +               struct kobj_attribute *attr, char *buf)
> +{
> +       struct of_overlay *ov = kobj_to_overlay(kobj);
> +       struct of_overlay_info *ovinfo;
> +       char *s, *e;
> +       ssize_t ret;
> +       int i, len;
> +
> +       s = buf;
> +       e = buf + PAGE_SIZE;
> +
> +       mutex_lock(&of_mutex);
> +
> +       /* targets */
> +       for (i = 0; i < ov->count; i++) {
> +               ovinfo = &ov->ovinfo_tab[i];
> +
> +               len = snprintf(s, e - s, "%s\n",
> +                               of_node_full_name(ovinfo->target));
> +               if (len == 0) {
> +                       ret = -ENOSPC;
> +                       goto err;
> +               }
> +               s += len;
> +       }
> +
> +       /* the buffer is zero terminated */
> +       ret = s - buf;
> +err:
> +       mutex_unlock(&of_mutex);
> +       return ret;
> +}
> +
> +static struct kobj_attribute can_remove_attr = __ATTR_RO(can_remove);
> +static struct kobj_attribute targets_attr = __ATTR_RO(targets);
> +
> +static struct attribute *overlay_attrs[] = {
> +       &can_remove_attr.attr,
> +       &targets_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;
> --
> 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] | [prev] | [next] | [standalone]

#1226399 — Re: [PATCH v5 3/4] of: overlay: add per overlay sysfs attributes

Hi Rob,

> On Sep 16, 2015, at 19:45 , Rob Herring <robherring2@gmail.com> wrote:
> 
> On Wed, Sep 16, 2015 at 11:09 AM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>> The two default overlay attributes are:
>> 
>> * A targets sysfs attribute listing the targets of the installed
>> overlay. The targets list the path on the kernel's device tree
>> where each overlay fragment is applied to
>> 
>> * A per overlay can_remove sysfs attribute that reports whether
>> the overlay can be removed or not due to another overlapping overlay.
>> 
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>> ---
> 
> How about an answer to my v4 questions?
> 

Q. "What will a user do with this information? Can this just be debugfs?
Don't we already have the targets in the overlay itself? If we are
going to provide some overlay info, shouldn't we provide all of it
(i.e. the FDT). For example, what do we do on a kexec?”

A. The user can easily tell whether an overlay is removable using the can_remove
attribute. This can be used by higher layer to inform users whether it’s safe to
remove an overlay or not (perhaps light up a BUSY led)

The targets attribute can let the user figure out what devices (as evident by the
nodes they point to) are affected by the overlay. This was actually used by me
to figure out the TI LCDC DRM driver use of overlays by looking at the targets
pointing there.

Debugfs is a bad fit since these are per-overlay attributes that a user-space application
can use to present some kind of meaningful information to the user.

Providing the FDT of the overlay is only possible when the overlay origin is indeed a
DT blob, which is not always the case. The overlay API only uses unflattened trees.
The DT blob can be provided only by the caller, which has access to the blob.
To hang the FDT blob to overlay kobj we need to extend the overlay API to 
return the kobj of the overlay (which I intent to do in the near future).

Kexec is really tricky. I purposefully didn’t try to address it since in the
previous discussion with Grant we couldn’t get to an agreed default. It seems it is
a per-platform/per-policy decision, and we lack the users to tell us which is the most
prominent.
We have two ways to handle it: a) Leave the kernel tree as it is, flatten it and
kexec with it. That would remove the overlays but the state of the the devices would persist.
b) Remove the overlays in the proper sequence and kexec with the clean kernel tree. That
would necessitate the re-application of the overlays to get to the same device tree state.


> Rob
> 

Regards

— Pantelis

>> drivers/of/overlay.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>> 
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 62cfb99..cff3c05 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -373,8 +373,61 @@ 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 ssize_t targets_show(struct kobject *kobj,
>> +               struct kobj_attribute *attr, char *buf)
>> +{
>> +       struct of_overlay *ov = kobj_to_overlay(kobj);
>> +       struct of_overlay_info *ovinfo;
>> +       char *s, *e;
>> +       ssize_t ret;
>> +       int i, len;
>> +
>> +       s = buf;
>> +       e = buf + PAGE_SIZE;
>> +
>> +       mutex_lock(&of_mutex);
>> +
>> +       /* targets */
>> +       for (i = 0; i < ov->count; i++) {
>> +               ovinfo = &ov->ovinfo_tab[i];
>> +
>> +               len = snprintf(s, e - s, "%s\n",
>> +                               of_node_full_name(ovinfo->target));
>> +               if (len == 0) {
>> +                       ret = -ENOSPC;
>> +                       goto err;
>> +               }
>> +               s += len;
>> +       }
>> +
>> +       /* the buffer is zero terminated */
>> +       ret = s - buf;
>> +err:
>> +       mutex_unlock(&of_mutex);
>> +       return ret;
>> +}
>> +
>> +static struct kobj_attribute can_remove_attr = __ATTR_RO(can_remove);
>> +static struct kobj_attribute targets_attr = __ATTR_RO(targets);
>> +
>> +static struct attribute *overlay_attrs[] = {
>> +       &can_remove_attr.attr,
>> +       &targets_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;
>> --
>> 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] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web