Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932839AbbDOB2A (ORCPT ); Tue, 14 Apr 2015 21:28:00 -0400 Received: from mail-wg0-f49.google.com ([74.125.82.49]:35324 "EHLO mail-wg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751911AbbDOB1v (ORCPT ); Tue, 14 Apr 2015 21:27:51 -0400 MIME-Version: 1.0 In-Reply-To: <1428434632-13789-4-git-send-email-pantelis.antoniou@konsulko.com> References: <1428434632-13789-1-git-send-email-pantelis.antoniou@konsulko.com> <1428434632-13789-4-git-send-email-pantelis.antoniou@konsulko.com> From: Rob Herring Date: Tue, 14 Apr 2015 20:27:30 -0500 Message-ID: Subject: Re: [PATCH v2 3/4] of: overlay: Add sysfs attributes To: Pantelis Antoniou Cc: Grant Likely , Andrew Morton , Matt Porter , Koen Kooi , Guenter Roeck , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-api@vger.kernel.org" , Pantelis Antoniou Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7883 Lines: 257 On Tue, Apr 7, 2015 at 2:23 PM, Pantelis Antoniou wrote: > Implement a number of sysfs attributes for overlays. > > * A throw once master enable switch to protect against any > further overlay applications if the administrator desires so. This one should be a separate patch. > * A per overlay targets sysfs attribute listing the targets of > the installed overlay. What are targets? "targets lists targets" does not help me. The documentation doesn't help me either. > * 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 > --- > drivers/of/overlay.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 166 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c > index f17f5ef..c54d097 100644 > --- a/drivers/of/overlay.c > +++ b/drivers/of/overlay.c > @@ -21,6 +21,7 @@ > #include > #include > #include > +#include > > #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) > @@ -345,6 +350,144 @@ static struct kobj_type of_overlay_ktype = { > > static struct kset *ov_kset; > > +static ssize_t enable_read(struct file *filp, struct kobject *kobj, > + struct bin_attribute *bin_attr, char *buf, > + loff_t offset, size_t count) > +{ > + char tbuf[3]; > + > + if (offset < 0) > + return -EINVAL; > + > + if (offset >= sizeof(tbuf)) > + return 0; > + > + if (count > sizeof(tbuf) - offset) > + count = sizeof(tbuf) - offset; > + > + /* fill in temp */ > + tbuf[0] = '0' + atomic_read(&ov_enable); > + tbuf[1] = '\n'; > + tbuf[2] = '\0'; > + > + /* copy to buffer */ > + memcpy(buf, tbuf + offset, count); > + > + return count; > +} > + > +static ssize_t enable_write(struct file *filp, struct kobject *kobj, > + struct bin_attribute *bin_attr, char *buf, > + loff_t off, size_t count) > +{ > + unsigned int new_enable; > + > + if (off != 0 || (buf[0] != '0' && buf[0] != '1')) > + return -EINVAL; > + > + new_enable = (unsigned int)(buf[0] - '0'); > + if (new_enable > 1) > + return -EINVAL; > + > + /* NOP for same value */ > + if (new_enable == atomic_read(&ov_enable)) > + return count; > + > + /* if we've disabled it, no going back */ > + if (atomic_read(&ov_enable) == 0) > + return -EPERM; > + > + atomic_set(&ov_enable, new_enable); > + return count; > +} > + > +/* just a single char + '\n' + '\0' */ > +static BIN_ATTR_RW(enable, 3); Why are you using bin attribute? You are complicating the implementation needlessly. > + > +static ssize_t targets_read(struct file *filp, struct kobject *kobj, > + struct bin_attribute *bin_attr, char *buf, > + loff_t offset, size_t count) > +{ > + struct of_overlay *ov = kobj_to_overlay(kobj); > + struct of_overlay_info *ovinfo; > + char *tmpbuf, *s, *e; > + const char *name; > + ssize_t ret; > + int i, len; > + > + /* allocate work buffer; we know that PAGE_SIZE is enough */ > + tmpbuf = kmalloc(PAGE_SIZE, GFP_KERNEL); > + if (tmpbuf == NULL) > + return -ENOMEM; > + > + s = tmpbuf; > + e = tmpbuf + PAGE_SIZE; > + > + mutex_lock(&of_mutex); > + > + /* targets */ > + for (i = 0; i < ov->count; i++) { > + ovinfo = &ov->ovinfo_tab[i]; > + > + name = of_node_full_name(ovinfo->target); > + len = strlen(name); > + if (s + len + 1 >= e) > + return -ENOMEM; Leaking memory here and holding the mutex. > + memcpy(s, name, len); > + s += len; > + *s++ = '\n'; > + } > + if (s + 1 >= e) > + return -ENOMEM; And here. > + *s++ = '\0'; > + > + /* the buffer is zero terminated */ > + len = s - tmpbuf; > + > + mutex_unlock(&of_mutex); > + > + /* perform the read */ > + ret = memory_read_from_buffer(buf, count, &offset, tmpbuf, len); > + > + /* free the temporary buffer */ > + kfree(tmpbuf); > + > + return ret; > +} > + > +/* targets property */ > +static BIN_ATTR_RO(targets, PAGE_SIZE); > + > +static ssize_t can_remove_read(struct file *filp, struct kobject *kobj, > + struct bin_attribute *bin_attr, char *buf, > + loff_t offset, size_t count) > +{ > + struct of_overlay *ov = kobj_to_overlay(kobj); > + char tbuf[3]; > + > + if (offset < 0) > + return -EINVAL; > + > + if (offset >= sizeof(tbuf)) > + return 0; > + > + if (count > sizeof(tbuf) - offset) > + count = sizeof(tbuf) - offset; > + > + /* fill in temp */ > + tbuf[0] = '0' + overlay_removal_is_ok(ov); > + tbuf[1] = '\n'; > + tbuf[2] = '\0'; > + > + /* copy to buffer */ > + memcpy(buf, tbuf + offset, count); > + > + return count; > +} > + > +/* can_remove property */ > +static BIN_ATTR_RO(can_remove, 3); Same question about bin attr here. > + > /** > * of_overlay_create() - Create and apply an overlay > * @tree: Device node containing all the overlays > @@ -360,6 +503,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) > @@ -416,6 +563,22 @@ int of_overlay_create(struct device_node *tree) > goto err_cancel_overlay; > } > > + /* create targets file */ > + err = sysfs_create_bin_file(&ov->kobj, &bin_attr_targets); > + if (err != 0) { > + pr_err("%s: sysfs_create_bin_file() failed for tree@%s\n", > + __func__, tree->full_name); > + goto err_cancel_overlay; > + } > + > + /* create can_remove file */ > + err = sysfs_create_bin_file(&ov->kobj, &bin_attr_can_remove); > + if (err != 0) { > + pr_err("%s: sysfs_create_bin_file() failed for tree@%s\n", > + __func__, tree->full_name); > + goto err_cancel_overlay; > + } > + > /* add to the tail of the overlay list */ > list_add_tail(&ov->node, &ov_list); > > @@ -596,5 +759,7 @@ int of_overlay_init(void) > if (!ov_kset) > return -ENOMEM; > > - return 0; > + rc = sysfs_create_bin_file(&ov_kset->kobj, &bin_attr_enable); > + WARN(rc, "%s: error adding enable attribute\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/