Messages in this thread | | | Subject | Re: [RESEND PATCH v17 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device | From | Vivek Gautam <> | Date | Mon, 26 Nov 2018 16:56:42 +0530 |
| |
On 11/26/2018 11:33 AM, Vivek Gautam wrote: > > > On 11/24/2018 12:06 AM, Will Deacon wrote: >> On Thu, Nov 22, 2018 at 05:32:24PM +0530, Vivek Gautam wrote: >>> Hi Will, >>> >>> On Wed, Nov 21, 2018 at 11:09 PM Will Deacon <will.deacon@arm.com> >>> wrote: >>>> On Fri, Nov 16, 2018 at 04:54:27PM +0530, Vivek Gautam wrote: >>>>> From: Sricharan R <sricharan@codeaurora.org> >>>>> >>>>> The smmu device probe/remove and add/remove master device callbacks >>>>> gets called when the smmu is not linked to its master, that is >>>>> without >>>>> the context of the master device. So calling runtime apis in those >>>>> places >>>>> separately. >>>>> Global locks are also initialized before enabling runtime pm as the >>>>> runtime_resume() calls device_reset() which does tlb_sync_global() >>>>> that ultimately requires locks to be initialized. >>>>> >>>>> Signed-off-by: Sricharan R <sricharan@codeaurora.org> >>>>> [vivek: Cleanup pm runtime calls] >>>>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> >>>>> Reviewed-by: Tomasz Figa <tfiga@chromium.org> >>>>> Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com> >>>>> --- >>>>> drivers/iommu/arm-smmu.c | 101 >>>>> ++++++++++++++++++++++++++++++++++++++++++----- >>>>> 1 file changed, 91 insertions(+), 10 deletions(-) >>>> Given that you're doing the get/put in the TLBI ops unconditionally: >>>> >>>>> static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain) >>>>> { >>>>> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); >>>>> + struct arm_smmu_device *smmu = smmu_domain->smmu; >>>>> >>>>> - if (smmu_domain->tlb_ops) >>>>> + if (smmu_domain->tlb_ops) { >>>>> + arm_smmu_rpm_get(smmu); >>>>> smmu_domain->tlb_ops->tlb_flush_all(smmu_domain); >>>>> + arm_smmu_rpm_put(smmu); >>>>> + } >>>>> } >>>>> >>>>> static void arm_smmu_iotlb_sync(struct iommu_domain *domain) >>>>> { >>>>> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); >>>>> + struct arm_smmu_device *smmu = smmu_domain->smmu; >>>>> >>>>> - if (smmu_domain->tlb_ops) >>>>> + if (smmu_domain->tlb_ops) { >>>>> + arm_smmu_rpm_get(smmu); >>>>> smmu_domain->tlb_ops->tlb_sync(smmu_domain); >>>>> + arm_smmu_rpm_put(smmu); >>>>> + } >>>> Why do you need them around the map/unmap calls as well? >>> We still have .tlb_add_flush path? >> Ok, so we could add the ops around that as well. Right now, we've got >> the runtime pm hooks crossing two parts of the API. > > Sure, will do that then, and remove the runtime pm hooks from map/unmap.
I missed this earlier - We are adding runtime pm hooks in the 'iommu_ops' callbacks and not really to 'tlb_ops'. So how the runtime pm hooks crossing the paths? '.map/.unmap' iommu_ops don't call '.flush_iotlb_all' or '.iotlb_sync' iommu_ops anywhere.
E.g., only callers to domain->ops->flush_iotlb_all() are: iommu_dma_flush_iotlb_all(), or iommu_flush_tlb_all() which are not in map/unmap paths.
Regards Vivek
> > Thanks > Vivek >> >> Will >
| |