On Tue, 28 Oct 2014 22:33:51 +0200
, Pantelis Antoniou <pantelis.anton...@konsulko.com>
 wrote:
> Make sure we call notifier only when the node is attached.
> When a detatched tree is being constructed we do not want the
> notifiers to fire at all.

The description does not match what the patch does. The patch moves the
test into of_{add,remove,update}_property() and out of
of_property_notify() itself. That leaves one other caller of
of_property_notify(); __of_changeset_entry_notify(). The effect of this
patch is that applying a changeset will cause notifiers to be fired for
each property modified in a changeset. The comment says nothing about
the change in behaviour and it sounds like it is a bug fix when it
doesn't actually change the behaviour at all for the
of_{add,remove,update}_property() paths.

This needs a better changelog. It needs to describe what the effects of
the patch are and why the change is being made. When someone is
bisecting a problem and they land on this change, the changelog needs to
give them a good idea about what is going on and why.

g.

> 
> Signed-off-by: Pantelis Antoniou <pantelis.anton...@konsulko.com>
> ---
>  drivers/of/base.c    | 9 ++++++---
>  drivers/of/dynamic.c | 5 +----
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 2305dc0..a79d4ee 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1695,7 +1695,8 @@ int of_add_property(struct device_node *np, struct 
> property *prop)
>  
>       mutex_unlock(&of_mutex);
>  
> -     if (!rc)
> +     /* only call notifiers if the node is attached and no error occurred */
> +     if (of_node_is_attached(np) && !rc)
>               of_property_notify(OF_RECONFIG_ADD_PROPERTY, np, prop, NULL);
>  
>       return rc;
> @@ -1754,7 +1755,8 @@ int of_remove_property(struct device_node *np, struct 
> property *prop)
>  
>       mutex_unlock(&of_mutex);
>  
> -     if (!rc)
> +     /* only call notifiers if the node is attached and no error occurred */
> +     if (of_node_is_attached(np) && !rc)
>               of_property_notify(OF_RECONFIG_REMOVE_PROPERTY, np, prop, NULL);
>  
>       return rc;
> @@ -1830,7 +1832,8 @@ int of_update_property(struct device_node *np, struct 
> property *newprop)
>  
>       mutex_unlock(&of_mutex);
>  
> -     if (!rc)
> +     /* only call notifiers if the node is attached and no error occurred */
> +     if (of_node_is_attached(np) && !rc)
>               of_property_notify(OF_RECONFIG_UPDATE_PROPERTY, np, newprop, 
> oldprop);
>  
>       return rc;
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index f297891..8e8b614 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -90,10 +90,6 @@ int of_property_notify(int action, struct device_node *np,
>  {
>       struct of_prop_reconfig pr;
>  
> -     /* only call notifiers if the node is attached */
> -     if (!of_node_is_attached(np))
> -             return 0;
> -
>       pr.dn = np;
>       pr.prop = prop;
>       pr.old_prop = oldprop;
> @@ -138,6 +134,7 @@ int of_attach_node(struct device_node *np)
>       __of_attach_node_sysfs(np);
>       mutex_unlock(&of_mutex);
>  
> +     /* node is guaranteed to be attached at this point */
>       of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, np);
>  
>       return 0;
> -- 
> 1.7.12
> 

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

Reply via email to