Re: [PATCH v3 09/33] iommu/mediatek: Remove for_each_m4u in tlb_sync_all

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 10/01/22 11:59, Yong Wu ha scritto:
On Mon, 2022-01-10 at 10:16 +0100, AngeloGioacchino Del Regno wrote:
Il 09/01/22 03:48, Yong Wu ha scritto:
On Tue, 2022-01-04 at 16:55 +0100, AngeloGioacchino Del Regno
wrote:
Il 23/09/21 13:58, Yong Wu ha scritto:
The tlb_sync_all is called from these three functions:
a) flush_iotlb_all: it will be called for each a iommu HW.
b) tlb_flush_range_sync: it already has for_each_m4u.
c) in irq: When IOMMU HW translation fault, Only need flush
itself.

Thus, No need for_each_m4u in this tlb_sync_all. Remove it.

Signed-off-by: Yong Wu <yong.wu@xxxxxxxxxxxx>
---
    drivers/iommu/mtk_iommu.c | 18 +++++++-----------
    1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c
b/drivers/iommu/mtk_iommu.c
index 6f4f6624e3ac..0b4c30baa864 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -206,19 +206,15 @@ static struct mtk_iommu_domain
*to_mtk_domain(struct iommu_domain *dom)
static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data
*data)
    {
-	struct list_head *head = data->hw_list;
-
-	for_each_m4u(data, head) {
-		if (pm_runtime_get_if_in_use(data->dev) <= 0)
-			continue;
+	if (pm_runtime_get_if_in_use(data->dev) <= 0)
+		return;
- writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
-			       data->base + data->plat_data-
inv_sel_reg);

-		writel_relaxed(F_ALL_INVLD, data->base +
REG_MMU_INVALIDATE);
-		wmb(); /* Make sure the tlb flush all done */
+	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
+		       data->base + data->plat_data-
inv_sel_reg);
+	writel_relaxed(F_ALL_INVLD, data->base +
REG_MMU_INVALIDATE);
+	wmb(); /* Make sure the tlb flush all done */

There aren't a lot of writes here - not anymore, since you are no
longer doing
this for_each_m4u()...
...so, please change writel_relaxed() to writel() calls, allowing
you
to also
remove the write barrier at the end (since in the non relaxed
version, order is already ensured).

In the "writel", the "__iowmb()" runs before "write_relaxed". Then
how
to guarantee the last register was wrote into the HW. Here the
flush
all don't have sync(waiting it complete)


That's right, I'm sorry for the invalid proposal.

Though, there's something else to mention here... if writing
(F_INVLD_EN1 | F_INVLD_EN0) to inv_sel_reg is *required* to happen
before
writing F_ALL_INVLD to REG_MMU_INVALIDATE (which I think is exactly
the
case here), then, in order to ensure write ordering, you should still
use
writel() instead of the relaxed accessor; after which, since (as you
mentioned)
there is no sync readback loop, you can keep that wmb() at the end.

The writel_relaxed also makes sure the order. I did try this:


https://patchwork.kernel.org/project/linux-mediatek/patch/1570627143-29441-3-git-send-email-yong.wu@xxxxxxxxxxxx/


Ok, that's fair. Means that this patch is fine as it is.
I'll release by R-b on Dafna's patch, as suggested.

Thank you,
- Angelo



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux