Messages in this thread | | | From | Grant Likely <> | Date | Thu, 6 Nov 2014 12:47:19 +0000 | Subject | Re: [PATCH 3/5] of: Only call notifiers when node is attached |
| |
On Thu, Nov 6, 2014 at 12:33 PM, Pantelis Antoniou <pantelis.antoniou@konsulko.com> wrote: > Hi Grant, > >> On Nov 5, 2014, at 23:39 , Grant Likely <grant.likely@secretlab.ca> wrote: >> >> On Tue, 28 Oct 2014 22:33:51 +0200 >> , Pantelis Antoniou <pantelis.antoniou@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. >> > > Valid points. In fact I performed some tests and with this reverted things > still work. > > The rationale behind this is for when nodes/properties are removed in the overlay, > but since we don't support this for now, we never hit the case where it was > needed. > > Please remove from the patch series, I'll revisit this when I add the removal > functionality.
Cool, thanks.
g.
| |