On 9/23/2020 9:54 PM, Robin Murphy wrote: > On 2020-09-23 15:53, Charan Teja Reddy wrote: >> In of_iommu_xlate(), check if iommu device is enabled before traversing >> the iommu_device_list through iommu_ops_from_fwnode(). It is of no use >> in traversing the iommu_device_list only to return NO_IOMMU because of >> iommu device node is disabled. > > Well, the "use" is that it keeps the code that much smaller and simpler > to have a single path for returning this condition. This whole callstack > isn't exactly a high-performance code path to begin with, and we've > always assumed that IOMMUs present but disabled in DT would be a pretty > rare exception.
Fine..I thought that it is logical to return when IOMMU DT node is disabled over code simplicity. And agree that it is not high-performance path. > Do you have a system that challenges those assumptions > and shows any benefit from this change? No, I don't have a system that challenges these assumptions. > > Robin. > >> Signed-off-by: Charan Teja Reddy <chara...@codeaurora.org> >> --- >> drivers/iommu/of_iommu.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c >> index e505b91..225598c 100644 >> --- a/drivers/iommu/of_iommu.c >> +++ b/drivers/iommu/of_iommu.c >> @@ -94,9 +94,10 @@ static int of_iommu_xlate(struct device *dev, >> struct fwnode_handle *fwnode = &iommu_spec->np->fwnode; >> int ret; >> + if (!of_device_is_available(iommu_spec->np)) >> + return NO_IOMMU; >> ops = iommu_ops_from_fwnode(fwnode); >> - if ((ops && !ops->of_xlate) || >> - !of_device_is_available(iommu_spec->np)) >> + if (ops && !ops->of_xlate) >> return NO_IOMMU; >> ret = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); >> -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
