[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8e1847d0-7c6f-b022-999b-666cb90e9931@collabora.com>
Date: Thu, 27 Jan 2022 12:15:18 +0100
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Yong Wu <yong.wu@...iatek.com>, Joerg Roedel <joro@...tes.org>,
Rob Herring <robh+dt@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>,
Will Deacon <will@...nel.org>
Cc: Robin Murphy <robin.murphy@....com>,
Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
Tomasz Figa <tfiga@...omium.org>,
linux-mediatek@...ts.infradead.org, srv_heupstream@...iatek.com,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
iommu@...ts.linux-foundation.org,
Hsin-Yi Wang <hsinyi@...omium.org>, youlin.pei@...iatek.com,
anan.sun@...iatek.com, xueqi.zhang@...iatek.com,
yen-chang.chen@...iatek.com, mingyuan.ma@...iatek.com,
yf.wang@...iatek.com, libo.kang@...iatek.com,
chengci.xu@...iatek.com
Subject: Re: [PATCH v4 27/35] iommu/mediatek: Remove mtk_iommu.h
Il 25/01/22 09:56, Yong Wu ha scritto:
> Currently there is only compare_of/release_of/a suspend structure in the
> header file. I think it is no need to keep a header file only for these.
> Move these into the c file and rm this header file.
>
> I think there should be a common helper for compare_of and release_of.
> There is many copy in drm, it should be another topic.
>
> Signed-off-by: Yong Wu <yong.wu@...iatek.com>
> ---
> drivers/iommu/mtk_iommu.c | 25 ++++++++++++++++++++-
> drivers/iommu/mtk_iommu.h | 42 ------------------------------------
> drivers/iommu/mtk_iommu_v1.c | 21 +++++++++++++++---
> 3 files changed, 42 insertions(+), 46 deletions(-)
> delete mode 100644 drivers/iommu/mtk_iommu.h
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 80c1e5a75868..f88c7bb235bf 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -14,6 +14,7 @@
> #include <linux/io.h>
> #include <linux/iommu.h>
> #include <linux/iopoll.h>
> +#include <linux/io-pgtable.h>
> #include <linux/list.h>
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> @@ -30,7 +31,7 @@
> #include <asm/barrier.h>
> #include <soc/mediatek/smi.h>
>
> -#include "mtk_iommu.h"
> +#include <dt-bindings/memory/mtk-memory-port.h>
>
> #define REG_MMU_PT_BASE_ADDR 0x000
> #define MMU_PT_ADDR_MASK GENMASK(31, 7)
> @@ -166,6 +167,17 @@ struct mtk_iommu_iova_region {
> unsigned long long size;
> };
>
> +struct mtk_iommu_suspend_reg {
> + u32 misc_ctrl;
> + u32 dcm_dis;
> + u32 ctrl_reg;
> + u32 int_control0;
> + u32 int_main_control;
> + u32 ivrp_paddr;
> + u32 vld_pa_rng;
> + u32 wr_len_ctrl;
> +};
> +
> struct mtk_iommu_plat_data {
> enum mtk_iommu_plat m4u_plat;
> u32 flags;
> @@ -219,6 +231,17 @@ struct mtk_iommu_domain {
> struct mutex mutex; /* Protect "data" in this structure */
> };
>
> +/* TODO: A common helper is expected. */
> +static inline int compare_of(struct device *dev, void *data)
> +{
> + return dev->of_node == data;
> +}
> +
> +static inline void release_of(struct device *dev, void *data)
> +{
> + of_node_put(data);
> +}
> +
Since it's just one line, at this point you should also open-code these,
as in you can then remove the two helper functions entirely.
So, please do that.
> static inline int mtk_iommu_bind(struct device *dev)
> {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> deleted file mode 100644
> index d332f9769f83..000000000000
> --- a/drivers/iommu/mtk_iommu.h
> +++ /dev/null
> @@ -1,42 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Copyright (c) 2015-2016 MediaTek Inc.
> - * Author: Honghui Zhang <honghui.zhang@...iatek.com>
> - */
> -
> -#ifndef _MTK_IOMMU_H_
> -#define _MTK_IOMMU_H_
> -
> -#include <linux/device.h>
> -#include <linux/io.h>
> -#include <linux/io-pgtable.h>
> -#include <linux/iommu.h>
> -#include <linux/spinlock.h>
> -#include <soc/mediatek/smi.h>
> -#include <dt-bindings/memory/mtk-memory-port.h>
> -
> -struct mtk_iommu_suspend_reg {
> - union {
> - u32 standard_axi_mode;/* v1 */
> - u32 misc_ctrl;/* v2 */
> - };
> - u32 dcm_dis;
> - u32 ctrl_reg;
> - u32 int_control0;
> - u32 int_main_control;
> - u32 ivrp_paddr;
> - u32 vld_pa_rng;
> - u32 wr_len_ctrl;
> -};
> -
> -static inline int compare_of(struct device *dev, void *data)
> -{
> - return dev->of_node == data;
> -}
> -
> -static inline void release_of(struct device *dev, void *data)
> -{
> - of_node_put(data);
> -}
> -
> -#endif
> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index b762a05328d4..23c3bc175153 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c
> @@ -7,7 +7,6 @@
> *
> * Based on driver/iommu/mtk_iommu.c
> */
> -#include <linux/memblock.h>
> #include <linux/bug.h>
> #include <linux/clk.h>
> #include <linux/component.h>
> @@ -28,10 +27,9 @@
> #include <linux/spinlock.h>
> #include <asm/barrier.h>
> #include <asm/dma-iommu.h>
> -#include <linux/init.h>
> +#include <dt-bindings/memory/mtk-memory-port.h>
> #include <dt-bindings/memory/mt2701-larb-port.h>
> #include <soc/mediatek/smi.h>
> -#include "mtk_iommu.h"
>
> #define REG_MMU_PT_BASE_ADDR 0x000
>
> @@ -87,6 +85,13 @@
> */
> #define M2701_IOMMU_PGT_SIZE SZ_4M
>
> +struct mtk_iommu_suspend_reg {
> + u32 standard_axi_mode;
> + u32 dcm_dis;
> + u32 ctrl_reg;
> + u32 int_control0;
> +};
> +
> struct mtk_iommu_data {
> void __iomem *base;
> int irq;
> @@ -110,6 +115,16 @@ struct mtk_iommu_domain {
> struct mtk_iommu_data *data;
> };
>
> +static inline int compare_of(struct device *dev, void *data)
> +{
> + return dev->of_node == data;
> +}
> +
> +static inline void release_of(struct device *dev, void *data)
> +{
> + of_node_put(data);
> +}
> +
....And the same comment applies here too.
> static inline int mtk_iommu_bind(struct device *dev)
> {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
>
Powered by blists - more mailing lists