On Wed, Mar 2, 2016 at 12:49 PM, atull <at...@opensource.altera.com> wrote: > On Wed, 2 Mar 2016, Rob Herring wrote: > >> On Fri, Feb 26, 2016 at 3:44 PM, Alan Tull <at...@opensource.altera.com> >> wrote: >> > This patch add of overlay notifications. >> > >> > When DT overlays are being added, some drivers/subsystems >> > need to see device tree overlays before the changes go into >> > the live tree.
[...] >> > @@ -389,6 +431,8 @@ int of_overlay_create(struct device_node *tree) >> > /* add to the tail of the overlay list */ >> > list_add_tail(&ov->node, &ov_list); >> > >> > + of_overlay_notify(ov, OF_OVERLAY_POST_APPLY); >> > + >> > mutex_unlock(&of_mutex); >> >> This means that any post-apply handlers can't make any calls that take >> the mutex. Maybe that is fine? On the flip side, maybe we want to >> prevent any changes while the notifiers are called. >> >> The other calls could have similar issues. We should make sure we are >> consistent. >> > > Currently it's consistent - all 4 notifiers hold the mutex, > so all 4 prevent dt changes. That's not super bad, but it's > different from the reconfig notifiers and I could see some > possible usefulness in allowing changes. > > I don't see any harm in unlocking for pre-apply, post-apply, > or post-remove. But for pre-remove, unlocking would allow > notifier code to make changes to the overlay's changeset > that is about to be removed. Is that something we need to > be super worried about preventing? > > For pre-apply, unlocking would allow changes to either the > live tree or the overlay. That's ok since the next thing > that will happen after the notifier is that the changeset > will be built from the overlay, so any changes made to the > overlay would make it into the changeset before it is > applied. Post-apply and post-remove are also ok to > be unlocked. > > So if the pre-remove case is ok, I could release the mutex > for all 4 cases in v3. The only one I can see it being useful to make changes would be pre-apply. The complication there is you would have to move the mutex. I don't think the mutex really needs to be held until before of_overlay_apply, but I'm not completely sure. For now, I think just leave things as they are now. Rob
