On 15/08/2019 11:56, Will Deacon wrote:
On Fri, Aug 09, 2019 at 06:07:45PM +0100, Robin Murphy wrote:Context bank accesses are fiddly enough to deserve a number of extra helpers to keep the callsites looking sane, even though there are only one or two of each.Signed-off-by: Robin Murphy <[email protected]> --- drivers/iommu/arm-smmu.c | 137 ++++++++++++++++++++------------------- 1 file changed, 72 insertions(+), 65 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 72505647b77d..abdcc3f52e2e 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -82,9 +82,6 @@ ((smmu->options & ARM_SMMU_OPT_SECURE_CFG_ACCESS) \ ? 0x400 : 0))-/* Translation context bank */-#define ARM_SMMU_CB(smmu, n) ((smmu)->base + (((smmu)->cb_base + (n)) << (smmu)->pgshift)) - #define MSI_IOVA_BASE 0x8000000 #define MSI_IOVA_LENGTH 0x100000@@ -265,9 +262,29 @@ static void arm_smmu_writel(struct arm_smmu_device *smmu, int page, int offset,writel_relaxed(val, arm_smmu_page(smmu, page) + offset); }+static u64 arm_smmu_readq(struct arm_smmu_device *smmu, int page, int offset)+{ + return readq_relaxed(arm_smmu_page(smmu, page) + offset); +} + +static void arm_smmu_writeq(struct arm_smmu_device *smmu, int page, int offset, + u64 val) +{ + writeq_relaxed(val, arm_smmu_page(smmu, page) + offset); +} + #define arm_smmu_read_gr1(s, r) arm_smmu_readl((s), 1, (r)) #define arm_smmu_write_gr1(s, r, v) arm_smmu_writel((s), 1, (r), (v))+#define arm_smmu_read_cb(s, n, r) \+ arm_smmu_readl((s), (s)->cb_base + (n), (r)) +#define arm_smmu_write_cb(s, n, r, v) \ + arm_smmu_writel((s), (s)->cb_base + (n), (r), (v)) +#define arm_smmu_read_cb_q(s, n, r) \ + arm_smmu_readq((s), (s)->cb_base + (n), (r)) +#define arm_smmu_write_cb_q(s, n, r, v) \ + arm_smmu_writeq((s), (s)->cb_base + (n), (r), (v))'r' for 'offset'? (maybe just rename offset => register in the helpers).
I think this all represents the mangled remains of an underlying notion of 'register offset' ;)
struct arm_smmu_option_prop { u32 opt; const char *prop; @@ -423,15 +440,17 @@ static void __arm_smmu_free_bitmap(unsigned long *map, int idx) }/* Wait for any pending TLB invalidations to complete */-static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu, - void __iomem *sync, void __iomem *status) +static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu, int page, + int sync, int status) { unsigned int spin_cnt, delay; + u32 reg;- writel_relaxed(QCOM_DUMMY_VAL, sync);+ arm_smmu_writel(smmu, page, sync, QCOM_DUMMY_VAL); for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) { for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) { - if (!(readl_relaxed(status) & sTLBGSTATUS_GSACTIVE)) + reg = arm_smmu_readl(smmu, page, status); + if (!(reg & sTLBGSTATUS_GSACTIVE)) return; cpu_relax(); } @@ -443,12 +462,11 @@ static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu,static void arm_smmu_tlb_sync_global(struct arm_smmu_device *smmu){ - void __iomem *base = ARM_SMMU_GR0(smmu); unsigned long flags;spin_lock_irqsave(&smmu->global_sync_lock, flags);- __arm_smmu_tlb_sync(smmu, base + ARM_SMMU_GR0_sTLBGSYNC, - base + ARM_SMMU_GR0_sTLBGSTATUS); + __arm_smmu_tlb_sync(smmu, 0, ARM_SMMU_GR0_sTLBGSYNC,Can we have a #define for page zero, please?
Again, now I recall pondering the exact same thought, so clearly I don't have any grounds to object. I guess it's worth reworking the previous ARM_SMMU_{GR0,GR1,CB()} macros into the page number scheme rather than just killing them off - let me give that a try.
Robin. _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
