Hi Jen, On Mon, Aug 21, 2023 at 08:53:38AM +0200, Jan Beulich wrote: > On 19.08.2023 02:28, Vikram Garhwal wrote: > > Rename iommu_dt_device_is_assigned() to > > iommu_dt_device_is_assigned_locked(). > > Remove static type so this can also be used by SMMU drivers to check if the > > device is being used before removing. > > > > Moving spin_lock to caller was done to prevent the concurrent access to > > iommu_dt_device_is_assigned while doing add/remove/assign/deassign. > > Follow-up > > patches in this series introduces node add/remove feature. > > > > Also, caller is required hold the correct lock so moved the function > > prototype > > to a private header. > > > > Signed-off-by: Vikram Garhwal <vikram.garh...@amd.com> > > > > --- > > Changes from v7: > > Update commit message. > > Add ASSERT(). > > --- > > --- > > xen/drivers/passthrough/device_tree.c | 26 +++++++++++++++++++++---- > > xen/include/xen/iommu-private.h | 28 +++++++++++++++++++++++++++ > > 2 files changed, 50 insertions(+), 4 deletions(-) > > create mode 100644 xen/include/xen/iommu-private.h > > I find it odd for new versions to be sent when discussion on the prior > version hasn't settled yet, and when you then also don't incorporate > the feedback given. I also find it odd to now be Cc-ed on the entire > series. Imo instead of that patches which aren't self-explanatory / > self-consistent simply need their descriptions improved (in the case > here to mention what further use of a function is intended). Yet > better would be to add declarations (and remove static) only at the > point that's actually needed. This then eliminates the risk of > subsequent changes in response to feedback (like Julien's here) > resulting in the declaration no longer being needed, thus leading to > a Misra violation (besides the general tidiness aspect). > Reason behind sending new version: Patch 15/19 is largest patch in terms of change and there was a long discussion with Julien and Stefano regarding a big change. Last week(after v8) we agreed on change and Stefano and Julien were okay to send v9 so it will be easier to review with that change.
Regarding cc-ing you to whole series, there was following comment on v8 09/1 "I don't think private headers should live in include/xen/. Judging from only the patches I was Cc-ed on." I thought you wanted to see the whole full series. Looks like i misunderstood the comment and Will remove the from cc-list in next version. The function itself will be needed in further patches in this series. I am okay with keeping static and other things same here to avoid MISRA violation and change wherever they are called first time. Regarding Julien's comment i am reply back in same thread on why this was not taken in account. > Jan
