Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2 2/2] soc: mediatek: add mtk-devapc driver
To:     Neal Liu <neal.liu@mediatek.com>, Rob Herring <robh+dt@kernel.org>
Cc:     devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org,
        lkml <linux-kernel@vger.kernel.org>, wsd_upstream@mediatek.com
References: <1594285927-1840-1-git-send-email-neal.liu@mediatek.com>
 <1594285927-1840-3-git-send-email-neal.liu@mediatek.com>
From:   Matthias Brugger <matthias.bgg@gmail.com>
Message-ID: <f93663d4-86f1-971f-ed2c-67cd65d5049d@gmail.com>
Date:   Fri, 10 Jul 2020 14:14:33 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.9.0
MIME-Version: 1.0
In-Reply-To: <1594285927-1840-3-git-send-email-neal.liu@mediatek.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 09/07/2020 11:12, Neal Liu wrote:
> MediaTek bus fabric provides TrustZone security support and data
> protection to prevent slaves from being accessed by unexpected
> masters.
> The security violation is logged and sent to the processor for
> further analysis or countermeasures.
> 
> Any occurrence of security violation would raise an interrupt, and
> it will be handled by mtk-devapc driver. The violation
> information is printed in order to find the murderer.
> 
> Signed-off-by: Neal Liu <neal.liu@mediatek.com>
> ---
>   drivers/soc/mediatek/Kconfig      |    9 +
>   drivers/soc/mediatek/Makefile     |    1 +
>   drivers/soc/mediatek/mtk-devapc.c |  466 ++++++++++++++++++++++++++
>   drivers/soc/mediatek/mtk-devapc.h |  670 +++++++++++++++++++++++++++++++++++++
>   4 files changed, 1146 insertions(+)
>   create mode 100644 drivers/soc/mediatek/mtk-devapc.c
>   create mode 100644 drivers/soc/mediatek/mtk-devapc.h
> 
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 59a56cd..1177c98 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -17,6 +17,15 @@ config MTK_CMDQ
>   	  time limitation, such as updating display configuration during the
>   	  vblank.
>   
> +config MTK_DEVAPC
> +	tristate "Mediatek Device APC Support"
> +	help
> +	  Say yes here to enable support for Mediatek Device APC driver.
> +	  This driver is mainly used to handle the violation which catches
> +	  unexpected transaction.
> +	  The violation information is logged for further analysis or
> +	  countermeasures.
> +
>   config MTK_INFRACFG
>   	bool "MediaTek INFRACFG Support"
>   	select REGMAP
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 01f9f87..abfd4ba 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -1,5 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
> +obj-$(CONFIG_MTK_DEVAPC) += mtk-devapc.o
>   obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>   obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>   obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
> new file mode 100644
> index 0000000..11fa366
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-devapc.c
> @@ -0,0 +1,466 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 MediaTek Inc.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include "mtk-devapc.h"
> +
> +/*
> + * mtk_devapc_pd_get - get devapc pd_types of register address.
> + *
> + * Returns the value of reg addr
> + */
> +static void __iomem *mtk_devapc_pd_get(struct mtk_devapc_context *devapc_ctx,
> +				       int slave_type,
> +				       enum DEVAPC_PD_REG_TYPE pd_reg_type,
> +				       u32 reg_idx)
> +{
> +	void __iomem *reg;
> +
> +	reg = devapc_ctx->devapc_pd_base[slave_type] +
> +		devapc_ctx->pds_offset[pd_reg_type];
> +
> +	if (pd_reg_type == VIO_MASK || pd_reg_type == VIO_STA)
> +		reg += 0x4 * reg_idx;
> +
> +	return reg;
> +}
> +
> +static int get_vio_slave_num(int slave_type)

I have a hard time to understand the usefullness of this, can you please explain.

> +{
> +	if (slave_type == 0)
> +		return ARRAY_SIZE(mtk_devices_infra);
> +
> +	return 0;
> +}
> +
> +static u32 get_shift_group(struct mtk_devapc_context *devapc_ctx,
> +			   int slave_type, int vio_idx)
> +{
> +	u32 vio_shift_sta;
> +	void __iomem *reg;
> +	int bit;
> +
> +	reg = mtk_devapc_pd_get(devapc_ctx, slave_type, VIO_SHIFT_STA, 0);
> +	vio_shift_sta = readl(reg);
> +
> +	for (bit = 0; bit < 32; bit++) {
> +		if ((vio_shift_sta >> bit) & 0x1) > +			break;
> +	}
> +
> +	return bit;

We return the first position (from the right) of the rigster with the bit set to 
one. Correct?
Can't we use __ffs() for this?

> +}
> +
> +static int check_vio_mask_sta(struct mtk_devapc_context *devapc_ctx,
> +			      int slave_type, u32 module, int pd_reg_type)
> +{
> +	u32 reg_index, reg_offset;
> +	void __iomem *reg;
> +	u32 value;
> +
> +	VIO_MASK_STA_REG_GET(module);
> +
> +	reg = mtk_devapc_pd_get(devapc_ctx, slave_type, pd_reg_type, reg_index);

reg = mtk_devapc_pd_get(devapc_ctx, slave_type, pd_reg_type, 
VIO_MOD_TO_REG_IND(module));

> +	value = readl(reg);
> +
> +	return ((value >> reg_offset) & 0x1);

return ((value >> VIO_MOD_TO_REG_OFF(module)) & 0x1);

> +}
> +
> +static int check_vio_mask(struct mtk_devapc_context *devapc_ctx, int slave_type,
> +			  u32 module)
> +{
> +	return check_vio_mask_sta(devapc_ctx, slave_type, module, VIO_MASK);
> +}
> +
> +static int check_vio_status(struct mtk_devapc_context *devapc_ctx,
> +			    int slave_type, u32 module)
> +{
> +	return check_vio_mask_sta(devapc_ctx, slave_type, module, VIO_STA);
> +}
> +
> +static void clear_vio_status(struct mtk_devapc_context *devapc_ctx,
> +			     int slave_type, u32 module)
> +{
> +	u32 reg_index, reg_offset;
> +	void __iomem *reg;
> +
> +	VIO_MASK_STA_REG_GET(module);
> +
> +	reg = mtk_devapc_pd_get(devapc_ctx, slave_type, VIO_STA, reg_index);
> +	writel(0x1 << reg_offset, reg);
> +
> +	if (check_vio_status(devapc_ctx, slave_type, module))
> +		pr_err(PFX "%s: Clear failed, slave_type:0x%x, module_index:0x%x\n",
> +		       __func__, slave_type, module);
> +}
> +
> +static void mask_module_irq(struct mtk_devapc_context *devapc_ctx,
> +			    int slave_type, u32 module, bool mask)
> +{
> +	u32 reg_index, reg_offset;
> +	void __iomem *reg;
> +	u32 value;
> +
> +	VIO_MASK_STA_REG_GET(module);
> +
> +	reg = mtk_devapc_pd_get(devapc_ctx, slave_type, VIO_MASK, reg_index);
> +
> +	value = readl(reg);
> +	if (mask)
> +		value |= (0x1 << reg_offset);
> +	else
> +		value &= ~(0x1 << reg_offset);
> +
> +	writel(value, reg);
> +}
> +
> +#define TIMEOUT_MS		10000
> +
> +static int read_poll_timeout(void __iomem *addr, u32 mask)

That function is defined in include/linux/iopoll.h

> +{
> +	unsigned long timeout = jiffies + msecs_to_jiffies(TIMEOUT_MS);
> +
> +	do {
> +		if (readl_relaxed(addr) & mask)

Please use a variable where you write your value to and then check for the mask. 
That maks the code easier to read and I think is part of the coding style.

> +			return 0;
> +
> +	} while (!time_after(jiffies, timeout));
> +
> +	return (readl_relaxed(addr) & mask) ? 0 : -ETIMEDOUT;
> +}
> +
> +/*
> + * sync_vio_dbg - start to get violation information by selecting violation
> + *		  group and enable violation shift.
> + *
> + * Returns sync done or not
> + */
> +static u32 sync_vio_dbg(struct mtk_devapc_context *devapc_ctx, int slave_type,
> +			u32 shift_bit)
> +{
> +	void __iomem *pd_vio_shift_sta_reg;
> +	void __iomem *pd_vio_shift_sel_reg;
> +	void __iomem *pd_vio_shift_con_reg;
> +	u32 sync_done = 0;
> +
> +	pd_vio_shift_sta_reg = mtk_devapc_pd_get(devapc_ctx, slave_type,
> +						 VIO_SHIFT_STA, 0);
> +	pd_vio_shift_sel_reg = mtk_devapc_pd_get(devapc_ctx, slave_type,
> +						 VIO_SHIFT_SEL, 0);
> +	pd_vio_shift_con_reg = mtk_devapc_pd_get(devapc_ctx, slave_type,
> +						 VIO_SHIFT_CON, 0);
> +
> +	writel(0x1 << shift_bit, pd_vio_shift_sel_reg);
> +	writel(0x1, pd_vio_shift_con_reg);
> +
> +	if (!read_poll_timeout(pd_vio_shift_con_reg, 0x2))
> +		sync_done = 1;
> +	else
> +		pr_err(PFX "%s: Shift violation info failed\n", __func__);
> +
> +	/* Disable shift mechanism */

Please add a comment explaining what the shift mechanism is about.

> +	writel(0x0, pd_vio_shift_con_reg);
> +	writel(0x0, pd_vio_shift_sel_reg);
> +	writel(0x1 << shift_bit, pd_vio_shift_sta_reg);
> +
> +	return sync_done;
> +}
> +
> +static void devapc_vio_info_print(struct mtk_devapc_context *devapc_ctx)
> +{
> +	struct mtk_devapc_vio_info *vio_info = devapc_ctx->vio_info;
> +
> +	/* Print violation information */
> +	if (vio_info->write)
> +		pr_info(PFX "Write Violation\n");
> +	else if (vio_info->read)
> +		pr_info(PFX "Read Violation\n");
> +
> +	pr_info(PFX "%s%x, %s%x, %s%x, %s%x\n",
> +		"Vio Addr:0x", vio_info->vio_addr,
> +		"High:0x", vio_info->vio_addr_high,
> +		"Bus ID:0x", vio_info->master_id,
> +		"Dom ID:0x", vio_info->domain_id);
> +}
> +
> +static void devapc_extract_vio_dbg(struct mtk_devapc_context *devapc_ctx,
> +				   int slave_type)
> +{
> +	void __iomem *vio_dbg0_reg, *vio_dbg1_reg;
> +	struct mtk_devapc_vio_dbgs_desc *vio_dbgs;
> +	struct mtk_devapc_vio_info *vio_info;
> +	u32 dbg0;
> +
> +	vio_dbg0_reg = mtk_devapc_pd_get(devapc_ctx, slave_type, VIO_DBG0, 0);
> +	vio_dbg1_reg = mtk_devapc_pd_get(devapc_ctx, slave_type, VIO_DBG1, 0);
> +
> +	vio_dbgs = devapc_ctx->vio_dbgs_desc;
> +	vio_info = devapc_ctx->vio_info;
> +
> +	/* Extract violation information */
> +	dbg0 = readl(vio_dbg0_reg);
> +	vio_info->vio_addr = readl(vio_dbg1_reg);
> +
> +	vio_info->master_id = (dbg0 & vio_dbgs[MSTID].mask) >>
> +			      vio_dbgs[MSTID].start_bit;
> +	vio_info->domain_id = (dbg0 & vio_dbgs[DMNID].mask) >>
> +			      vio_dbgs[DMNID].start_bit;
> +	vio_info->write = ((dbg0 & vio_dbgs[VIO_W].mask) >>
> +			   vio_dbgs[VIO_W].start_bit) == 1;
> +	vio_info->read = ((dbg0 & vio_dbgs[VIO_R].mask) >>
> +			  vio_dbgs[VIO_R].start_bit) == 1;
> +	vio_info->vio_addr_high = (dbg0 & vio_dbgs[ADDR_H].mask) >>
> +				  vio_dbgs[ADDR_H].start_bit;
> +
> +	devapc_vio_info_print(devapc_ctx);
> +}
> +
> +/*
> + * mtk_devapc_dump_vio_dbg - shift & dump the violation debug information.
> + */
> +static bool mtk_devapc_dump_vio_dbg(struct mtk_devapc_context *devapc_ctx,
> +				    int slave_type, int *vio_idx)
> +{
> +	const struct mtk_device_info **device_info;
> +	u32 shift_bit;
> +	int i;
> +
> +	device_info = devapc_ctx->device_info;
> +
> +	for (i = 0; i < get_vio_slave_num(slave_type); i++) {
> +		*vio_idx = device_info[slave_type][i].vio_index;
> +
> +		if (check_vio_mask(devapc_ctx, slave_type, *vio_idx))
> +			continue;
> +
> +		if (!check_vio_status(devapc_ctx, slave_type, *vio_idx))
> +			continue;
> +
> +		shift_bit = get_shift_group(devapc_ctx, slave_type, *vio_idx);
> +
> +		if (!sync_vio_dbg(devapc_ctx, slave_type, shift_bit))
> +			continue;
> +
> +		devapc_extract_vio_dbg(devapc_ctx, slave_type);
> +
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +/*
> + * devapc_violation_irq - the devapc Interrupt Service Routine (ISR) will dump
> + *			  violation information including which master violates
> + *			  access slave.
> + */
> +static irqreturn_t devapc_violation_irq(int irq_number,
> +					struct mtk_devapc_context *devapc_ctx)
> +{
> +	const struct mtk_device_info **device_info;
> +	int slave_type_num;
> +	int vio_idx = -1;
> +	int slave_type;
> +
> +	slave_type_num = devapc_ctx->slave_type_num;
> +	device_info = devapc_ctx->device_info;
> +
> +	for (slave_type = 0; slave_type < slave_type_num; slave_type++) {
> +		if (!mtk_devapc_dump_vio_dbg(devapc_ctx, slave_type, &vio_idx))
> +			continue;
> +
> +		/* Ensure that violation info are written before
> +		 * further operations
> +		 */
> +		smp_mb();
> +
> +		mask_module_irq(devapc_ctx, slave_type, vio_idx, true);
> +
> +		clear_vio_status(devapc_ctx, slave_type, vio_idx);
> +
> +		mask_module_irq(devapc_ctx, slave_type, vio_idx, false);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/*
> + * start_devapc - initialize devapc status and start receiving interrupt
> + *		  while devapc violation is triggered.
> + */
> +static void start_devapc(struct mtk_devapc_context *devapc_ctx)
> +{
> +	const struct mtk_device_info **device_info;
> +	void __iomem *pd_vio_shift_sta_reg;
> +	void __iomem *pd_apc_con_reg;
> +	u32 vio_shift_sta;
> +	int slave_type, slave_type_num;
> +	int i, vio_idx;
> +
> +	device_info = devapc_ctx->device_info;
> +	slave_type_num = devapc_ctx->slave_type_num;
> +
> +	for (slave_type = 0; slave_type < slave_type_num; slave_type++) {
> +		pd_apc_con_reg = mtk_devapc_pd_get(devapc_ctx, slave_type,
> +						   APC_CON, 0);
> +		pd_vio_shift_sta_reg = mtk_devapc_pd_get(devapc_ctx, slave_type,
> +							 VIO_SHIFT_STA, 0);
> +		if (!pd_apc_con_reg || !pd_vio_shift_sta_reg)
> +			return;
> +
> +		/* Clear devapc violation status */
> +		writel(BIT(31), pd_apc_con_reg);
> +
> +		/* Clear violation shift status */
> +		vio_shift_sta = readl(pd_vio_shift_sta_reg);
> +		if (vio_shift_sta)
> +			writel(vio_shift_sta, pd_vio_shift_sta_reg);
> +
> +		/* Clear slave violation status */
> +		for (i = 0; i < get_vio_slave_num(slave_type); i++) {
> +			vio_idx = device_info[slave_type][i].vio_index;
> +
> +			clear_vio_status(devapc_ctx, slave_type, vio_idx);
> +
> +			mask_module_irq(devapc_ctx, slave_type, vio_idx, false);
> +		}
> +	}
> +}
> +
> +static int mtk_devapc_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct mtk_devapc_context *devapc_ctx;
> +	struct clk *devapc_infra_clk;
> +	u32 vio_dbgs_num, pds_num;
> +	u8 slave_type_num;
> +	u32 devapc_irq;
> +	size_t size;
> +	int i, ret;
> +
> +	if (IS_ERR(node))
> +		return -ENODEV;
> +
> +	devapc_ctx = devm_kzalloc(&pdev->dev, sizeof(struct mtk_devapc_context),
> +				  GFP_KERNEL);
> +	if (!devapc_ctx)
> +		return -ENOMEM;
> +
> +	if (of_property_read_u8(node, "mediatek-slv_type_num", &slave_type_num))
> +		return -ENXIO;
> +
> +	devapc_ctx->slave_type_num = slave_type_num;
> +
> +	size = slave_type_num * sizeof(void *);
> +	devapc_ctx->devapc_pd_base = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> +	if (!devapc_ctx->devapc_pd_base)
> +		return -ENOMEM;
> +
> +	size = slave_type_num * sizeof(struct mtk_device_info *);
> +	devapc_ctx->device_info = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> +	if (!devapc_ctx->device_info)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < slave_type_num; i++) {
> +		devapc_ctx->devapc_pd_base[i] = of_iomap(node, i);
> +		if (!devapc_ctx->devapc_pd_base[i])
> +			return -EINVAL;
> +
> +		if (i == 0)
> +			devapc_ctx->device_info[i] = mtk_devices_infra;
> +	}
> +
> +	size = sizeof(struct mtk_devapc_vio_info);
> +	devapc_ctx->vio_info = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> +	if (!devapc_ctx->vio_info)
> +		return -ENOMEM;
> +
> +	vio_dbgs_num = of_property_count_u32_elems(node, "mediatek-vio_dbgs");
> +	if (vio_dbgs_num <= 0)
> +		return -ENXIO;
> +
> +	size = (vio_dbgs_num / 2) * sizeof(struct mtk_devapc_vio_dbgs_desc);
> +	devapc_ctx->vio_dbgs_desc = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> +	if (!devapc_ctx->vio_dbgs_desc)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < vio_dbgs_num / 2; i++) {
> +		if (of_property_read_u32_index(node, "mediatek-vio_dbgs",
> +					       i * 2,
> +					       &devapc_ctx->vio_dbgs_desc[i].mask))
> +			return -ENXIO;
> +
> +		if (of_property_read_u32_index(node, "mediatek-vio_dbgs",
> +					       (i * 2) + 1,
> +					       &devapc_ctx->vio_dbgs_desc[i].start_bit))
> +			return -ENXIO;
> +	}
> +
> +	pds_num = of_property_count_u32_elems(node, "mediatek-pds_offset");
> +	if (pds_num <= 0)
> +		return -ENXIO;
> +
> +	size = pds_num * sizeof(u32);
> +	devapc_ctx->pds_offset = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> +	if (!devapc_ctx->pds_offset)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < pds_num; i++) {
> +		if (of_property_read_u32_index(node, "mediatek-pds_offset", i,
> +					       &devapc_ctx->pds_offset[i]))
> +			return -ENXIO;
> +	}
> +
> +	devapc_irq = irq_of_parse_and_map(node, 0);
> +	if (!devapc_irq)
> +		return -EINVAL;
> +
> +	devapc_infra_clk = devm_clk_get(&pdev->dev, "devapc-infra-clock");
> +	if (IS_ERR(devapc_infra_clk))
> +		return -EINVAL;
> +
> +	if (clk_prepare_enable(devapc_infra_clk))
> +		return -EINVAL;
> +
> +	start_devapc(devapc_ctx);
> +
> +	ret = devm_request_irq(&pdev->dev, devapc_irq,
> +			       (irq_handler_t)devapc_violation_irq,
> +			       IRQF_TRIGGER_NONE, "devapc", devapc_ctx);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int mtk_devapc_remove(struct platform_device *dev)
> +{
> +	return 0;
> +}
> +
> +static const struct of_device_id mtk_devapc_dt_match[] = {
> +	{ .compatible = "mediatek,mt6779-devapc" },
> +	{},
> +};
> +
> +static struct platform_driver mtk_devapc_driver = {
> +	.probe = mtk_devapc_probe,
> +	.remove = mtk_devapc_remove,
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = mtk_devapc_dt_match,
> +	},
> +};
> +
> +module_platform_driver(mtk_devapc_driver);
> +
> +MODULE_DESCRIPTION("Mediatek Device APC Driver");
> +MODULE_AUTHOR("Neal Liu <neal.liu@mediatek.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/soc/mediatek/mtk-devapc.h b/drivers/soc/mediatek/mtk-devapc.h
> new file mode 100644
> index 0000000..ab2cb14
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-devapc.h
> @@ -0,0 +1,670 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2020 MediaTek Inc.
> + */
> +
> +#ifndef __MTK_DEVAPC_H__
> +#define __MTK_DEVAPC_H__
> +
> +#define PFX			"[DEVAPC]: "

use dev_err() and friends instead.

> +
> +#define VIO_MASK_STA_REG_GET(m) \
> +({ \
> +	typeof(m) (_m) = (m); \
> +	reg_index = _m / 32; \
> +	reg_offset = _m % 32; \
> +})

don't do that. no explicit variable assingment in a macro, the macro should 
return the value.

> +
> +enum DEVAPC_PD_REG_TYPE {
> +	VIO_MASK = 0,
> +	VIO_STA,
> +	VIO_DBG0,
> +	VIO_DBG1,
> +	APC_CON,
> +	VIO_SHIFT_STA,
> +	VIO_SHIFT_SEL,
> +	VIO_SHIFT_CON,
> +	PD_REG_TYPE_NUM,
> +};
> +
> +enum DEVAPC_VIO_DBGS_TYPE {
> +	MSTID = 0,
> +	DMNID,
> +	VIO_W,
> +	VIO_R,
> +	ADDR_H,
> +};
> +
> +struct mtk_device_info {
> +	int sys_index;
> +	int ctrl_index;
> +	int vio_index;
> +};
> +
> +static struct mtk_device_info mtk_devices_infra[] = {

That's for mt6779, correct? Should be stated in the name.

> +	/* sys_idx, ctrl_idx, vio_idx */
> +	/* 0 */
> +	{0, 0, 0},
> +	{0, 1, 1},
> +	{0, 2, 2},
> +	{0, 3, 3},
> +	{0, 4, 4},
> +	{0, 5, 5},
> +	{0, 6, 6},
> +	{0, 7, 7},
> +	{0, 8, 8},
> +	{0, 9, 9},
> +
> +	/* 10 */
> +	{0, 10, 10},
> +	{0, 11, 11},
> +	{0, 12, 12},
> +	{0, 13, 13},
> +	{0, 14, 14},
> +	{0, 15, 15},
> +	{0, 16, 16},
> +	{0, 17, 17},
> +	{0, 18, 18},
> +	{0, 19, 19},
> +
> +	/* 20 */
> +	{0, 20, 20},
> +	{0, 21, 21},
> +	{0, 22, 22},
> +	{0, 23, 23},
> +	{0, 24, 24},
> +	{0, 25, 25},
> +	{0, 26, 26},
> +	{0, 27, 27},
> +	{0, 28, 28},
> +	{0, 29, 29},
> +
> +	/* 30 */
> +	{0, 30, 30},
> +	{0, 31, 31},
> +	{0, 32, 32},
> +	{0, 33, 77},
> +	{0, 34, 78},
> +	{0, 35, 79},
> +	{0, 35, 80},
> +	{0, 37, 37},
> +	{0, 38, 38},
> +	{0, 39, 39},
> +
> +	/* 40 */
> +	{0, 40, 40},
> +	{0, 41, 41},
> +	{0, 42, 42},
> +	{0, 43, 43},
> +	{0, 44, 44},
> +	{0, 45, 45},
> +	{0, 46, 46},
> +	{0, 47, 47},
> +	{0, 48, 48},
> +	{0, 49, 49},
> +
> +	/* 50 */
> +	{0, 50, 50},
> +	{0, 51, 51},
> +	{0, 52, 52},
> +	{0, 53, 53},
> +	{0, 54, 54},
> +	{0, 55, 55},
> +	{0, 56, 56},
> +	{0, 57, 57},
> +	{0, 58, 58},
> +	{0, 59, 59},
> +
> +	/* 60 */
> +	{0, 60, 60},
> +	{0, 61, 61},
> +	{0, 62, 62},
> +	{0, 63, 63},
> +	{0, 64, 64},
> +	{0, 65, 70},
> +	{0, 66, 71},
> +	{0, 67, 72},
> +	{0, 68, 73},
> +	{0, 70, 81},
> +
> +	/* 70 */
> +	{0, 71, 82},
> +	{0, 72, 83},
> +	{0, 73, 84},
> +	{0, 74, 85},
> +	{0, 75, 86},
> +	{0, 76, 87},
> +	{0, 77, 88},
> +	{0, 78, 89},
> +	{0, 79, 90},
> +	{0, 80, 91},
> +
> +	/* 80 */
> +	{0, 81, 92},
> +	{0, 82, 93},
> +	{0, 83, 94},
> +	{0, 84, 95},
> +	{0, 85, 96},
> +	{0, 86, 97},
> +	{0, 87, 98},
> +	{0, 88, 99},
> +	{0, 89, 100},
> +	{0, 90, 101},
> +
> +	/* 90 */
> +	{0, 91, 102},
> +	{0, 92, 103},
> +	{0, 93, 104},
> +	{0, 94, 105},
> +	{0, 95, 106},
> +	{0, 96, 107},
> +	{0, 97, 108},
> +	{0, 98, 109},
> +	{0, 110, 110},
> +	{0, 111, 111},
> +
> +	/* 100 */
> +	{0, 112, 112},
> +	{0, 113, 113},
> +	{0, 114, 114},
> +	{0, 115, 115},
> +	{0, 116, 116},
> +	{0, 117, 117},
> +	{0, 118, 118},
> +	{0, 119, 119},
> +	{0, 120, 120},
> +	{0, 121, 121},
> +
> +	/* 110 */
> +	{0, 122, 122},
> +	{0, 123, 123},
> +	{0, 124, 124},
> +	{0, 125, 125},
> +	{0, 126, 126},
> +	{0, 127, 127},
> +	{0, 128, 128},
> +	{0, 129, 129},
> +	{0, 130, 130},
> +	{0, 131, 131},
> +
> +	/* 120 */
> +	{0, 132, 132},
> +	{0, 133, 133},
> +	{0, 134, 134},
> +	{0, 135, 135},
> +	{0, 136, 136},
> +	{0, 137, 137},
> +	{0, 138, 138},
> +	{0, 139, 139},
> +	{0, 140, 140},
> +	{0, 141, 141},
> +
> +	/* 130 */
> +	{0, 142, 142},
> +	{0, 143, 143},
> +	{0, 144, 144},
> +	{0, 145, 145},
> +	{0, 146, 146},
> +	{0, 147, 147},
> +	{0, 148, 148},
> +	{0, 149, 149},
> +	{0, 150, 150},
> +	{0, 151, 151},
> +
> +	/* 140 */
> +	{0, 152, 152},
> +	{0, 153, 153},
> +	{0, 154, 154},
> +	{0, 155, 155},
> +	{0, 156, 156},
> +	{0, 157, 157},
> +	{0, 158, 158},
> +	{0, 159, 159},
> +	{0, 160, 160},
> +	{0, 161, 161},
> +
> +	/* 150 */
> +	{0, 162, 162},
> +	{0, 163, 163},
> +	{0, 164, 164},
> +	{0, 165, 165},
> +	{0, 166, 166},
> +	{0, 167, 167},
> +	{0, 168, 168},
> +	{0, 169, 169},
> +	{0, 170, 170},
> +	{0, 171, 171},
> +
> +	/* 160 */
> +	{0, 172, 172},
> +	{0, 173, 173},
> +	{0, 174, 174},
> +	{0, 175, 175},
> +	{0, 176, 176},
> +	{0, 177, 177},
> +	{0, 178, 178},
> +	{0, 179, 179},
> +	{0, 180, 180},
> +	{0, 181, 181},
> +
> +	/* 170 */
> +	{0, 182, 182},
> +	{0, 183, 183},
> +	{0, 184, 184},
> +	{0, 185, 185},
> +	{0, 186, 186},
> +	{0, 187, 187},
> +	{0, 188, 188},
> +	{0, 189, 189},
> +	{0, 190, 190},
> +	{0, 191, 191},
> +
> +	/* 180 */
> +	{0, 192, 192},
> +	{0, 193, 193},
> +	{0, 194, 194},
> +	{0, 195, 195},
> +	{0, 196, 196},
> +	{0, 197, 197},
> +	{0, 198, 198},
> +	{0, 199, 199},
> +	{0, 200, 200},
> +	{0, 201, 201},
> +
> +	/* 190 */
> +	{0, 202, 202},
> +	{0, 203, 203},
> +	{0, 204, 204},
> +	{0, 205, 205},
> +	{0, 206, 206},
> +	{0, 207, 207},
> +	{0, 208, 208},
> +	{0, 209, 209},
> +	{0, 210, 210},
> +	{0, 211, 211},
> +
> +	/* 200 */
> +	{0, 212, 212},
> +	{0, 213, 213},
> +	{0, 214, 214},
> +	{0, 215, 215},
> +	{0, 216, 216},
> +	{0, 217, 217},
> +	{0, 218, 218},
> +	{0, 219, 219},
> +	{0, 220, 220},
> +	{0, 221, 221},
> +
> +	/* 210 */
> +	{0, 222, 222},
> +	{0, 223, 223},
> +	{0, 224, 224},
> +	{0, 225, 225},
> +	{0, 226, 226},
> +	{0, 227, 227},
> +	{0, 228, 228},
> +	{0, 229, 229},
> +	{0, 230, 230},
> +	{0, 231, 231},
> +
> +	/* 220 */
> +	{1, 0, 232},
> +	{1, 1, 233},
> +	{1, 2, 234},
> +	{1, 3, 235},
> +	{1, 4, 236},
> +	{1, 5, 237},
> +	{1, 6, 238},
> +	{1, 7, 239},
> +	{1, 8, 240},
> +	{1, 9, 241},
> +
> +	/* 230 */
> +	{1, 10, 242},
> +	{1, 11, 243},
> +	{1, 12, 244},
> +	{1, 13, 245},
> +	{1, 14, 246},
> +	{1, 15, 247},
> +	{1, 16, 248},
> +	{1, 17, 249},
> +	{1, 18, 250},
> +	{1, 19, 251},
> +
> +	/* 240 */
> +	{1, 20, 252},
> +	{1, 21, 253},
> +	{1, 22, 254},
> +	{1, 23, 255},
> +	{1, 24, 256},
> +	{1, 25, 257},
> +	{1, 26, 258},
> +	{1, 27, 259},
> +	{1, 28, 260},
> +	{1, 29, 261},
> +
> +	/* 250 */
> +	{1, 30, 262},
> +	{1, 31, 263},
> +	{1, 32, 264},
> +	{1, 33, 265},
> +	{1, 34, 266},
> +	{1, 35, 267},
> +	{1, 36, 268},
> +	{1, 37, 269},
> +	{1, 38, 270},
> +	{1, 39, 271},
> +
> +	/* 260 */
> +	{1, 40, 272},
> +	{1, 41, 273},
> +	{1, 42, 274},
> +	{1, 43, 275},
> +	{1, 44, 276},
> +	{1, 45, 277},
> +	{1, 46, 278},
> +	{1, 47, 279},
> +	{1, 48, 280},
> +	{1, 49, 281},
> +
> +	/* 270 */
> +	{1, 50, 282},
> +	{1, 51, 283},
> +	{1, 52, 284},
> +	{1, 53, 285},
> +	{1, 54, 286},
> +	{1, 55, 287},
> +	{1, 56, 288},
> +	{1, 57, 289},
> +	{1, 58, 290},
> +	{1, 59, 291},
> +
> +	/* 280 */
> +	{1, 60, 292},
> +	{1, 61, 293},
> +	{1, 62, 294},
> +	{1, 63, 295},
> +	{1, 64, 296},
> +	{1, 65, 297},
> +	{1, 66, 298},
> +	{1, 67, 299},
> +	{1, 68, 300},
> +	{1, 69, 301},
> +
> +	/* 290 */
> +	{1, 70, 302},
> +	{1, 71, 303},
> +	{1, 72, 304},
> +	{1, 73, 305},
> +	{1, 74, 306},
> +	{1, 75, 307},
> +	{1, 76, 308},
> +	{1, 77, 309},
> +	{1, 78, 310},
> +	{1, 79, 311},
> +
> +	/* 300 */
> +	{1, 80, 312},
> +	{1, 81, 313},
> +	{1, 82, 314},
> +	{1, 83, 315},
> +	{1, 84, 316},
> +	{1, 85, 317},
> +	{1, 86, 318},
> +	{1, 87, 319},
> +	{1, 88, 320},
> +	{1, 89, 321},
> +
> +	/* 310 */
> +	{1, 90, 322},
> +	{1, 91, 323},
> +	{1, 92, 324},
> +	{1, 93, 325},
> +	{1, 94, 326},
> +	{1, 95, 327},
> +	{1, 96, 328},
> +	{1, 97, 329},
> +	{1, 98, 330},
> +	{1, 99, 331},
> +
> +	/* 320 */
> +	{1, 100, 332},
> +	{1, 101, 333},
> +	{1, 102, 334},
> +	{1, 103, 335},
> +	{1, 104, 336},
> +	{1, 105, 337},
> +	{1, 106, 338},
> +	{1, 107, 339},
> +	{1, 108, 340},
> +	{1, 109, 341},
> +
> +	/* 330 */
> +	{1, 110, 342},
> +	{1, 111, 343},
> +	{1, 112, 344},
> +	{1, 113, 345},
> +	{1, 114, 346},
> +	{1, 115, 347},
> +	{1, 116, 348},
> +	{1, 117, 349},
> +	{1, 118, 350},
> +	{1, 119, 351},
> +
> +	/* 340 */
> +	{1, 120, 352},
> +	{1, 121, 353},
> +	{1, 122, 354},
> +	{1, 123, 355},
> +	{1, 124, 356},
> +	{1, 125, 357},
> +	{1, 126, 358},
> +	{1, 127, 359},
> +	{1, 128, 360},
> +	{1, 129, 361},
> +
> +	/* 350 */
> +	{1, 130, 362},
> +	{1, 131, 363},
> +	{1, 132, 364},
> +	{1, 133, 365},
> +	{1, 134, 366},
> +	{1, 135, 367},
> +	{1, 136, 368},
> +	{1, 137, 369},
> +	{1, 138, 370},
> +	{1, 139, 371},
> +
> +	/* 360 */
> +	{1, 140, 372},
> +	{1, 141, 373},
> +	{1, 142, 374},
> +	{1, 143, 375},
> +	{1, 144, 376},
> +	{1, 145, 377},
> +	{1, 146, 378},
> +	{1, 147, 379},
> +	{1, 148, 380},
> +	{1, 149, 381},
> +
> +	/* 370 */
> +	{1, 150, 382},
> +	{1, 151, 383},
> +	{1, 152, 384},
> +	{1, 153, 385},
> +	{1, 154, 386},
> +	{1, 155, 387},
> +	{1, 156, 388},
> +	{1, 157, 389},
> +	{1, 158, 390},
> +	{1, 159, 391},
> +
> +	/* 380 */
> +	{1, 160, 392},
> +	{1, 161, 393},
> +	{1, 162, 394},
> +	{1, 163, 395},
> +	{1, 164, 396},
> +	{1, 165, 397},
> +	{1, 166, 398},
> +	{1, 167, 399},
> +	{1, 168, 400},
> +	{1, 169, 401},
> +
> +	/* 390 */
> +	{1, 170, 402},
> +	{1, 171, 403},
> +	{1, 172, 404},
> +	{1, 173, 405},
> +	{1, 174, 406},
> +	{1, 175, 407},
> +	{1, 176, 408},
> +	{1, 177, 409},
> +	{1, 178, 410},
> +	{1, 179, 411},
> +
> +	/* 400 */
> +	{1, 180, 412},
> +	{1, 181, 413},
> +	{1, 182, 414},
> +	{1, 183, 415},
> +	{1, 184, 416},
> +	{1, 185, 417},
> +	{1, 186, 418},
> +	{1, 187, 419},
> +	{1, 188, 420},
> +	{1, 189, 421},
> +
> +	/* 410 */
> +	{1, 190, 422},
> +	{1, 191, 423},
> +	{1, 192, 424},
> +	{1, 193, 425},
> +	{1, 194, 426},
> +	{1, 195, 427},
> +	{1, 196, 428},
> +	{1, 197, 429},
> +	{1, 198, 430},
> +	{1, 199, 431},
> +
> +	/* 420 */
> +	{1, 200, 432},
> +	{1, 201, 433},
> +	{1, 202, 434},
> +	{1, 203, 435},
> +	{1, 204, 436},
> +	{1, 205, 437},
> +	{1, 206, 438},
> +	{1, 207, 439},
> +	{1, 208, 440},
> +	{1, 209, 441},
> +
> +	/* 430 */
> +	{1, 210, 442},
> +	{1, 211, 443},
> +	{1, 212, 444},
> +	{1, 213, 445},
> +	{1, 214, 446},
> +	{1, 215, 447},
> +	{1, 216, 448},
> +	{1, 217, 449},
> +	{1, 218, 450},
> +	{1, 219, 451},
> +
> +	/* 440 */
> +	{1, 220, 452},
> +	{1, 221, 453},
> +	{1, 222, 454},
> +	{1, 223, 455},
> +	{1, 224, 456},
> +	{1, 225, 457},
> +	{1, 226, 458},
> +	{1, 227, 459},
> +	{1, 228, 460},
> +	{1, 229, 461},
> +
> +	/* 450 */
> +	{1, 230, 462},
> +	{1, 231, 463},
> +	{1, 232, 464},
> +	{1, 233, 465},
> +	{1, 234, 466},
> +	{1, 235, 467},
> +	{1, 236, 468},
> +	{1, 237, 469},
> +	{1, 238, 470},
> +	{1, 239, 471},
> +
> +	/* 460 */
> +	{1, 240, 472},
> +	{1, 241, 473},
> +	{1, 242, 474},
> +	{1, 243, 475},
> +	{1, 244, 476},
> +	{1, 245, 477},
> +	{1, 246, 478},
> +	{-1, -1, 479},
> +	{-1, -1, 480},
> +	{-1, -1, 481},
> +
> +	/* 470 */
> +	{-1, -1, 482},
> +	{-1, -1, 483},
> +	{-1, -1, 484},
> +	{-1, -1, 485},
> +	{-1, -1, 486},
> +	{-1, -1, 487},
> +	{-1, -1, 488},
> +	{-1, -1, 489},
> +	{-1, -1, 490},
> +	{-1, -1, 491},
> +
> +	/* 480 */
> +	{-1, -1, 492},
> +	{-1, -1, 493},
> +	{-1, -1, 494},
> +	{-1, -1, 495},
> +	{-1, -1, 496},
> +	{-1, -1, 497},
> +	{-1, -1, 498},
> +	{-1, -1, 499},
> +	{-1, -1, 500},
> +	{-1, -1, 501},
> +
> +	/* 490 */
> +	{-1, -1, 502},
> +	{-1, -1, 503},
> +	{-1, -1, 504},
> +	{-1, -1, 505},
> +	{-1, -1, 506},
> +	{-1, -1, 507},
> +	{-1, -1, 508},
> +	{-1, -1, 509},
> +	{-1, -1, 510},
> +
> +};
> +
> +struct mtk_devapc_vio_info {
> +	bool read;
> +	bool write;
> +	u32 vio_addr;
> +	u32 vio_addr_high;
> +	u32 master_id;
> +	u32 domain_id;
> +};
> +
> +struct mtk_devapc_vio_dbgs_desc {
> +	u32 mask;
> +	u32 start_bit;
> +};
> +
> +struct mtk_devapc_context {
> +	u8 slave_type_num;
> +	void __iomem **devapc_pd_base;
> +	const struct mtk_device_info **device_info;
> +	struct mtk_devapc_vio_info *vio_info;
> +	struct mtk_devapc_vio_dbgs_desc *vio_dbgs_desc;
> +	u32 *pds_offset;
> +};
> +

Not sure if I get this right:

struct mtk_devapc_offset {
	u32 vio_mask;
	u32 vio_sta;
	u32 vio_dbg0;
	u32 vio_dbg1;
	...
}

struct mtk_devapc_context {
	u8 pd_base_num;
	void __iomem **devapc_pd_base;
	struct mtk_devapc_offset *offset;
	const struct mtk_device_info **device_info;
	struct mtk_devapc_vio_info *vio_info;
	struct mtk_devapc_vio_dbgs_desc *vio_dbgs_desc;
};

With this I think we can get rid of mtk_devapc_pd_get().

Sorry I'm not able to review the whole driver right now. Please also have a look 
on my comments from v1.

We will have to go little by little to get this into a good state. In case it 
makes sense to have this in the kernel at all.

Regards,
Matthias