On Fri, Feb 26, 2021 at 3:03 PM Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx> wrote: > if (smmu->impl && unlikely(smmu->impl->write_reg)) > smmu->impl->write_reg(smmu, page, offset, val); > - else > + else if (dev_64bit_mmio_supported(smmu->dev)) > writel_relaxed(val, arm_smmu_page(smmu, page) + offset); > + else > + hi_lo_writeq_relaxed(val, arm_smmu_page(smmu, page) + offset); > } This is a writel_relaxed(), not a writeq_relaxed(), so I suppose you don't have to change it at all. > + else if (dev_64bit_mmio_supported(smmu->dev)) > + return readq_relaxed(arm_smmu_page(smmu, page) + offset); > + else > + return hi_lo_readq_relaxed(arm_smmu_page(smmu, page) + offset); > } I see this pattern repeat across multiple drivers. I think Christoph had originally suggested folding the if/else logic into the writel_relaxed() that is defined in include/linux/io-64-nonatomic-hi-lo.h, but of course that doesn't work if you need to pass a device pointer. It might still make sense to have another wrapper in that same file though, something like static inline hi_lo_writeq_relaxed_if_possible(struct device *dev, __u64 val, volatile void __iomem *addr) { if (dev_64bit_mmio_supported(smmu->dev)) { readq_relaxed(arm_smmu_page(smmu, page) + offset); } else { writel_relaxed(val >> 32, addr + 4); writel_relaxed(val, addr); } } Arnd