| Message ID | 1539657458-24401-10-git-send-email-frowand.list@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | of: overlay: validation checks, subsequent fixes | expand |
On Mon, Oct 15, 2018 at 07:37:29PM -0700, frowand.list@gmail.com wrote: > From: Frank Rowand <frank.rowand@sony.com> > > If overlay properties #address-cells or #size-cells are already in > the live devicetree for any given node, then the values in the > overlay must match the values in the live tree. > > If the properties are already in the live tree then there is no > need to create a changeset entry to add them since they must > have the same value. This reduces the memory used by the > changeset and eliminates a possible memory leak. This is > verified by 12 fewer warnings during the devicetree unittest, > as the possible memory leak warnings about #address-cells and Still missing the rest... And what about my other comments too? > > Signed-off-by: Frank Rowand <frank.rowand@sony.com> > --- > Changes since v3: > - for errors of an overlay changing the value of #size-cells or > #address-cells, return -EINVAL so that overlay apply will fail > - for errors of an overlay changing the value of #size-cells or > #address-cells, make the message more direct. > Old message: > OF: overlay: ERROR: overlay and/or live tree #size-cells invalid in node /soc/base_fpga_region > New message: > OF: overlay: ERROR: changing value of /soc/base_fpga_region/#size-cells not allowed > > drivers/of/overlay.c | 42 +++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 39 insertions(+), 3 deletions(-)
On 10/18/18 11:13, Rob Herring wrote: > On Mon, Oct 15, 2018 at 07:37:29PM -0700, frowand.list@gmail.com wrote: >> From: Frank Rowand <frank.rowand@sony.com> >> >> If overlay properties #address-cells or #size-cells are already in >> the live devicetree for any given node, then the values in the >> overlay must match the values in the live tree. >> >> If the properties are already in the live tree then there is no >> need to create a changeset entry to add them since they must >> have the same value. This reduces the memory used by the >> changeset and eliminates a possible memory leak. This is >> verified by 12 fewer warnings during the devicetree unittest, >> as the possible memory leak warnings about #address-cells and > > Still missing the rest... > > And what about my other comments too? Apologies, paper bag. Will fix all. -Frank > >> >> Signed-off-by: Frank Rowand <frank.rowand@sony.com> >> --- >> Changes since v3: >> - for errors of an overlay changing the value of #size-cells or >> #address-cells, return -EINVAL so that overlay apply will fail >> - for errors of an overlay changing the value of #size-cells or >> #address-cells, make the message more direct. >> Old message: >> OF: overlay: ERROR: overlay and/or live tree #size-cells invalid in node /soc/base_fpga_region >> New message: >> OF: overlay: ERROR: changing value of /soc/base_fpga_region/#size-cells not allowed >> >> drivers/of/overlay.c | 42 +++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 39 insertions(+), 3 deletions(-) >
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 272a0d1a5e18..3e1e519c12f0 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -287,7 +287,12 @@ static struct property *dup_and_fixup_symbol_prop( * @target may be either in the live devicetree or in a new subtree that * is contained in the changeset. * - * Some special properties are not updated (no error returned). + * Some special properties are not added or updated (no error returned): + * "name", "phandle", "linux,phandle". + * + * Properties "#address-cells" and "#size-cells" are not updated if they + * are already in the live tree, but if present in the live tree, the values + * in the overlay must match the values in the live tree. * * Update of property in symbols node is not allowed. * @@ -300,6 +305,7 @@ static int add_changeset_property(struct overlay_changeset *ovcs, { struct property *new_prop = NULL, *prop; int ret = 0; + bool check_for_non_overlay_node = false; if (!of_prop_cmp(overlay_prop->name, "name") || !of_prop_cmp(overlay_prop->name, "phandle") || @@ -322,13 +328,43 @@ static int add_changeset_property(struct overlay_changeset *ovcs, if (!new_prop) return -ENOMEM; - if (!prop) + if (!prop) { + + check_for_non_overlay_node = true; ret = of_changeset_add_property(&ovcs->cset, target->np, new_prop); - else + + } else if (!of_prop_cmp(prop->name, "#address-cells")) { + + if (prop->length != 4 || new_prop->length != 4 || + *(u32 *)prop->value != *(u32 *)new_prop->value) { + pr_err("ERROR: changing value of %pOF/#address-cells is not allowed\n", + target->np); + ret = -EINVAL; + } + + } else if (!of_prop_cmp(prop->name, "#size-cells")) { + + if (prop->length != 4 || new_prop->length != 4 || + *(u32 *)prop->value != *(u32 *)new_prop->value) { + pr_err("ERROR: changing value of %pOF/#size-cells is not allowed\n", + target->np); + ret = -EINVAL; + } + + } else { + + check_for_non_overlay_node = true; ret = of_changeset_update_property(&ovcs->cset, target->np, new_prop); + } + + if (check_for_non_overlay_node && + !of_node_check_flag(target->np, OF_OVERLAY)) + pr_err("WARNING: %s(), memory leak will occur if overlay removed. Property: %pOF/%s\n", + __func__, target->np, new_prop->name); + if (ret) { kfree(new_prop->name); kfree(new_prop->value);