Re: [PATCH] iommu: of: skip iommu_device_list traversal in of_iommu_xlate()
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 >> --- >> 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 = _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, _spec->np->fwnode, ops); >> -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH] iommu: of: skip iommu_device_list traversal in of_iommu_xlate()
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. Do you have a system that challenges those assumptions and shows any benefit from this change? Robin. Signed-off-by: Charan Teja Reddy --- 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 = _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, _spec->np->fwnode, ops);
[PATCH] iommu: of: skip iommu_device_list traversal in of_iommu_xlate()
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. Signed-off-by: Charan Teja Reddy --- 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 = _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, _spec->np->fwnode, ops); -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
