[PATCH 1/2] dt-bindings: devapc: Update bindings

[PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

From: Nina Wu <Nina-CM.Wu@mediatek.com>

To support newer hardware architecture of devapc,
update device tree bindings.

Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
---
 .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
index 31e4d3c..489f6a9 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
@@ -20,9 +20,27 @@ properties:
   compatible:
     enum:
       - mediatek,mt6779-devapc
+      - mediatek,mt8192-devapc
+
+  version:
+    description: The version of the hardware architecture
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+    maxItems: 1
+
+  slave_type_num:
+    description: The number of the devapc set
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 4]
+    maxItems: 1
 
   reg:
     description: The base address of devapc register bank
+    maxItems: 4
+
+  vio_idx_num:
+    description: The number of the devices controlled by devapc
+    $ref: /schemas/types.yaml#/definitions/uint32-array
     maxItems: 1
 
   interrupts:
@@ -39,7 +57,10 @@ properties:
 
 required:
   - compatible
+  - version
+  - slave_type_num
   - reg
+  - vio_idx_num
   - interrupts
   - clocks
   - clock-names
@@ -53,8 +74,28 @@ examples:
 
     devapc: devapc@10207000 {
       compatible = "mediatek,mt6779-devapc";
+      version = <1>;
+      slave_type_num = <1>;
       reg = <0x10207000 0x1000>;
+      vio_idx_num = <511>;
       interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
       clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
       clock-names = "devapc-infra-clock";
     };
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/mt8192-clk.h>
+
+    devapc: devapc@10207000 {
+        compatible = "mediatek,mt8192-devapc";
+        version = <2>;
+        slave_type_num = <4>;
+        reg = <0 0x10207000 0 0x1000>,
+            <0 0x10274000 0 0x1000>,
+            <0 0x10275000 0 0x1000>,
+            <0 0x11020000 0 0x1000>;
+        vio_idx_num = <367 292 242 58>;
+        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
+        clock-names = "devapc-infra-clock";
+    };
-- 
2.6.4

[PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

From: Nina Wu <Nina-CM.Wu@mediatek.com>

To support newer hardware architecture of devapc,
update device tree bindings.

Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
---
 .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
index 31e4d3c..489f6a9 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
@@ -20,9 +20,27 @@ properties:
   compatible:
     enum:
       - mediatek,mt6779-devapc
+      - mediatek,mt8192-devapc
+
+  version:
+    description: The version of the hardware architecture
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+    maxItems: 1
+
+  slave_type_num:
+    description: The number of the devapc set
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 4]
+    maxItems: 1
 
   reg:
     description: The base address of devapc register bank
+    maxItems: 4
+
+  vio_idx_num:
+    description: The number of the devices controlled by devapc
+    $ref: /schemas/types.yaml#/definitions/uint32-array
     maxItems: 1
 
   interrupts:
@@ -39,7 +57,10 @@ properties:
 
 required:
   - compatible
+  - version
+  - slave_type_num
   - reg
+  - vio_idx_num
   - interrupts
   - clocks
   - clock-names
@@ -53,8 +74,28 @@ examples:
 
     devapc: devapc@10207000 {
       compatible = "mediatek,mt6779-devapc";
+      version = <1>;
+      slave_type_num = <1>;
       reg = <0x10207000 0x1000>;
+      vio_idx_num = <511>;
       interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
       clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
       clock-names = "devapc-infra-clock";
     };
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/mt8192-clk.h>
+
+    devapc: devapc@10207000 {
+        compatible = "mediatek,mt8192-devapc";
+        version = <2>;
+        slave_type_num = <4>;
+        reg = <0 0x10207000 0 0x1000>,
+            <0 0x10274000 0 0x1000>,
+            <0 0x10275000 0 0x1000>,
+            <0 0x11020000 0 0x1000>;
+        vio_idx_num = <367 292 242 58>;
+        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
+        clock-names = "devapc-infra-clock";
+    };
-- 
2.6.4
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

From: Nina Wu <Nina-CM.Wu@mediatek.com>

To support newer hardware architecture of devapc,
update device tree bindings.

Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
---
 .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
index 31e4d3c..489f6a9 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
@@ -20,9 +20,27 @@ properties:
   compatible:
     enum:
       - mediatek,mt6779-devapc
+      - mediatek,mt8192-devapc
+
+  version:
+    description: The version of the hardware architecture
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+    maxItems: 1
+
+  slave_type_num:
+    description: The number of the devapc set
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 4]
+    maxItems: 1
 
   reg:
     description: The base address of devapc register bank
+    maxItems: 4
+
+  vio_idx_num:
+    description: The number of the devices controlled by devapc
+    $ref: /schemas/types.yaml#/definitions/uint32-array
     maxItems: 1
 
   interrupts:
@@ -39,7 +57,10 @@ properties:
 
 required:
   - compatible
+  - version
+  - slave_type_num
   - reg
+  - vio_idx_num
   - interrupts
   - clocks
   - clock-names
@@ -53,8 +74,28 @@ examples:
 
     devapc: devapc@10207000 {
       compatible = "mediatek,mt6779-devapc";
+      version = <1>;
+      slave_type_num = <1>;
       reg = <0x10207000 0x1000>;
+      vio_idx_num = <511>;
       interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
       clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
       clock-names = "devapc-infra-clock";
     };
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/mt8192-clk.h>
+
+    devapc: devapc@10207000 {
+        compatible = "mediatek,mt8192-devapc";
+        version = <2>;
+        slave_type_num = <4>;
+        reg = <0 0x10207000 0 0x1000>,
+            <0 0x10274000 0 0x1000>,
+            <0 0x10275000 0 0x1000>,
+            <0 0x11020000 0 0x1000>;
+        vio_idx_num = <367 292 242 58>;
+        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
+        clock-names = "devapc-infra-clock";
+    };
-- 
2.6.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Nina Wu]

From: Nina Wu <Nina-CM.Wu@mediatek.com>

The hardware architecture of mt8192 devapc is slightly
different from that of the previous IC.
We add necessary extensions to support mt8192 and be
back-compatible with other ICs.

Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
---
 drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
 1 file changed, 156 insertions(+), 57 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
index f1cea04..1e40a52 100644
--- a/drivers/soc/mediatek/mtk-devapc.c
+++ b/drivers/soc/mediatek/mtk-devapc.c
@@ -15,6 +15,11 @@
 #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
 #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
 
+#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
+	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)
+#define BASE(i)			(ctx->base_list[i])
+#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])
+
 struct mtk_devapc_vio_dbgs {
 	union {
 		u32 vio_dbg0;
@@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
 			u32 addr_h:4;
 			u32 resv:4;
 		} dbg0_bits;
+
+		/* Not used, reference only */
+		struct {
+			u32 dmnid:6;
+			u32 vio_w:1;
+			u32 vio_r:1;
+			u32 addr_h:4;
+			u32 resv:20;
+		} dbg0_bits_ver2;
 	};
 
 	u32 vio_dbg1;
+	u32 vio_dbg2;
 };
 
 struct mtk_devapc_data {
-	/* numbers of violation index */
-	u32 vio_idx_num;
-
 	/* reg offset */
 	u32 vio_mask_offset;
 	u32 vio_sta_offset;
 	u32 vio_dbg0_offset;
 	u32 vio_dbg1_offset;
+	u32 vio_dbg2_offset;
 	u32 apc_con_offset;
 	u32 vio_shift_sta_offset;
 	u32 vio_shift_sel_offset;
@@ -48,7 +61,10 @@ struct mtk_devapc_data {
 
 struct mtk_devapc_context {
 	struct device *dev;
-	void __iomem *infra_base;
+	u32 arch_ver;
+	u32 slave_type_num;
+	void __iomem **base_list;
+	u32 *vio_idx_num;
 	struct clk *infra_clk;
 	const struct mtk_devapc_data *data;
 };
@@ -56,39 +72,39 @@ struct mtk_devapc_context {
 static void clear_vio_status(struct mtk_devapc_context *ctx)
 {
 	void __iomem *reg;
-	int i;
+	int i, j;
 
-	reg = ctx->infra_base + ctx->data->vio_sta_offset;
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg = BASE(i) + ctx->data->vio_sta_offset;
 
-	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
-		writel(GENMASK(31, 0), reg + 4 * i);
+		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
+			writel(GENMASK(31, 0), reg + 4 * j);
+
+		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
+		       reg + 4 * j);
+	}
 
-	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
-	       reg + 4 * i);
 }
 
-static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
+static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
 {
-	void __iomem *reg;
 	u32 val;
 	int i;
 
-	reg = ctx->infra_base + ctx->data->vio_mask_offset;
-
 	if (mask)
 		val = GENMASK(31, 0);
 	else
 		val = 0;
 
-	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
+	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
 		writel(val, reg + 4 * i);
 
 	val = readl(reg + 4 * i);
 	if (mask)
-		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
+		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
 			       0);
 	else
-		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
+		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
 				0);
 
 	writel(val, reg + 4 * i);
@@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
  */
 static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
 {
+	int i;
+	void __iomem *reg_base;
 	void __iomem *pd_vio_shift_sta_reg;
 	void __iomem *pd_vio_shift_sel_reg;
 	void __iomem *pd_vio_shift_con_reg;
@@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
 	int ret;
 	u32 val;
 
-	pd_vio_shift_sta_reg = ctx->infra_base +
-			       ctx->data->vio_shift_sta_offset;
-	pd_vio_shift_sel_reg = ctx->infra_base +
-			       ctx->data->vio_shift_sel_offset;
-	pd_vio_shift_con_reg = ctx->infra_base +
-			       ctx->data->vio_shift_con_offset;
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg_base = BASE(i);
 
-	/* Find the minimum shift group which has violation */
-	val = readl(pd_vio_shift_sta_reg);
-	if (!val)
-		return false;
+		pd_vio_shift_sta_reg = reg_base +
+				       ctx->data->vio_shift_sta_offset;
+		pd_vio_shift_sel_reg = reg_base +
+				       ctx->data->vio_shift_sel_offset;
+		pd_vio_shift_con_reg = reg_base +
+				       ctx->data->vio_shift_con_offset;
 
-	min_shift_group = __ffs(val);
+		/* Find the minimum shift group which has violation */
+		val = readl(pd_vio_shift_sta_reg);
+		if (!val)
+			continue;
 
-	/* Assign the group to sync */
-	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
+		min_shift_group = __ffs(val);
 
-	/* Start syncing */
-	writel(0x1, pd_vio_shift_con_reg);
+		/* Assign the group to sync */
+		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
 
-	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
-				 PHY_DEVAPC_TIMEOUT);
-	if (ret) {
-		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
-		return false;
-	}
+		/* Start syncing */
+		writel(0x1, pd_vio_shift_con_reg);
+
+		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
+					 0, PHY_DEVAPC_TIMEOUT);
+		if (ret) {
+			dev_err(ctx->dev, "%s: Shift violation info failed\n",
+				__func__);
+			return -ETIMEDOUT;
+		}
 
-	/* Stop syncing */
-	writel(0x0, pd_vio_shift_con_reg);
+		/* Stop syncing */
+		writel(0x0, pd_vio_shift_con_reg);
 
-	/* Write clear */
-	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
+		/* Write clear */
+		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
 
-	return true;
+		return i;
+	}
+
+	/* No violation found */
+	return -ENODATA;
 }
 
 /*
  * devapc_extract_vio_dbg - extract full violation information after doing
  *                          shift mechanism.
  */
-static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
+static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
+				   int vio_slave_type)
 {
 	struct mtk_devapc_vio_dbgs vio_dbgs;
 	void __iomem *vio_dbg0_reg;
 	void __iomem *vio_dbg1_reg;
+	void __iomem *vio_dbg2_reg;
+	u32 vio_addr, bus_id;
 
-	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
-	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
+	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
+	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
+	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
 
 	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
 	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
+	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
+
+	switch (ctx->arch_ver) {
+	case 1:
+		bus_id = vio_dbgs.dbg0_bits.mstid;
+		vio_addr = vio_dbgs.vio_dbg1;
+		break;
+	case 2:
+		bus_id = vio_dbgs.vio_dbg1;
+		vio_addr = vio_dbgs.vio_dbg2;
+
+		/* To align with the bit definition of arch_ver 1 */
+		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
+		break;
+	default:
+		/* Not Supported */
+		return;
+	}
 
 	/* Print violation information */
 	if (vio_dbgs.dbg0_bits.vio_w)
@@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
 		dev_info(ctx->dev, "Read Violation\n");
 
 	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
-		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
-		 vio_dbgs.vio_dbg1);
+		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
 }
 
 /*
@@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
 static irqreturn_t devapc_violation_irq(int irq_number, void *data)
 {
 	struct mtk_devapc_context *ctx = data;
+	int vio_slave_type;
 
-	while (devapc_sync_vio_dbg(ctx))
-		devapc_extract_vio_dbg(ctx);
+	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
+		devapc_extract_vio_dbg(ctx, vio_slave_type);
 
 	clear_vio_status(ctx);
 
@@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
  */
 static void start_devapc(struct mtk_devapc_context *ctx)
 {
-	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
+	int i;
+	void __iomem *reg_base;
 
-	mask_module_irq(ctx, false);
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
+
+		reg_base = BASE(i) + ctx->data->vio_mask_offset;
+		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
+	}
 }
 
 /*
@@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
  */
 static void stop_devapc(struct mtk_devapc_context *ctx)
 {
-	mask_module_irq(ctx, true);
+	int i;
+	void __iomem *reg_base;
+
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg_base = BASE(i) + ctx->data->vio_mask_offset;
+		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
 
-	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
+		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
+	}
 }
 
 static const struct mtk_devapc_data devapc_mt6779 = {
-	.vio_idx_num = 511,
 	.vio_mask_offset = 0x0,
 	.vio_sta_offset = 0x400,
 	.vio_dbg0_offset = 0x900,
@@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
 	.vio_shift_con_offset = 0xF20,
 };
 
+static const struct mtk_devapc_data devapc_mt8192 = {
+	.vio_mask_offset = 0x0,
+	.vio_sta_offset = 0x400,
+	.vio_dbg0_offset = 0x900,
+	.vio_dbg1_offset = 0x904,
+	.vio_dbg2_offset = 0x908,
+	.apc_con_offset = 0xF00,
+	.vio_shift_sta_offset = 0xF20,
+	.vio_shift_sel_offset = 0xF30,
+	.vio_shift_con_offset = 0xF10,
+};
+
 static const struct of_device_id mtk_devapc_dt_match[] = {
 	{
 		.compatible = "mediatek,mt6779-devapc",
 		.data = &devapc_mt6779,
 	}, {
+		.compatible = "mediatek,mt8192-devapc",
+		.data = &devapc_mt8192,
+	}, {
 	},
 };
 
@@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
 	struct mtk_devapc_context *ctx;
+	int i;
 	u32 devapc_irq;
 	int ret;
 
@@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
 	ctx->data = of_device_get_match_data(&pdev->dev);
 	ctx->dev = &pdev->dev;
 
-	ctx->infra_base = of_iomap(node, 0);
-	if (!ctx->infra_base)
+	if (of_property_read_u32(node, "version", &ctx->arch_ver))
+		return -EINVAL;
+
+	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))
+		return -EINVAL;
+
+	ctx->base_list = devm_kzalloc(&pdev->dev,
+				      sizeof(void *) * ctx->slave_type_num,
+				      GFP_KERNEL);
+	if (!ctx->base_list)
+		return -ENOMEM;
+
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		BASE(i) = of_iomap(node, i);
+		if (!BASE(i))
+			return -EINVAL;
+	}
+
+	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
+					sizeof(u32) * ctx->slave_type_num,
+					GFP_KERNEL);
+	if (!ctx->vio_idx_num)
+		return -ENOMEM;
+
+	if (of_property_read_u32_array(node, "vio_idx_num",
+				       ctx->vio_idx_num, ctx->slave_type_num))
 		return -EINVAL;
 
 	devapc_irq = irq_of_parse_and_map(node, 0);
-- 
2.6.4

[PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Nina Wu]

From: Nina Wu <Nina-CM.Wu@mediatek.com>

The hardware architecture of mt8192 devapc is slightly
different from that of the previous IC.
We add necessary extensions to support mt8192 and be
back-compatible with other ICs.

Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
---
 drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
 1 file changed, 156 insertions(+), 57 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
index f1cea04..1e40a52 100644
--- a/drivers/soc/mediatek/mtk-devapc.c
+++ b/drivers/soc/mediatek/mtk-devapc.c
@@ -15,6 +15,11 @@
 #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
 #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
 
+#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
+	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)
+#define BASE(i)			(ctx->base_list[i])
+#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])
+
 struct mtk_devapc_vio_dbgs {
 	union {
 		u32 vio_dbg0;
@@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
 			u32 addr_h:4;
 			u32 resv:4;
 		} dbg0_bits;
+
+		/* Not used, reference only */
+		struct {
+			u32 dmnid:6;
+			u32 vio_w:1;
+			u32 vio_r:1;
+			u32 addr_h:4;
+			u32 resv:20;
+		} dbg0_bits_ver2;
 	};
 
 	u32 vio_dbg1;
+	u32 vio_dbg2;
 };
 
 struct mtk_devapc_data {
-	/* numbers of violation index */
-	u32 vio_idx_num;
-
 	/* reg offset */
 	u32 vio_mask_offset;
 	u32 vio_sta_offset;
 	u32 vio_dbg0_offset;
 	u32 vio_dbg1_offset;
+	u32 vio_dbg2_offset;
 	u32 apc_con_offset;
 	u32 vio_shift_sta_offset;
 	u32 vio_shift_sel_offset;
@@ -48,7 +61,10 @@ struct mtk_devapc_data {
 
 struct mtk_devapc_context {
 	struct device *dev;
-	void __iomem *infra_base;
+	u32 arch_ver;
+	u32 slave_type_num;
+	void __iomem **base_list;
+	u32 *vio_idx_num;
 	struct clk *infra_clk;
 	const struct mtk_devapc_data *data;
 };
@@ -56,39 +72,39 @@ struct mtk_devapc_context {
 static void clear_vio_status(struct mtk_devapc_context *ctx)
 {
 	void __iomem *reg;
-	int i;
+	int i, j;
 
-	reg = ctx->infra_base + ctx->data->vio_sta_offset;
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg = BASE(i) + ctx->data->vio_sta_offset;
 
-	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
-		writel(GENMASK(31, 0), reg + 4 * i);
+		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
+			writel(GENMASK(31, 0), reg + 4 * j);
+
+		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
+		       reg + 4 * j);
+	}
 
-	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
-	       reg + 4 * i);
 }
 
-static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
+static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
 {
-	void __iomem *reg;
 	u32 val;
 	int i;
 
-	reg = ctx->infra_base + ctx->data->vio_mask_offset;
-
 	if (mask)
 		val = GENMASK(31, 0);
 	else
 		val = 0;
 
-	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
+	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
 		writel(val, reg + 4 * i);
 
 	val = readl(reg + 4 * i);
 	if (mask)
-		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
+		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
 			       0);
 	else
-		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
+		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
 				0);
 
 	writel(val, reg + 4 * i);
@@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
  */
 static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
 {
+	int i;
+	void __iomem *reg_base;
 	void __iomem *pd_vio_shift_sta_reg;
 	void __iomem *pd_vio_shift_sel_reg;
 	void __iomem *pd_vio_shift_con_reg;
@@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
 	int ret;
 	u32 val;
 
-	pd_vio_shift_sta_reg = ctx->infra_base +
-			       ctx->data->vio_shift_sta_offset;
-	pd_vio_shift_sel_reg = ctx->infra_base +
-			       ctx->data->vio_shift_sel_offset;
-	pd_vio_shift_con_reg = ctx->infra_base +
-			       ctx->data->vio_shift_con_offset;
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg_base = BASE(i);
 
-	/* Find the minimum shift group which has violation */
-	val = readl(pd_vio_shift_sta_reg);
-	if (!val)
-		return false;
+		pd_vio_shift_sta_reg = reg_base +
+				       ctx->data->vio_shift_sta_offset;
+		pd_vio_shift_sel_reg = reg_base +
+				       ctx->data->vio_shift_sel_offset;
+		pd_vio_shift_con_reg = reg_base +
+				       ctx->data->vio_shift_con_offset;
 
-	min_shift_group = __ffs(val);
+		/* Find the minimum shift group which has violation */
+		val = readl(pd_vio_shift_sta_reg);
+		if (!val)
+			continue;
 
-	/* Assign the group to sync */
-	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
+		min_shift_group = __ffs(val);
 
-	/* Start syncing */
-	writel(0x1, pd_vio_shift_con_reg);
+		/* Assign the group to sync */
+		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
 
-	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
-				 PHY_DEVAPC_TIMEOUT);
-	if (ret) {
-		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
-		return false;
-	}
+		/* Start syncing */
+		writel(0x1, pd_vio_shift_con_reg);
+
+		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
+					 0, PHY_DEVAPC_TIMEOUT);
+		if (ret) {
+			dev_err(ctx->dev, "%s: Shift violation info failed\n",
+				__func__);
+			return -ETIMEDOUT;
+		}
 
-	/* Stop syncing */
-	writel(0x0, pd_vio_shift_con_reg);
+		/* Stop syncing */
+		writel(0x0, pd_vio_shift_con_reg);
 
-	/* Write clear */
-	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
+		/* Write clear */
+		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
 
-	return true;
+		return i;
+	}
+
+	/* No violation found */
+	return -ENODATA;
 }
 
 /*
  * devapc_extract_vio_dbg - extract full violation information after doing
  *                          shift mechanism.
  */
-static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
+static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
+				   int vio_slave_type)
 {
 	struct mtk_devapc_vio_dbgs vio_dbgs;
 	void __iomem *vio_dbg0_reg;
 	void __iomem *vio_dbg1_reg;
+	void __iomem *vio_dbg2_reg;
+	u32 vio_addr, bus_id;
 
-	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
-	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
+	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
+	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
+	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
 
 	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
 	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
+	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
+
+	switch (ctx->arch_ver) {
+	case 1:
+		bus_id = vio_dbgs.dbg0_bits.mstid;
+		vio_addr = vio_dbgs.vio_dbg1;
+		break;
+	case 2:
+		bus_id = vio_dbgs.vio_dbg1;
+		vio_addr = vio_dbgs.vio_dbg2;
+
+		/* To align with the bit definition of arch_ver 1 */
+		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
+		break;
+	default:
+		/* Not Supported */
+		return;
+	}
 
 	/* Print violation information */
 	if (vio_dbgs.dbg0_bits.vio_w)
@@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
 		dev_info(ctx->dev, "Read Violation\n");
 
 	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
-		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
-		 vio_dbgs.vio_dbg1);
+		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
 }
 
 /*
@@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
 static irqreturn_t devapc_violation_irq(int irq_number, void *data)
 {
 	struct mtk_devapc_context *ctx = data;
+	int vio_slave_type;
 
-	while (devapc_sync_vio_dbg(ctx))
-		devapc_extract_vio_dbg(ctx);
+	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
+		devapc_extract_vio_dbg(ctx, vio_slave_type);
 
 	clear_vio_status(ctx);
 
@@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
  */
 static void start_devapc(struct mtk_devapc_context *ctx)
 {
-	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
+	int i;
+	void __iomem *reg_base;
 
-	mask_module_irq(ctx, false);
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
+
+		reg_base = BASE(i) + ctx->data->vio_mask_offset;
+		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
+	}
 }
 
 /*
@@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
  */
 static void stop_devapc(struct mtk_devapc_context *ctx)
 {
-	mask_module_irq(ctx, true);
+	int i;
+	void __iomem *reg_base;
+
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg_base = BASE(i) + ctx->data->vio_mask_offset;
+		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
 
-	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
+		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
+	}
 }
 
 static const struct mtk_devapc_data devapc_mt6779 = {
-	.vio_idx_num = 511,
 	.vio_mask_offset = 0x0,
 	.vio_sta_offset = 0x400,
 	.vio_dbg0_offset = 0x900,
@@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
 	.vio_shift_con_offset = 0xF20,
 };
 
+static const struct mtk_devapc_data devapc_mt8192 = {
+	.vio_mask_offset = 0x0,
+	.vio_sta_offset = 0x400,
+	.vio_dbg0_offset = 0x900,
+	.vio_dbg1_offset = 0x904,
+	.vio_dbg2_offset = 0x908,
+	.apc_con_offset = 0xF00,
+	.vio_shift_sta_offset = 0xF20,
+	.vio_shift_sel_offset = 0xF30,
+	.vio_shift_con_offset = 0xF10,
+};
+
 static const struct of_device_id mtk_devapc_dt_match[] = {
 	{
 		.compatible = "mediatek,mt6779-devapc",
 		.data = &devapc_mt6779,
 	}, {
+		.compatible = "mediatek,mt8192-devapc",
+		.data = &devapc_mt8192,
+	}, {
 	},
 };
 
@@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
 	struct mtk_devapc_context *ctx;
+	int i;
 	u32 devapc_irq;
 	int ret;
 
@@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
 	ctx->data = of_device_get_match_data(&pdev->dev);
 	ctx->dev = &pdev->dev;
 
-	ctx->infra_base = of_iomap(node, 0);
-	if (!ctx->infra_base)
+	if (of_property_read_u32(node, "version", &ctx->arch_ver))
+		return -EINVAL;
+
+	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))
+		return -EINVAL;
+
+	ctx->base_list = devm_kzalloc(&pdev->dev,
+				      sizeof(void *) * ctx->slave_type_num,
+				      GFP_KERNEL);
+	if (!ctx->base_list)
+		return -ENOMEM;
+
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		BASE(i) = of_iomap(node, i);
+		if (!BASE(i))
+			return -EINVAL;
+	}
+
+	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
+					sizeof(u32) * ctx->slave_type_num,
+					GFP_KERNEL);
+	if (!ctx->vio_idx_num)
+		return -ENOMEM;
+
+	if (of_property_read_u32_array(node, "vio_idx_num",
+				       ctx->vio_idx_num, ctx->slave_type_num))
 		return -EINVAL;
 
 	devapc_irq = irq_of_parse_and_map(node, 0);
-- 
2.6.4
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Nina Wu]

From: Nina Wu <Nina-CM.Wu@mediatek.com>

The hardware architecture of mt8192 devapc is slightly
different from that of the previous IC.
We add necessary extensions to support mt8192 and be
back-compatible with other ICs.

Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
---
 drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
 1 file changed, 156 insertions(+), 57 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
index f1cea04..1e40a52 100644
--- a/drivers/soc/mediatek/mtk-devapc.c
+++ b/drivers/soc/mediatek/mtk-devapc.c
@@ -15,6 +15,11 @@
 #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
 #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
 
+#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
+	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)
+#define BASE(i)			(ctx->base_list[i])
+#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])
+
 struct mtk_devapc_vio_dbgs {
 	union {
 		u32 vio_dbg0;
@@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
 			u32 addr_h:4;
 			u32 resv:4;
 		} dbg0_bits;
+
+		/* Not used, reference only */
+		struct {
+			u32 dmnid:6;
+			u32 vio_w:1;
+			u32 vio_r:1;
+			u32 addr_h:4;
+			u32 resv:20;
+		} dbg0_bits_ver2;
 	};
 
 	u32 vio_dbg1;
+	u32 vio_dbg2;
 };
 
 struct mtk_devapc_data {
-	/* numbers of violation index */
-	u32 vio_idx_num;
-
 	/* reg offset */
 	u32 vio_mask_offset;
 	u32 vio_sta_offset;
 	u32 vio_dbg0_offset;
 	u32 vio_dbg1_offset;
+	u32 vio_dbg2_offset;
 	u32 apc_con_offset;
 	u32 vio_shift_sta_offset;
 	u32 vio_shift_sel_offset;
@@ -48,7 +61,10 @@ struct mtk_devapc_data {
 
 struct mtk_devapc_context {
 	struct device *dev;
-	void __iomem *infra_base;
+	u32 arch_ver;
+	u32 slave_type_num;
+	void __iomem **base_list;
+	u32 *vio_idx_num;
 	struct clk *infra_clk;
 	const struct mtk_devapc_data *data;
 };
@@ -56,39 +72,39 @@ struct mtk_devapc_context {
 static void clear_vio_status(struct mtk_devapc_context *ctx)
 {
 	void __iomem *reg;
-	int i;
+	int i, j;
 
-	reg = ctx->infra_base + ctx->data->vio_sta_offset;
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg = BASE(i) + ctx->data->vio_sta_offset;
 
-	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
-		writel(GENMASK(31, 0), reg + 4 * i);
+		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
+			writel(GENMASK(31, 0), reg + 4 * j);
+
+		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
+		       reg + 4 * j);
+	}
 
-	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
-	       reg + 4 * i);
 }
 
-static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
+static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
 {
-	void __iomem *reg;
 	u32 val;
 	int i;
 
-	reg = ctx->infra_base + ctx->data->vio_mask_offset;
-
 	if (mask)
 		val = GENMASK(31, 0);
 	else
 		val = 0;
 
-	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
+	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
 		writel(val, reg + 4 * i);
 
 	val = readl(reg + 4 * i);
 	if (mask)
-		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
+		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
 			       0);
 	else
-		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
+		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
 				0);
 
 	writel(val, reg + 4 * i);
@@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
  */
 static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
 {
+	int i;
+	void __iomem *reg_base;
 	void __iomem *pd_vio_shift_sta_reg;
 	void __iomem *pd_vio_shift_sel_reg;
 	void __iomem *pd_vio_shift_con_reg;
@@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
 	int ret;
 	u32 val;
 
-	pd_vio_shift_sta_reg = ctx->infra_base +
-			       ctx->data->vio_shift_sta_offset;
-	pd_vio_shift_sel_reg = ctx->infra_base +
-			       ctx->data->vio_shift_sel_offset;
-	pd_vio_shift_con_reg = ctx->infra_base +
-			       ctx->data->vio_shift_con_offset;
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg_base = BASE(i);
 
-	/* Find the minimum shift group which has violation */
-	val = readl(pd_vio_shift_sta_reg);
-	if (!val)
-		return false;
+		pd_vio_shift_sta_reg = reg_base +
+				       ctx->data->vio_shift_sta_offset;
+		pd_vio_shift_sel_reg = reg_base +
+				       ctx->data->vio_shift_sel_offset;
+		pd_vio_shift_con_reg = reg_base +
+				       ctx->data->vio_shift_con_offset;
 
-	min_shift_group = __ffs(val);
+		/* Find the minimum shift group which has violation */
+		val = readl(pd_vio_shift_sta_reg);
+		if (!val)
+			continue;
 
-	/* Assign the group to sync */
-	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
+		min_shift_group = __ffs(val);
 
-	/* Start syncing */
-	writel(0x1, pd_vio_shift_con_reg);
+		/* Assign the group to sync */
+		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
 
-	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
-				 PHY_DEVAPC_TIMEOUT);
-	if (ret) {
-		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
-		return false;
-	}
+		/* Start syncing */
+		writel(0x1, pd_vio_shift_con_reg);
+
+		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
+					 0, PHY_DEVAPC_TIMEOUT);
+		if (ret) {
+			dev_err(ctx->dev, "%s: Shift violation info failed\n",
+				__func__);
+			return -ETIMEDOUT;
+		}
 
-	/* Stop syncing */
-	writel(0x0, pd_vio_shift_con_reg);
+		/* Stop syncing */
+		writel(0x0, pd_vio_shift_con_reg);
 
-	/* Write clear */
-	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
+		/* Write clear */
+		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
 
-	return true;
+		return i;
+	}
+
+	/* No violation found */
+	return -ENODATA;
 }
 
 /*
  * devapc_extract_vio_dbg - extract full violation information after doing
  *                          shift mechanism.
  */
-static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
+static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
+				   int vio_slave_type)
 {
 	struct mtk_devapc_vio_dbgs vio_dbgs;
 	void __iomem *vio_dbg0_reg;
 	void __iomem *vio_dbg1_reg;
+	void __iomem *vio_dbg2_reg;
+	u32 vio_addr, bus_id;
 
-	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
-	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
+	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
+	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
+	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
 
 	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
 	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
+	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
+
+	switch (ctx->arch_ver) {
+	case 1:
+		bus_id = vio_dbgs.dbg0_bits.mstid;
+		vio_addr = vio_dbgs.vio_dbg1;
+		break;
+	case 2:
+		bus_id = vio_dbgs.vio_dbg1;
+		vio_addr = vio_dbgs.vio_dbg2;
+
+		/* To align with the bit definition of arch_ver 1 */
+		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
+		break;
+	default:
+		/* Not Supported */
+		return;
+	}
 
 	/* Print violation information */
 	if (vio_dbgs.dbg0_bits.vio_w)
@@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
 		dev_info(ctx->dev, "Read Violation\n");
 
 	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
-		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
-		 vio_dbgs.vio_dbg1);
+		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
 }
 
 /*
@@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
 static irqreturn_t devapc_violation_irq(int irq_number, void *data)
 {
 	struct mtk_devapc_context *ctx = data;
+	int vio_slave_type;
 
-	while (devapc_sync_vio_dbg(ctx))
-		devapc_extract_vio_dbg(ctx);
+	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
+		devapc_extract_vio_dbg(ctx, vio_slave_type);
 
 	clear_vio_status(ctx);
 
@@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
  */
 static void start_devapc(struct mtk_devapc_context *ctx)
 {
-	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
+	int i;
+	void __iomem *reg_base;
 
-	mask_module_irq(ctx, false);
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
+
+		reg_base = BASE(i) + ctx->data->vio_mask_offset;
+		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
+	}
 }
 
 /*
@@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
  */
 static void stop_devapc(struct mtk_devapc_context *ctx)
 {
-	mask_module_irq(ctx, true);
+	int i;
+	void __iomem *reg_base;
+
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		reg_base = BASE(i) + ctx->data->vio_mask_offset;
+		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
 
-	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
+		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
+	}
 }
 
 static const struct mtk_devapc_data devapc_mt6779 = {
-	.vio_idx_num = 511,
 	.vio_mask_offset = 0x0,
 	.vio_sta_offset = 0x400,
 	.vio_dbg0_offset = 0x900,
@@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
 	.vio_shift_con_offset = 0xF20,
 };
 
+static const struct mtk_devapc_data devapc_mt8192 = {
+	.vio_mask_offset = 0x0,
+	.vio_sta_offset = 0x400,
+	.vio_dbg0_offset = 0x900,
+	.vio_dbg1_offset = 0x904,
+	.vio_dbg2_offset = 0x908,
+	.apc_con_offset = 0xF00,
+	.vio_shift_sta_offset = 0xF20,
+	.vio_shift_sel_offset = 0xF30,
+	.vio_shift_con_offset = 0xF10,
+};
+
 static const struct of_device_id mtk_devapc_dt_match[] = {
 	{
 		.compatible = "mediatek,mt6779-devapc",
 		.data = &devapc_mt6779,
 	}, {
+		.compatible = "mediatek,mt8192-devapc",
+		.data = &devapc_mt8192,
+	}, {
 	},
 };
 
@@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
 	struct mtk_devapc_context *ctx;
+	int i;
 	u32 devapc_irq;
 	int ret;
 
@@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
 	ctx->data = of_device_get_match_data(&pdev->dev);
 	ctx->dev = &pdev->dev;
 
-	ctx->infra_base = of_iomap(node, 0);
-	if (!ctx->infra_base)
+	if (of_property_read_u32(node, "version", &ctx->arch_ver))
+		return -EINVAL;
+
+	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))
+		return -EINVAL;
+
+	ctx->base_list = devm_kzalloc(&pdev->dev,
+				      sizeof(void *) * ctx->slave_type_num,
+				      GFP_KERNEL);
+	if (!ctx->base_list)
+		return -ENOMEM;
+
+	FOR_EACH_SLAVE_TYPE(ctx, i) {
+		BASE(i) = of_iomap(node, i);
+		if (!BASE(i))
+			return -EINVAL;
+	}
+
+	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
+					sizeof(u32) * ctx->slave_type_num,
+					GFP_KERNEL);
+	if (!ctx->vio_idx_num)
+		return -ENOMEM;
+
+	if (of_property_read_u32_array(node, "vio_idx_num",
+				       ctx->vio_idx_num, ctx->slave_type_num))
 		return -EINVAL;
 
 	devapc_irq = irq_of_parse_and_map(node, 0);
-- 
2.6.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Rob Herring]

On Fri, 26 Mar 2021 15:31:10 +0800, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: properties:version:enum: False schema does not allow [1, 2]
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: properties:slave_type_num:enum: False schema does not allow [1, 4]
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: ignoring, error in schema: properties: slave_type_num: enum
warning: no schema found in file: ./Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
Documentation/devicetree/bindings/soc/mediatek/devapc.example.dts:51:18: fatal error: dt-bindings/clock/mt8192-clk.h: No such file or directory
   51 |         #include <dt-bindings/clock/mt8192-clk.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/soc/mediatek/devapc.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1380: dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1458687

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Rob Herring]

On Fri, 26 Mar 2021 15:31:10 +0800, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: properties:version:enum: False schema does not allow [1, 2]
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: properties:slave_type_num:enum: False schema does not allow [1, 4]
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: ignoring, error in schema: properties: slave_type_num: enum
warning: no schema found in file: ./Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
Documentation/devicetree/bindings/soc/mediatek/devapc.example.dts:51:18: fatal error: dt-bindings/clock/mt8192-clk.h: No such file or directory
   51 |         #include <dt-bindings/clock/mt8192-clk.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/soc/mediatek/devapc.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1380: dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1458687

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Rob Herring]

On Fri, 26 Mar 2021 15:31:10 +0800, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: properties:version:enum: False schema does not allow [1, 2]
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: properties:slave_type_num:enum: False schema does not allow [1, 4]
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: ignoring, error in schema: properties: slave_type_num: enum
warning: no schema found in file: ./Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
Documentation/devicetree/bindings/soc/mediatek/devapc.example.dts:51:18: fatal error: dt-bindings/clock/mt8192-clk.h: No such file or directory
   51 |         #include <dt-bindings/clock/mt8192-clk.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/soc/mediatek/devapc.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1380: dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1458687

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Chun-Kuang Hu]

Hi, Nina:

Nina Wu <nina-cm.wu@mediatek.com> 於 2021年3月26日 週五 下午3:34寫道：
>
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
>
> To support newer hardware architecture of devapc,
> update device tree bindings.
>
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:
> +    description: The number of the devapc set
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4
> +
> +  vio_idx_num:
> +    description: The number of the devices controlled by devapc
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>      maxItems: 1
>
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;

I think version is redundant. For example, if mt0001-devapc is
identical to mt6779-devapc, its compatible should be

compatible = "mediatek,mt0001-devapc", "mediatek,mt6779-devapc";

In driver, only keep compatible for mt6779 and no mt0001 because
mt0001 is identical to mt6779.
In probe sequence, try first compatible string
"mediatek,mt0001-devapc", but it does not exist in driver, so try next
compatible string "mediatek,mt6779-devapc" and match.
So mt0001-devapc would work as mt6779-devapc.

> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;
> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };

It looks like that there are 4 devapc device in mt8192.
These 4 device work independently, so I would like to decouple them
rather than couple them.

devapc0: devapc@10207000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10207000 0 0x1000>;
    vio_idx_num = <367>;
    ...
};

devapc1: devapc@10274000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10274000 0 0x1000>;
    vio_idx_num = <292>;
    ...
};

devapc2: devapc@10275000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10275000 0 0x1000>;
    vio_idx_num = <242>;
    ...
};

devapc3: devapc@11020000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x11020000 0 0x1000>;
    vio_idx_num = <58>;
    ...
};

Regards,
Chun-Kuang.

> --
> 2.6.4
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Chun-Kuang Hu]

Hi, Nina:

Nina Wu <nina-cm.wu@mediatek.com> 於 2021年3月26日 週五 下午3:34寫道：
>
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
>
> To support newer hardware architecture of devapc,
> update device tree bindings.
>
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:
> +    description: The number of the devapc set
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4
> +
> +  vio_idx_num:
> +    description: The number of the devices controlled by devapc
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>      maxItems: 1
>
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;

I think version is redundant. For example, if mt0001-devapc is
identical to mt6779-devapc, its compatible should be

compatible = "mediatek,mt0001-devapc", "mediatek,mt6779-devapc";

In driver, only keep compatible for mt6779 and no mt0001 because
mt0001 is identical to mt6779.
In probe sequence, try first compatible string
"mediatek,mt0001-devapc", but it does not exist in driver, so try next
compatible string "mediatek,mt6779-devapc" and match.
So mt0001-devapc would work as mt6779-devapc.

> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;
> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };

It looks like that there are 4 devapc device in mt8192.
These 4 device work independently, so I would like to decouple them
rather than couple them.

devapc0: devapc@10207000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10207000 0 0x1000>;
    vio_idx_num = <367>;
    ...
};

devapc1: devapc@10274000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10274000 0 0x1000>;
    vio_idx_num = <292>;
    ...
};

devapc2: devapc@10275000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10275000 0 0x1000>;
    vio_idx_num = <242>;
    ...
};

devapc3: devapc@11020000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x11020000 0 0x1000>;
    vio_idx_num = <58>;
    ...
};

Regards,
Chun-Kuang.

> --
> 2.6.4
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Chun-Kuang Hu]

Hi, Nina:

Nina Wu <nina-cm.wu@mediatek.com> 於 2021年3月26日 週五 下午3:34寫道：
>
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
>
> To support newer hardware architecture of devapc,
> update device tree bindings.
>
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:
> +    description: The number of the devapc set
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4
> +
> +  vio_idx_num:
> +    description: The number of the devices controlled by devapc
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>      maxItems: 1
>
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;

I think version is redundant. For example, if mt0001-devapc is
identical to mt6779-devapc, its compatible should be

compatible = "mediatek,mt0001-devapc", "mediatek,mt6779-devapc";

In driver, only keep compatible for mt6779 and no mt0001 because
mt0001 is identical to mt6779.
In probe sequence, try first compatible string
"mediatek,mt0001-devapc", but it does not exist in driver, so try next
compatible string "mediatek,mt6779-devapc" and match.
So mt0001-devapc would work as mt6779-devapc.

> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;
> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };

It looks like that there are 4 devapc device in mt8192.
These 4 device work independently, so I would like to decouple them
rather than couple them.

devapc0: devapc@10207000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10207000 0 0x1000>;
    vio_idx_num = <367>;
    ...
};

devapc1: devapc@10274000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10274000 0 0x1000>;
    vio_idx_num = <292>;
    ...
};

devapc2: devapc@10275000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x10275000 0 0x1000>;
    vio_idx_num = <242>;
    ...
};

devapc3: devapc@11020000 {
    compatible = "mediatek,mt8192-devapc";
    reg = <0 0x11020000 0 0x1000>;
    vio_idx_num = <58>;
    ...
};

Regards,
Chun-Kuang.

> --
> 2.6.4
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Rob Herring]

On Fri, Mar 26, 2021 at 03:31:10PM +0800, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture

This should be implied by the compatible string.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:

vendor prefix needed and s/_/-/

> +    description: The number of the devapc set

What?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>  
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4

Need to define what each region is.

> +
> +  vio_idx_num:

vendor prefix needed and s/_/-/

> +    description: The number of the devices controlled by devapc

No need to know which devices?

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>      maxItems: 1

uint32-array with 'maxItems: 1' is just 'uint32'

>  
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>  
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>  
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;
> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;

Is the length of this the same as the value of slave_type_num? If so, 
don't need both.

> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };
> -- 
> 2.6.4
> 

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Rob Herring]

On Fri, Mar 26, 2021 at 03:31:10PM +0800, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture

This should be implied by the compatible string.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:

vendor prefix needed and s/_/-/

> +    description: The number of the devapc set

What?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>  
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4

Need to define what each region is.

> +
> +  vio_idx_num:

vendor prefix needed and s/_/-/

> +    description: The number of the devices controlled by devapc

No need to know which devices?

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>      maxItems: 1

uint32-array with 'maxItems: 1' is just 'uint32'

>  
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>  
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>  
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;
> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;

Is the length of this the same as the value of slave_type_num? If so, 
don't need both.

> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };
> -- 
> 2.6.4
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Rob Herring]

On Fri, Mar 26, 2021 at 03:31:10PM +0800, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture

This should be implied by the compatible string.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:

vendor prefix needed and s/_/-/

> +    description: The number of the devapc set

What?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>  
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4

Need to define what each region is.

> +
> +  vio_idx_num:

vendor prefix needed and s/_/-/

> +    description: The number of the devices controlled by devapc

No need to know which devices?

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>      maxItems: 1

uint32-array with 'maxItems: 1' is just 'uint32'

>  
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>  
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>  
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;
> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;

Is the length of this the same as the value of slave_type_num? If so, 
don't need both.

> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };
> -- 
> 2.6.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

Hi, Rob

I just found that there is the un-merged dependent patch.
https://patchwork.kernel.org/project/linux-mediatek/patch/20210324104110.13383-7-chun-jie.chen@mediatek.com/

I will add this to commit message in the next version.

Thanks

On Fri, 2021-03-26 at 09:10 -0600, Rob Herring wrote:
> On Fri, 26 Mar 2021 15:31:10 +0800, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > To support newer hardware architecture of devapc,
> > update device tree bindings.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: properties:version:enum: False schema does not allow [1, 2]
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: properties:slave_type_num:enum: False schema does not allow [1, 4]
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml: ignoring, error in schema: properties: slave_type_num: enum
> warning: no schema found in file: ./Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> Documentation/devicetree/bindings/soc/mediatek/devapc.example.dts:51:18: fatal error: dt-bindings/clock/mt8192-clk.h: No such file or directory
>    51 |         #include <dt-bindings/clock/mt8192-clk.h>
>       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/soc/mediatek/devapc.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1380: dt_binding_check] Error 2
> 
> See https://urldefense.com/v3/__https://patchwork.ozlabs.org/patch/1458687__;!!CTRNKA9wMg0ARbw!zZmjm74Leee8o-eaQUB_yHYvh-66g88Rgjozv_ecSkwW-yfo7G_c9o6-p0JlFfst3VI$ 
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

Hi, Chun-Kuang

On Sat, 2021-03-27 at 00:18 +0800, Chun-Kuang Hu wrote:
> Hi, Nina:
> 
> Nina Wu <nina-cm.wu@mediatek.com> 於 2021年3月26日 週五 下午3:34寫道：
> >
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> >
> > To support newer hardware architecture of devapc,
> > update device tree bindings.
> >
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > index 31e4d3c..489f6a9 100644
> > --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > @@ -20,9 +20,27 @@ properties:
> >    compatible:
> >      enum:
> >        - mediatek,mt6779-devapc
> > +      - mediatek,mt8192-devapc
> > +
> > +  version:
> > +    description: The version of the hardware architecture
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2]
> > +    maxItems: 1
> > +
> > +  slave_type_num:
> > +    description: The number of the devapc set
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 4]
> > +    maxItems: 1
> >
> >    reg:
> >      description: The base address of devapc register bank
> > +    maxItems: 4
> > +
> > +  vio_idx_num:
> > +    description: The number of the devices controlled by devapc
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >      maxItems: 1
> >
> >    interrupts:
> > @@ -39,7 +57,10 @@ properties:
> >
> >  required:
> >    - compatible
> > +  - version
> > +  - slave_type_num
> >    - reg
> > +  - vio_idx_num
> >    - interrupts
> >    - clocks
> >    - clock-names
> > @@ -53,8 +74,28 @@ examples:
> >
> >      devapc: devapc@10207000 {
> >        compatible = "mediatek,mt6779-devapc";
> > +      version = <1>;
> 
> I think version is redundant. For example, if mt0001-devapc is
> identical to mt6779-devapc, its compatible should be
> 
> compatible = "mediatek,mt0001-devapc", "mediatek,mt6779-devapc";
> 
> In driver, only keep compatible for mt6779 and no mt0001 because
> mt0001 is identical to mt6779.
> In probe sequence, try first compatible string
> "mediatek,mt0001-devapc", but it does not exist in driver, so try next
> compatible string "mediatek,mt6779-devapc" and match.
> So mt0001-devapc would work as mt6779-devapc.
> 

I think the version is still needed.
Because there is little difference in the registers which save debug
info.


> > +      slave_type_num = <1>;
> >        reg = <0x10207000 0x1000>;
> > +      vio_idx_num = <511>;
> >        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
> >        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> >        clock-names = "devapc-infra-clock";
> >      };
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/mt8192-clk.h>
> > +
> > +    devapc: devapc@10207000 {
> > +        compatible = "mediatek,mt8192-devapc";
> > +        version = <2>;
> > +        slave_type_num = <4>;
> > +        reg = <0 0x10207000 0 0x1000>,
> > +            <0 0x10274000 0 0x1000>,
> > +            <0 0x10275000 0 0x1000>,
> > +            <0 0x11020000 0 0x1000>;
> > +        vio_idx_num = <367 292 242 58>;
> > +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> > +        clock-names = "devapc-infra-clock";
> > +    };
> 
> It looks like that there are 4 devapc device in mt8192.
> These 4 device work independently, so I would like to decouple them
> rather than couple them.
> 
> devapc0: devapc@10207000 {
>     compatible = "mediatek,mt8192-devapc";
>     reg = <0 0x10207000 0 0x1000>;
>     vio_idx_num = <367>;
>     ...
> };
> 
> devapc1: devapc@10274000 {
>     compatible = "mediatek,mt8192-devapc";
>     reg = <0 0x10274000 0 0x1000>;
>     vio_idx_num = <292>;
>     ...
> };
> 
> devapc2: devapc@10275000 {
>     compatible = "mediatek,mt8192-devapc";
>     reg = <0 0x10275000 0 0x1000>;
>     vio_idx_num = <242>;
>     ...
> };
> 
> devapc3: devapc@11020000 {
>     compatible = "mediatek,mt8192-devapc";
>     reg = <0 0x11020000 0 0x1000>;
>     vio_idx_num = <58>;
>     ...
> };
> 

I will try this with shared IRQ and re-submit another version.


> Regards,
> Chun-Kuang.
> 
> > --
> > 2.6.4
> > _______________________________________________
> > Linux-mediatek mailing list
> > Linux-mediatek@lists.infradead.org
> > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mediatek__;!!CTRNKA9wMg0ARbw!02jtHESdXiknfQKFC-IqkUJOuWEjeE-GMqwk3RmPMm3_T-Xv9pmUk9Zoi2e2kvXjoKc$ 

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

Hi, Rob


On Fri, 2021-03-26 at 13:58 -0600, Rob Herring wrote:
> On Fri, Mar 26, 2021 at 03:31:10PM +0800, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > To support newer hardware architecture of devapc,
> > update device tree bindings.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > index 31e4d3c..489f6a9 100644
> > --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > @@ -20,9 +20,27 @@ properties:
> >    compatible:
> >      enum:
> >        - mediatek,mt6779-devapc
> > +      - mediatek,mt8192-devapc
> > +
> > +  version:
> > +    description: The version of the hardware architecture
> 
> This should be implied by the compatible string.

The version attribute is used to decide how we interpret the debug info
got from registers.
As you mentioned, we can know the version of the architecture from the
compatible, but I think there will be code like this:

if (compatible is mt6779) version = 1
else if (compatible is mt8192) version = 2

And once we have more chips to support, the code will be quite long.
So I prefer to add a 'version' here.


> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2]
> > +    maxItems: 1
> > +
> > +  slave_type_num:
> 
> vendor prefix needed and s/_/-/

I will fixed in next version.

> 
> > +    description: The number of the devapc set
> 
> What?

For mt8192, there are multiple pieces of devapc HW for different subsys.
EX: infra devapc, peri devapc, etc.
'slave_type_num' is the total number of the devapc HW.
I cannot come up with an accurate description, though.


> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 4]
> > +    maxItems: 1
> >  
> >    reg:
> >      description: The base address of devapc register bank
> > +    maxItems: 4
> 
> Need to define what each region is.

I will fix it in the next version.

> 
> > +
> > +  vio_idx_num:
> 
> vendor prefix needed and s/_/-/

OK, will be fixed in the next version.

> 
> > +    description: The number of the devices controlled by devapc
> 
> No need to know which devices?

yes, the current driver does not care each of them.

> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >      maxItems: 1
> 
> uint32-array with 'maxItems: 1' is just 'uint32'
> 

got it, so it should be 'maxItems: 4'

> >  
> >    interrupts:
> > @@ -39,7 +57,10 @@ properties:
> >  
> >  required:
> >    - compatible
> > +  - version
> > +  - slave_type_num
> >    - reg
> > +  - vio_idx_num
> >    - interrupts
> >    - clocks
> >    - clock-names
> > @@ -53,8 +74,28 @@ examples:
> >  
> >      devapc: devapc@10207000 {
> >        compatible = "mediatek,mt6779-devapc";
> > +      version = <1>;
> > +      slave_type_num = <1>;
> >        reg = <0x10207000 0x1000>;
> > +      vio_idx_num = <511>;
> >        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
> >        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> >        clock-names = "devapc-infra-clock";
> >      };
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/mt8192-clk.h>
> > +
> > +    devapc: devapc@10207000 {
> > +        compatible = "mediatek,mt8192-devapc";
> > +        version = <2>;
> > +        slave_type_num = <4>;
> > +        reg = <0 0x10207000 0 0x1000>,
> > +            <0 0x10274000 0 0x1000>,
> > +            <0 0x10275000 0 0x1000>,
> > +            <0 0x11020000 0 0x1000>;
> > +        vio_idx_num = <367 292 242 58>;
> 
> Is the length of this the same as the value of slave_type_num? If so, 
> don't need both.
> 

yes, the length is equal to slave_type_num.
I will try to remove it in the next version.

> > +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> > +        clock-names = "devapc-infra-clock";
> > +    };
> > -- 
> > 2.6.4
> > 

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

Hi, Rob


On Fri, 2021-03-26 at 13:58 -0600, Rob Herring wrote:
> On Fri, Mar 26, 2021 at 03:31:10PM +0800, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > To support newer hardware architecture of devapc,
> > update device tree bindings.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > index 31e4d3c..489f6a9 100644
> > --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > @@ -20,9 +20,27 @@ properties:
> >    compatible:
> >      enum:
> >        - mediatek,mt6779-devapc
> > +      - mediatek,mt8192-devapc
> > +
> > +  version:
> > +    description: The version of the hardware architecture
> 
> This should be implied by the compatible string.

The version attribute is used to decide how we interpret the debug info
got from registers.
As you mentioned, we can know the version of the architecture from the
compatible, but I think there will be code like this:

if (compatible is mt6779) version = 1
else if (compatible is mt8192) version = 2

And once we have more chips to support, the code will be quite long.
So I prefer to add a 'version' here.


> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2]
> > +    maxItems: 1
> > +
> > +  slave_type_num:
> 
> vendor prefix needed and s/_/-/

I will fixed in next version.

> 
> > +    description: The number of the devapc set
> 
> What?

For mt8192, there are multiple pieces of devapc HW for different subsys.
EX: infra devapc, peri devapc, etc.
'slave_type_num' is the total number of the devapc HW.
I cannot come up with an accurate description, though.


> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 4]
> > +    maxItems: 1
> >  
> >    reg:
> >      description: The base address of devapc register bank
> > +    maxItems: 4
> 
> Need to define what each region is.

I will fix it in the next version.

> 
> > +
> > +  vio_idx_num:
> 
> vendor prefix needed and s/_/-/

OK, will be fixed in the next version.

> 
> > +    description: The number of the devices controlled by devapc
> 
> No need to know which devices?

yes, the current driver does not care each of them.

> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >      maxItems: 1
> 
> uint32-array with 'maxItems: 1' is just 'uint32'
> 

got it, so it should be 'maxItems: 4'

> >  
> >    interrupts:
> > @@ -39,7 +57,10 @@ properties:
> >  
> >  required:
> >    - compatible
> > +  - version
> > +  - slave_type_num
> >    - reg
> > +  - vio_idx_num
> >    - interrupts
> >    - clocks
> >    - clock-names
> > @@ -53,8 +74,28 @@ examples:
> >  
> >      devapc: devapc@10207000 {
> >        compatible = "mediatek,mt6779-devapc";
> > +      version = <1>;
> > +      slave_type_num = <1>;
> >        reg = <0x10207000 0x1000>;
> > +      vio_idx_num = <511>;
> >        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
> >        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> >        clock-names = "devapc-infra-clock";
> >      };
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/mt8192-clk.h>
> > +
> > +    devapc: devapc@10207000 {
> > +        compatible = "mediatek,mt8192-devapc";
> > +        version = <2>;
> > +        slave_type_num = <4>;
> > +        reg = <0 0x10207000 0 0x1000>,
> > +            <0 0x10274000 0 0x1000>,
> > +            <0 0x10275000 0 0x1000>,
> > +            <0 0x11020000 0 0x1000>;
> > +        vio_idx_num = <367 292 242 58>;
> 
> Is the length of this the same as the value of slave_type_num? If so, 
> don't need both.
> 

yes, the length is equal to slave_type_num.
I will try to remove it in the next version.

> > +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> > +        clock-names = "devapc-infra-clock";
> > +    };
> > -- 
> > 2.6.4
> > 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

Hi, Rob


On Fri, 2021-03-26 at 13:58 -0600, Rob Herring wrote:
> On Fri, Mar 26, 2021 at 03:31:10PM +0800, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > To support newer hardware architecture of devapc,
> > update device tree bindings.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > index 31e4d3c..489f6a9 100644
> > --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > @@ -20,9 +20,27 @@ properties:
> >    compatible:
> >      enum:
> >        - mediatek,mt6779-devapc
> > +      - mediatek,mt8192-devapc
> > +
> > +  version:
> > +    description: The version of the hardware architecture
> 
> This should be implied by the compatible string.

The version attribute is used to decide how we interpret the debug info
got from registers.
As you mentioned, we can know the version of the architecture from the
compatible, but I think there will be code like this:

if (compatible is mt6779) version = 1
else if (compatible is mt8192) version = 2

And once we have more chips to support, the code will be quite long.
So I prefer to add a 'version' here.


> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2]
> > +    maxItems: 1
> > +
> > +  slave_type_num:
> 
> vendor prefix needed and s/_/-/

I will fixed in next version.

> 
> > +    description: The number of the devapc set
> 
> What?

For mt8192, there are multiple pieces of devapc HW for different subsys.
EX: infra devapc, peri devapc, etc.
'slave_type_num' is the total number of the devapc HW.
I cannot come up with an accurate description, though.


> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 4]
> > +    maxItems: 1
> >  
> >    reg:
> >      description: The base address of devapc register bank
> > +    maxItems: 4
> 
> Need to define what each region is.

I will fix it in the next version.

> 
> > +
> > +  vio_idx_num:
> 
> vendor prefix needed and s/_/-/

OK, will be fixed in the next version.

> 
> > +    description: The number of the devices controlled by devapc
> 
> No need to know which devices?

yes, the current driver does not care each of them.

> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >      maxItems: 1
> 
> uint32-array with 'maxItems: 1' is just 'uint32'
> 

got it, so it should be 'maxItems: 4'

> >  
> >    interrupts:
> > @@ -39,7 +57,10 @@ properties:
> >  
> >  required:
> >    - compatible
> > +  - version
> > +  - slave_type_num
> >    - reg
> > +  - vio_idx_num
> >    - interrupts
> >    - clocks
> >    - clock-names
> > @@ -53,8 +74,28 @@ examples:
> >  
> >      devapc: devapc@10207000 {
> >        compatible = "mediatek,mt6779-devapc";
> > +      version = <1>;
> > +      slave_type_num = <1>;
> >        reg = <0x10207000 0x1000>;
> > +      vio_idx_num = <511>;
> >        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
> >        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> >        clock-names = "devapc-infra-clock";
> >      };
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/mt8192-clk.h>
> > +
> > +    devapc: devapc@10207000 {
> > +        compatible = "mediatek,mt8192-devapc";
> > +        version = <2>;
> > +        slave_type_num = <4>;
> > +        reg = <0 0x10207000 0 0x1000>,
> > +            <0 0x10274000 0 0x1000>,
> > +            <0 0x10275000 0 0x1000>,
> > +            <0 0x11020000 0 0x1000>;
> > +        vio_idx_num = <367 292 242 58>;
> 
> Is the length of this the same as the value of slave_type_num? If so, 
> don't need both.
> 

yes, the length is equal to slave_type_num.
I will try to remove it in the next version.

> > +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> > +        clock-names = "devapc-infra-clock";
> > +    };
> > -- 
> > 2.6.4
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Matthias Brugger]

As a general comment:

Please split your patch in several, one introducing changes to the existing code
base which are needed for newer SoCs (depending on the changes more then one)
and one which actually adds support for the new SoC.

More comments below.


On 26/03/2021 08:31, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> The hardware architecture of mt8192 devapc is slightly
> different from that of the previous IC.
> We add necessary extensions to support mt8192 and be
> back-compatible with other ICs.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
>  1 file changed, 156 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
> index f1cea04..1e40a52 100644
> --- a/drivers/soc/mediatek/mtk-devapc.c
> +++ b/drivers/soc/mediatek/mtk-devapc.c
> @@ -15,6 +15,11 @@
>  #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
>  #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
>  
> +#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
> +	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)

Not really needed, please drop.

> +#define BASE(i)			(ctx->base_list[i])

same here.

> +#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])

same here.

> +
>  struct mtk_devapc_vio_dbgs {
>  	union {
>  		u32 vio_dbg0;
> @@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
>  			u32 addr_h:4;
>  			u32 resv:4;
>  		} dbg0_bits;
> +
> +		/* Not used, reference only */
> +		struct {
> +			u32 dmnid:6;
> +			u32 vio_w:1;
> +			u32 vio_r:1;
> +			u32 addr_h:4;
> +			u32 resv:20;
> +		} dbg0_bits_ver2;
>  	};
>  
>  	u32 vio_dbg1;
> +	u32 vio_dbg2;
>  };
>  
>  struct mtk_devapc_data {
> -	/* numbers of violation index */
> -	u32 vio_idx_num;
> -
>  	/* reg offset */
>  	u32 vio_mask_offset;
>  	u32 vio_sta_offset;
>  	u32 vio_dbg0_offset;
>  	u32 vio_dbg1_offset;
> +	u32 vio_dbg2_offset;
>  	u32 apc_con_offset;
>  	u32 vio_shift_sta_offset;
>  	u32 vio_shift_sel_offset;
> @@ -48,7 +61,10 @@ struct mtk_devapc_data {
>  
>  struct mtk_devapc_context {
>  	struct device *dev;
> -	void __iomem *infra_base;
> +	u32 arch_ver;
> +	u32 slave_type_num;
> +	void __iomem **base_list;
> +	u32 *vio_idx_num;
>  	struct clk *infra_clk;
>  	const struct mtk_devapc_data *data;
>  };
> @@ -56,39 +72,39 @@ struct mtk_devapc_context {
>  static void clear_vio_status(struct mtk_devapc_context *ctx)
>  {
>  	void __iomem *reg;
> -	int i;
> +	int i, j;
>  
> -	reg = ctx->infra_base + ctx->data->vio_sta_offset;
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg = BASE(i) + ctx->data->vio_sta_offset;
>  
> -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> -		writel(GENMASK(31, 0), reg + 4 * i);
> +		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
> +			writel(GENMASK(31, 0), reg + 4 * j);
> +
> +		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
> +		       reg + 4 * j);
> +	}
>  
> -	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
> -	       reg + 4 * i);
>  }
>  
> -static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> +static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
>  {
> -	void __iomem *reg;
>  	u32 val;
>  	int i;
>  
> -	reg = ctx->infra_base + ctx->data->vio_mask_offset;
> -
>  	if (mask)
>  		val = GENMASK(31, 0);
>  	else
>  		val = 0;
>  
> -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> +	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
>  		writel(val, reg + 4 * i);
>  
>  	val = readl(reg + 4 * i);
>  	if (mask)
> -		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> +		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
>  			       0);
>  	else
> -		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> +		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
>  				0);
>  
>  	writel(val, reg + 4 * i);
> @@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
>   */
>  static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
>  {
> +	int i;
> +	void __iomem *reg_base;

Not needed.

>  	void __iomem *pd_vio_shift_sta_reg;
>  	void __iomem *pd_vio_shift_sel_reg;
>  	void __iomem *pd_vio_shift_con_reg;
> @@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
>  	int ret;
>  	u32 val;
>  
> -	pd_vio_shift_sta_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_sta_offset;
> -	pd_vio_shift_sel_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_sel_offset;
> -	pd_vio_shift_con_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_con_offset;
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg_base = BASE(i);
>  
> -	/* Find the minimum shift group which has violation */
> -	val = readl(pd_vio_shift_sta_reg);
> -	if (!val)
> -		return false;
> +		pd_vio_shift_sta_reg = reg_base +
> +				       ctx->data->vio_shift_sta_offset;
> +		pd_vio_shift_sel_reg = reg_base +
> +				       ctx->data->vio_shift_sel_offset;
> +		pd_vio_shift_con_reg = reg_base +
> +				       ctx->data->vio_shift_con_offset;
>  
> -	min_shift_group = __ffs(val);
> +		/* Find the minimum shift group which has violation */
> +		val = readl(pd_vio_shift_sta_reg);
> +		if (!val)
> +			continue;
>  
> -	/* Assign the group to sync */
> -	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> +		min_shift_group = __ffs(val);
>  
> -	/* Start syncing */
> -	writel(0x1, pd_vio_shift_con_reg);
> +		/* Assign the group to sync */
> +		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
>  
> -	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
> -				 PHY_DEVAPC_TIMEOUT);
> -	if (ret) {
> -		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
> -		return false;
> -	}
> +		/* Start syncing */
> +		writel(0x1, pd_vio_shift_con_reg);
> +
> +		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
> +					 0, PHY_DEVAPC_TIMEOUT);
> +		if (ret) {
> +			dev_err(ctx->dev, "%s: Shift violation info failed\n",
> +				__func__);
> +			return -ETIMEDOUT;
> +		}
>  
> -	/* Stop syncing */
> -	writel(0x0, pd_vio_shift_con_reg);
> +		/* Stop syncing */
> +		writel(0x0, pd_vio_shift_con_reg);
>  
> -	/* Write clear */
> -	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> +		/* Write clear */
> +		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
>  
> -	return true;
> +		return i;

Not sure why you change that.

> +	}
> +
> +	/* No violation found */
> +	return -ENODATA;
>  }
>  
>  /*
>   * devapc_extract_vio_dbg - extract full violation information after doing
>   *                          shift mechanism.
>   */
> -static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
> +				   int vio_slave_type)
>  {
>  	struct mtk_devapc_vio_dbgs vio_dbgs;
>  	void __iomem *vio_dbg0_reg;
>  	void __iomem *vio_dbg1_reg;
> +	void __iomem *vio_dbg2_reg;
> +	u32 vio_addr, bus_id;
>  
> -	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
> -	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
> +	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
> +	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
> +	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
>  
>  	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
>  	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
> +	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
> +
> +	switch (ctx->arch_ver) {
> +	case 1:
> +		bus_id = vio_dbgs.dbg0_bits.mstid;
> +		vio_addr = vio_dbgs.vio_dbg1;
> +		break;
> +	case 2:
> +		bus_id = vio_dbgs.vio_dbg1;
> +		vio_addr = vio_dbgs.vio_dbg2;
> +
> +		/* To align with the bit definition of arch_ver 1 */
> +		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
> +		break;
> +	default:
> +		/* Not Supported */
> +		return;
> +	}
>  
>  	/* Print violation information */
>  	if (vio_dbgs.dbg0_bits.vio_w)
> @@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
>  		dev_info(ctx->dev, "Read Violation\n");
>  
>  	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
> -		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
> -		 vio_dbgs.vio_dbg1);
> +		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
>  }
>  
>  /*
> @@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
>  static irqreturn_t devapc_violation_irq(int irq_number, void *data)
>  {
>  	struct mtk_devapc_context *ctx = data;
> +	int vio_slave_type;
>  
> -	while (devapc_sync_vio_dbg(ctx))
> -		devapc_extract_vio_dbg(ctx);
> +	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
> +		devapc_extract_vio_dbg(ctx, vio_slave_type);
>  
>  	clear_vio_status(ctx);
>  
> @@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
>   */
>  static void start_devapc(struct mtk_devapc_context *ctx)
>  {
> -	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
> +	int i;
> +	void __iomem *reg_base;
>  
> -	mask_module_irq(ctx, false);
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
> +
> +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> +		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
> +	}
>  }
>  
>  /*
> @@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
>   */
>  static void stop_devapc(struct mtk_devapc_context *ctx)
>  {
> -	mask_module_irq(ctx, true);
> +	int i;
> +	void __iomem *reg_base;
> +
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> +		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
>  
> -	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
> +		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
> +	}
>  }
>  
>  static const struct mtk_devapc_data devapc_mt6779 = {
> -	.vio_idx_num = 511,
>  	.vio_mask_offset = 0x0,
>  	.vio_sta_offset = 0x400,
>  	.vio_dbg0_offset = 0x900,
> @@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
>  	.vio_shift_con_offset = 0xF20,
>  };
>  
> +static const struct mtk_devapc_data devapc_mt8192 = {
> +	.vio_mask_offset = 0x0,
> +	.vio_sta_offset = 0x400,
> +	.vio_dbg0_offset = 0x900,
> +	.vio_dbg1_offset = 0x904,
> +	.vio_dbg2_offset = 0x908,
> +	.apc_con_offset = 0xF00,
> +	.vio_shift_sta_offset = 0xF20,
> +	.vio_shift_sel_offset = 0xF30,
> +	.vio_shift_con_offset = 0xF10,
> +};
> +
>  static const struct of_device_id mtk_devapc_dt_match[] = {
>  	{
>  		.compatible = "mediatek,mt6779-devapc",
>  		.data = &devapc_mt6779,
>  	}, {
> +		.compatible = "mediatek,mt8192-devapc",
> +		.data = &devapc_mt8192,
> +	}, {
>  	},
>  };
>  
> @@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
>  {
>  	struct device_node *node = pdev->dev.of_node;
>  	struct mtk_devapc_context *ctx;
> +	int i;
>  	u32 devapc_irq;
>  	int ret;
>  
> @@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
>  	ctx->data = of_device_get_match_data(&pdev->dev);
>  	ctx->dev = &pdev->dev;
>  
> -	ctx->infra_base = of_iomap(node, 0);
> -	if (!ctx->infra_base)
> +	if (of_property_read_u32(node, "version", &ctx->arch_ver))
> +		return -EINVAL;
> +
> +	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))

arch_ver and slave_type_num can be ddriver internal parameters set through the
DT data instead of through a new property.

> +		return -EINVAL;
> +
> +	ctx->base_list = devm_kzalloc(&pdev->dev,
> +				      sizeof(void *) * ctx->slave_type_num,
> +				      GFP_KERNEL);
> +	if (!ctx->base_list)
> +		return -ENOMEM;
> +
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		BASE(i) = of_iomap(node, i);
> +		if (!BASE(i))
> +			return -EINVAL;
> +	}
> +
> +	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
> +					sizeof(u32) * ctx->slave_type_num,
> +					GFP_KERNEL);
> +	if (!ctx->vio_idx_num)
> +		return -ENOMEM;
> +
> +	if (of_property_read_u32_array(node, "vio_idx_num",
> +				       ctx->vio_idx_num, ctx->slave_type_num))
>  		return -EINVAL;
>  
>  	devapc_irq = irq_of_parse_and_map(node, 0);
> 

Re: [PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Matthias Brugger]

As a general comment:

Please split your patch in several, one introducing changes to the existing code
base which are needed for newer SoCs (depending on the changes more then one)
and one which actually adds support for the new SoC.

More comments below.


On 26/03/2021 08:31, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> The hardware architecture of mt8192 devapc is slightly
> different from that of the previous IC.
> We add necessary extensions to support mt8192 and be
> back-compatible with other ICs.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
>  1 file changed, 156 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
> index f1cea04..1e40a52 100644
> --- a/drivers/soc/mediatek/mtk-devapc.c
> +++ b/drivers/soc/mediatek/mtk-devapc.c
> @@ -15,6 +15,11 @@
>  #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
>  #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
>  
> +#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
> +	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)

Not really needed, please drop.

> +#define BASE(i)			(ctx->base_list[i])

same here.

> +#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])

same here.

> +
>  struct mtk_devapc_vio_dbgs {
>  	union {
>  		u32 vio_dbg0;
> @@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
>  			u32 addr_h:4;
>  			u32 resv:4;
>  		} dbg0_bits;
> +
> +		/* Not used, reference only */
> +		struct {
> +			u32 dmnid:6;
> +			u32 vio_w:1;
> +			u32 vio_r:1;
> +			u32 addr_h:4;
> +			u32 resv:20;
> +		} dbg0_bits_ver2;
>  	};
>  
>  	u32 vio_dbg1;
> +	u32 vio_dbg2;
>  };
>  
>  struct mtk_devapc_data {
> -	/* numbers of violation index */
> -	u32 vio_idx_num;
> -
>  	/* reg offset */
>  	u32 vio_mask_offset;
>  	u32 vio_sta_offset;
>  	u32 vio_dbg0_offset;
>  	u32 vio_dbg1_offset;
> +	u32 vio_dbg2_offset;
>  	u32 apc_con_offset;
>  	u32 vio_shift_sta_offset;
>  	u32 vio_shift_sel_offset;
> @@ -48,7 +61,10 @@ struct mtk_devapc_data {
>  
>  struct mtk_devapc_context {
>  	struct device *dev;
> -	void __iomem *infra_base;
> +	u32 arch_ver;
> +	u32 slave_type_num;
> +	void __iomem **base_list;
> +	u32 *vio_idx_num;
>  	struct clk *infra_clk;
>  	const struct mtk_devapc_data *data;
>  };
> @@ -56,39 +72,39 @@ struct mtk_devapc_context {
>  static void clear_vio_status(struct mtk_devapc_context *ctx)
>  {
>  	void __iomem *reg;
> -	int i;
> +	int i, j;
>  
> -	reg = ctx->infra_base + ctx->data->vio_sta_offset;
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg = BASE(i) + ctx->data->vio_sta_offset;
>  
> -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> -		writel(GENMASK(31, 0), reg + 4 * i);
> +		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
> +			writel(GENMASK(31, 0), reg + 4 * j);
> +
> +		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
> +		       reg + 4 * j);
> +	}
>  
> -	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
> -	       reg + 4 * i);
>  }
>  
> -static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> +static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
>  {
> -	void __iomem *reg;
>  	u32 val;
>  	int i;
>  
> -	reg = ctx->infra_base + ctx->data->vio_mask_offset;
> -
>  	if (mask)
>  		val = GENMASK(31, 0);
>  	else
>  		val = 0;
>  
> -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> +	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
>  		writel(val, reg + 4 * i);
>  
>  	val = readl(reg + 4 * i);
>  	if (mask)
> -		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> +		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
>  			       0);
>  	else
> -		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> +		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
>  				0);
>  
>  	writel(val, reg + 4 * i);
> @@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
>   */
>  static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
>  {
> +	int i;
> +	void __iomem *reg_base;

Not needed.

>  	void __iomem *pd_vio_shift_sta_reg;
>  	void __iomem *pd_vio_shift_sel_reg;
>  	void __iomem *pd_vio_shift_con_reg;
> @@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
>  	int ret;
>  	u32 val;
>  
> -	pd_vio_shift_sta_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_sta_offset;
> -	pd_vio_shift_sel_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_sel_offset;
> -	pd_vio_shift_con_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_con_offset;
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg_base = BASE(i);
>  
> -	/* Find the minimum shift group which has violation */
> -	val = readl(pd_vio_shift_sta_reg);
> -	if (!val)
> -		return false;
> +		pd_vio_shift_sta_reg = reg_base +
> +				       ctx->data->vio_shift_sta_offset;
> +		pd_vio_shift_sel_reg = reg_base +
> +				       ctx->data->vio_shift_sel_offset;
> +		pd_vio_shift_con_reg = reg_base +
> +				       ctx->data->vio_shift_con_offset;
>  
> -	min_shift_group = __ffs(val);
> +		/* Find the minimum shift group which has violation */
> +		val = readl(pd_vio_shift_sta_reg);
> +		if (!val)
> +			continue;
>  
> -	/* Assign the group to sync */
> -	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> +		min_shift_group = __ffs(val);
>  
> -	/* Start syncing */
> -	writel(0x1, pd_vio_shift_con_reg);
> +		/* Assign the group to sync */
> +		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
>  
> -	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
> -				 PHY_DEVAPC_TIMEOUT);
> -	if (ret) {
> -		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
> -		return false;
> -	}
> +		/* Start syncing */
> +		writel(0x1, pd_vio_shift_con_reg);
> +
> +		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
> +					 0, PHY_DEVAPC_TIMEOUT);
> +		if (ret) {
> +			dev_err(ctx->dev, "%s: Shift violation info failed\n",
> +				__func__);
> +			return -ETIMEDOUT;
> +		}
>  
> -	/* Stop syncing */
> -	writel(0x0, pd_vio_shift_con_reg);
> +		/* Stop syncing */
> +		writel(0x0, pd_vio_shift_con_reg);
>  
> -	/* Write clear */
> -	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> +		/* Write clear */
> +		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
>  
> -	return true;
> +		return i;

Not sure why you change that.

> +	}
> +
> +	/* No violation found */
> +	return -ENODATA;
>  }
>  
>  /*
>   * devapc_extract_vio_dbg - extract full violation information after doing
>   *                          shift mechanism.
>   */
> -static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
> +				   int vio_slave_type)
>  {
>  	struct mtk_devapc_vio_dbgs vio_dbgs;
>  	void __iomem *vio_dbg0_reg;
>  	void __iomem *vio_dbg1_reg;
> +	void __iomem *vio_dbg2_reg;
> +	u32 vio_addr, bus_id;
>  
> -	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
> -	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
> +	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
> +	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
> +	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
>  
>  	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
>  	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
> +	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
> +
> +	switch (ctx->arch_ver) {
> +	case 1:
> +		bus_id = vio_dbgs.dbg0_bits.mstid;
> +		vio_addr = vio_dbgs.vio_dbg1;
> +		break;
> +	case 2:
> +		bus_id = vio_dbgs.vio_dbg1;
> +		vio_addr = vio_dbgs.vio_dbg2;
> +
> +		/* To align with the bit definition of arch_ver 1 */
> +		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
> +		break;
> +	default:
> +		/* Not Supported */
> +		return;
> +	}
>  
>  	/* Print violation information */
>  	if (vio_dbgs.dbg0_bits.vio_w)
> @@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
>  		dev_info(ctx->dev, "Read Violation\n");
>  
>  	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
> -		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
> -		 vio_dbgs.vio_dbg1);
> +		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
>  }
>  
>  /*
> @@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
>  static irqreturn_t devapc_violation_irq(int irq_number, void *data)
>  {
>  	struct mtk_devapc_context *ctx = data;
> +	int vio_slave_type;
>  
> -	while (devapc_sync_vio_dbg(ctx))
> -		devapc_extract_vio_dbg(ctx);
> +	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
> +		devapc_extract_vio_dbg(ctx, vio_slave_type);
>  
>  	clear_vio_status(ctx);
>  
> @@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
>   */
>  static void start_devapc(struct mtk_devapc_context *ctx)
>  {
> -	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
> +	int i;
> +	void __iomem *reg_base;
>  
> -	mask_module_irq(ctx, false);
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
> +
> +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> +		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
> +	}
>  }
>  
>  /*
> @@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
>   */
>  static void stop_devapc(struct mtk_devapc_context *ctx)
>  {
> -	mask_module_irq(ctx, true);
> +	int i;
> +	void __iomem *reg_base;
> +
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> +		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
>  
> -	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
> +		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
> +	}
>  }
>  
>  static const struct mtk_devapc_data devapc_mt6779 = {
> -	.vio_idx_num = 511,
>  	.vio_mask_offset = 0x0,
>  	.vio_sta_offset = 0x400,
>  	.vio_dbg0_offset = 0x900,
> @@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
>  	.vio_shift_con_offset = 0xF20,
>  };
>  
> +static const struct mtk_devapc_data devapc_mt8192 = {
> +	.vio_mask_offset = 0x0,
> +	.vio_sta_offset = 0x400,
> +	.vio_dbg0_offset = 0x900,
> +	.vio_dbg1_offset = 0x904,
> +	.vio_dbg2_offset = 0x908,
> +	.apc_con_offset = 0xF00,
> +	.vio_shift_sta_offset = 0xF20,
> +	.vio_shift_sel_offset = 0xF30,
> +	.vio_shift_con_offset = 0xF10,
> +};
> +
>  static const struct of_device_id mtk_devapc_dt_match[] = {
>  	{
>  		.compatible = "mediatek,mt6779-devapc",
>  		.data = &devapc_mt6779,
>  	}, {
> +		.compatible = "mediatek,mt8192-devapc",
> +		.data = &devapc_mt8192,
> +	}, {
>  	},
>  };
>  
> @@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
>  {
>  	struct device_node *node = pdev->dev.of_node;
>  	struct mtk_devapc_context *ctx;
> +	int i;
>  	u32 devapc_irq;
>  	int ret;
>  
> @@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
>  	ctx->data = of_device_get_match_data(&pdev->dev);
>  	ctx->dev = &pdev->dev;
>  
> -	ctx->infra_base = of_iomap(node, 0);
> -	if (!ctx->infra_base)
> +	if (of_property_read_u32(node, "version", &ctx->arch_ver))
> +		return -EINVAL;
> +
> +	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))

arch_ver and slave_type_num can be ddriver internal parameters set through the
DT data instead of through a new property.

> +		return -EINVAL;
> +
> +	ctx->base_list = devm_kzalloc(&pdev->dev,
> +				      sizeof(void *) * ctx->slave_type_num,
> +				      GFP_KERNEL);
> +	if (!ctx->base_list)
> +		return -ENOMEM;
> +
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		BASE(i) = of_iomap(node, i);
> +		if (!BASE(i))
> +			return -EINVAL;
> +	}
> +
> +	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
> +					sizeof(u32) * ctx->slave_type_num,
> +					GFP_KERNEL);
> +	if (!ctx->vio_idx_num)
> +		return -ENOMEM;
> +
> +	if (of_property_read_u32_array(node, "vio_idx_num",
> +				       ctx->vio_idx_num, ctx->slave_type_num))
>  		return -EINVAL;
>  
>  	devapc_irq = irq_of_parse_and_map(node, 0);
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Matthias Brugger]

As a general comment:

Please split your patch in several, one introducing changes to the existing code
base which are needed for newer SoCs (depending on the changes more then one)
and one which actually adds support for the new SoC.

More comments below.


On 26/03/2021 08:31, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> The hardware architecture of mt8192 devapc is slightly
> different from that of the previous IC.
> We add necessary extensions to support mt8192 and be
> back-compatible with other ICs.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
>  1 file changed, 156 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
> index f1cea04..1e40a52 100644
> --- a/drivers/soc/mediatek/mtk-devapc.c
> +++ b/drivers/soc/mediatek/mtk-devapc.c
> @@ -15,6 +15,11 @@
>  #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
>  #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
>  
> +#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
> +	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)

Not really needed, please drop.

> +#define BASE(i)			(ctx->base_list[i])

same here.

> +#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])

same here.

> +
>  struct mtk_devapc_vio_dbgs {
>  	union {
>  		u32 vio_dbg0;
> @@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
>  			u32 addr_h:4;
>  			u32 resv:4;
>  		} dbg0_bits;
> +
> +		/* Not used, reference only */
> +		struct {
> +			u32 dmnid:6;
> +			u32 vio_w:1;
> +			u32 vio_r:1;
> +			u32 addr_h:4;
> +			u32 resv:20;
> +		} dbg0_bits_ver2;
>  	};
>  
>  	u32 vio_dbg1;
> +	u32 vio_dbg2;
>  };
>  
>  struct mtk_devapc_data {
> -	/* numbers of violation index */
> -	u32 vio_idx_num;
> -
>  	/* reg offset */
>  	u32 vio_mask_offset;
>  	u32 vio_sta_offset;
>  	u32 vio_dbg0_offset;
>  	u32 vio_dbg1_offset;
> +	u32 vio_dbg2_offset;
>  	u32 apc_con_offset;
>  	u32 vio_shift_sta_offset;
>  	u32 vio_shift_sel_offset;
> @@ -48,7 +61,10 @@ struct mtk_devapc_data {
>  
>  struct mtk_devapc_context {
>  	struct device *dev;
> -	void __iomem *infra_base;
> +	u32 arch_ver;
> +	u32 slave_type_num;
> +	void __iomem **base_list;
> +	u32 *vio_idx_num;
>  	struct clk *infra_clk;
>  	const struct mtk_devapc_data *data;
>  };
> @@ -56,39 +72,39 @@ struct mtk_devapc_context {
>  static void clear_vio_status(struct mtk_devapc_context *ctx)
>  {
>  	void __iomem *reg;
> -	int i;
> +	int i, j;
>  
> -	reg = ctx->infra_base + ctx->data->vio_sta_offset;
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg = BASE(i) + ctx->data->vio_sta_offset;
>  
> -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> -		writel(GENMASK(31, 0), reg + 4 * i);
> +		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
> +			writel(GENMASK(31, 0), reg + 4 * j);
> +
> +		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
> +		       reg + 4 * j);
> +	}
>  
> -	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
> -	       reg + 4 * i);
>  }
>  
> -static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> +static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
>  {
> -	void __iomem *reg;
>  	u32 val;
>  	int i;
>  
> -	reg = ctx->infra_base + ctx->data->vio_mask_offset;
> -
>  	if (mask)
>  		val = GENMASK(31, 0);
>  	else
>  		val = 0;
>  
> -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> +	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
>  		writel(val, reg + 4 * i);
>  
>  	val = readl(reg + 4 * i);
>  	if (mask)
> -		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> +		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
>  			       0);
>  	else
> -		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> +		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
>  				0);
>  
>  	writel(val, reg + 4 * i);
> @@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
>   */
>  static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
>  {
> +	int i;
> +	void __iomem *reg_base;

Not needed.

>  	void __iomem *pd_vio_shift_sta_reg;
>  	void __iomem *pd_vio_shift_sel_reg;
>  	void __iomem *pd_vio_shift_con_reg;
> @@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
>  	int ret;
>  	u32 val;
>  
> -	pd_vio_shift_sta_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_sta_offset;
> -	pd_vio_shift_sel_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_sel_offset;
> -	pd_vio_shift_con_reg = ctx->infra_base +
> -			       ctx->data->vio_shift_con_offset;
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg_base = BASE(i);
>  
> -	/* Find the minimum shift group which has violation */
> -	val = readl(pd_vio_shift_sta_reg);
> -	if (!val)
> -		return false;
> +		pd_vio_shift_sta_reg = reg_base +
> +				       ctx->data->vio_shift_sta_offset;
> +		pd_vio_shift_sel_reg = reg_base +
> +				       ctx->data->vio_shift_sel_offset;
> +		pd_vio_shift_con_reg = reg_base +
> +				       ctx->data->vio_shift_con_offset;
>  
> -	min_shift_group = __ffs(val);
> +		/* Find the minimum shift group which has violation */
> +		val = readl(pd_vio_shift_sta_reg);
> +		if (!val)
> +			continue;
>  
> -	/* Assign the group to sync */
> -	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> +		min_shift_group = __ffs(val);
>  
> -	/* Start syncing */
> -	writel(0x1, pd_vio_shift_con_reg);
> +		/* Assign the group to sync */
> +		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
>  
> -	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
> -				 PHY_DEVAPC_TIMEOUT);
> -	if (ret) {
> -		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
> -		return false;
> -	}
> +		/* Start syncing */
> +		writel(0x1, pd_vio_shift_con_reg);
> +
> +		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
> +					 0, PHY_DEVAPC_TIMEOUT);
> +		if (ret) {
> +			dev_err(ctx->dev, "%s: Shift violation info failed\n",
> +				__func__);
> +			return -ETIMEDOUT;
> +		}
>  
> -	/* Stop syncing */
> -	writel(0x0, pd_vio_shift_con_reg);
> +		/* Stop syncing */
> +		writel(0x0, pd_vio_shift_con_reg);
>  
> -	/* Write clear */
> -	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> +		/* Write clear */
> +		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
>  
> -	return true;
> +		return i;

Not sure why you change that.

> +	}
> +
> +	/* No violation found */
> +	return -ENODATA;
>  }
>  
>  /*
>   * devapc_extract_vio_dbg - extract full violation information after doing
>   *                          shift mechanism.
>   */
> -static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
> +				   int vio_slave_type)
>  {
>  	struct mtk_devapc_vio_dbgs vio_dbgs;
>  	void __iomem *vio_dbg0_reg;
>  	void __iomem *vio_dbg1_reg;
> +	void __iomem *vio_dbg2_reg;
> +	u32 vio_addr, bus_id;
>  
> -	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
> -	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
> +	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
> +	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
> +	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
>  
>  	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
>  	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
> +	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
> +
> +	switch (ctx->arch_ver) {
> +	case 1:
> +		bus_id = vio_dbgs.dbg0_bits.mstid;
> +		vio_addr = vio_dbgs.vio_dbg1;
> +		break;
> +	case 2:
> +		bus_id = vio_dbgs.vio_dbg1;
> +		vio_addr = vio_dbgs.vio_dbg2;
> +
> +		/* To align with the bit definition of arch_ver 1 */
> +		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
> +		break;
> +	default:
> +		/* Not Supported */
> +		return;
> +	}
>  
>  	/* Print violation information */
>  	if (vio_dbgs.dbg0_bits.vio_w)
> @@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
>  		dev_info(ctx->dev, "Read Violation\n");
>  
>  	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
> -		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
> -		 vio_dbgs.vio_dbg1);
> +		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
>  }
>  
>  /*
> @@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
>  static irqreturn_t devapc_violation_irq(int irq_number, void *data)
>  {
>  	struct mtk_devapc_context *ctx = data;
> +	int vio_slave_type;
>  
> -	while (devapc_sync_vio_dbg(ctx))
> -		devapc_extract_vio_dbg(ctx);
> +	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
> +		devapc_extract_vio_dbg(ctx, vio_slave_type);
>  
>  	clear_vio_status(ctx);
>  
> @@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
>   */
>  static void start_devapc(struct mtk_devapc_context *ctx)
>  {
> -	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
> +	int i;
> +	void __iomem *reg_base;
>  
> -	mask_module_irq(ctx, false);
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
> +
> +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> +		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
> +	}
>  }
>  
>  /*
> @@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
>   */
>  static void stop_devapc(struct mtk_devapc_context *ctx)
>  {
> -	mask_module_irq(ctx, true);
> +	int i;
> +	void __iomem *reg_base;
> +
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> +		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
>  
> -	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
> +		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
> +	}
>  }
>  
>  static const struct mtk_devapc_data devapc_mt6779 = {
> -	.vio_idx_num = 511,
>  	.vio_mask_offset = 0x0,
>  	.vio_sta_offset = 0x400,
>  	.vio_dbg0_offset = 0x900,
> @@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
>  	.vio_shift_con_offset = 0xF20,
>  };
>  
> +static const struct mtk_devapc_data devapc_mt8192 = {
> +	.vio_mask_offset = 0x0,
> +	.vio_sta_offset = 0x400,
> +	.vio_dbg0_offset = 0x900,
> +	.vio_dbg1_offset = 0x904,
> +	.vio_dbg2_offset = 0x908,
> +	.apc_con_offset = 0xF00,
> +	.vio_shift_sta_offset = 0xF20,
> +	.vio_shift_sel_offset = 0xF30,
> +	.vio_shift_con_offset = 0xF10,
> +};
> +
>  static const struct of_device_id mtk_devapc_dt_match[] = {
>  	{
>  		.compatible = "mediatek,mt6779-devapc",
>  		.data = &devapc_mt6779,
>  	}, {
> +		.compatible = "mediatek,mt8192-devapc",
> +		.data = &devapc_mt8192,
> +	}, {
>  	},
>  };
>  
> @@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
>  {
>  	struct device_node *node = pdev->dev.of_node;
>  	struct mtk_devapc_context *ctx;
> +	int i;
>  	u32 devapc_irq;
>  	int ret;
>  
> @@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
>  	ctx->data = of_device_get_match_data(&pdev->dev);
>  	ctx->dev = &pdev->dev;
>  
> -	ctx->infra_base = of_iomap(node, 0);
> -	if (!ctx->infra_base)
> +	if (of_property_read_u32(node, "version", &ctx->arch_ver))
> +		return -EINVAL;
> +
> +	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))

arch_ver and slave_type_num can be ddriver internal parameters set through the
DT data instead of through a new property.

> +		return -EINVAL;
> +
> +	ctx->base_list = devm_kzalloc(&pdev->dev,
> +				      sizeof(void *) * ctx->slave_type_num,
> +				      GFP_KERNEL);
> +	if (!ctx->base_list)
> +		return -ENOMEM;
> +
> +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> +		BASE(i) = of_iomap(node, i);
> +		if (!BASE(i))
> +			return -EINVAL;
> +	}
> +
> +	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
> +					sizeof(u32) * ctx->slave_type_num,
> +					GFP_KERNEL);
> +	if (!ctx->vio_idx_num)
> +		return -ENOMEM;
> +
> +	if (of_property_read_u32_array(node, "vio_idx_num",
> +				       ctx->vio_idx_num, ctx->slave_type_num))
>  		return -EINVAL;
>  
>  	devapc_irq = irq_of_parse_and_map(node, 0);
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Matthias Brugger]

On 26/03/2021 08:31, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:
> +    description: The number of the devapc set
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>  
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4
> +
> +  vio_idx_num:
> +    description: The number of the devices controlled by devapc
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

This can all per compatible DT data objects in the driver. Don't add new
properties here.

Regards,
Matthias

>      maxItems: 1
>  
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>  
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>  
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;
> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;
> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };
> 

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Matthias Brugger]

On 26/03/2021 08:31, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:
> +    description: The number of the devapc set
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>  
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4
> +
> +  vio_idx_num:
> +    description: The number of the devices controlled by devapc
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

This can all per compatible DT data objects in the driver. Don't add new
properties here.

Regards,
Matthias

>      maxItems: 1
>  
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>  
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>  
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;
> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;
> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Matthias Brugger]

On 26/03/2021 08:31, Nina Wu wrote:
> From: Nina Wu <Nina-CM.Wu@mediatek.com>
> 
> To support newer hardware architecture of devapc,
> update device tree bindings.
> 
> Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 31e4d3c..489f6a9 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -20,9 +20,27 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-devapc
> +      - mediatek,mt8192-devapc
> +
> +  version:
> +    description: The version of the hardware architecture
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    maxItems: 1
> +
> +  slave_type_num:
> +    description: The number of the devapc set
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 4]
> +    maxItems: 1
>  
>    reg:
>      description: The base address of devapc register bank
> +    maxItems: 4
> +
> +  vio_idx_num:
> +    description: The number of the devices controlled by devapc
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

This can all per compatible DT data objects in the driver. Don't add new
properties here.

Regards,
Matthias

>      maxItems: 1
>  
>    interrupts:
> @@ -39,7 +57,10 @@ properties:
>  
>  required:
>    - compatible
> +  - version
> +  - slave_type_num
>    - reg
> +  - vio_idx_num
>    - interrupts
>    - clocks
>    - clock-names
> @@ -53,8 +74,28 @@ examples:
>  
>      devapc: devapc@10207000 {
>        compatible = "mediatek,mt6779-devapc";
> +      version = <1>;
> +      slave_type_num = <1>;
>        reg = <0x10207000 0x1000>;
> +      vio_idx_num = <511>;
>        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
>        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
>        clock-names = "devapc-infra-clock";
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8192-clk.h>
> +
> +    devapc: devapc@10207000 {
> +        compatible = "mediatek,mt8192-devapc";
> +        version = <2>;
> +        slave_type_num = <4>;
> +        reg = <0 0x10207000 0 0x1000>,
> +            <0 0x10274000 0 0x1000>,
> +            <0 0x10275000 0 0x1000>,
> +            <0 0x11020000 0 0x1000>;
> +        vio_idx_num = <367 292 242 58>;
> +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> +        clock-names = "devapc-infra-clock";
> +    };
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Nina Wu]

Hi, Matthias


On Mon, 2021-03-29 at 13:16 +0200, Matthias Brugger wrote:
> As a general comment:
> 
> Please split your patch in several, one introducing changes to the existing code
> base which are needed for newer SoCs (depending on the changes more then one)
> and one which actually adds support for the new SoC.
> 
> More comments below.
> 


Thanks for the advice.
I will try to split it in the next version.

> 
> On 26/03/2021 08:31, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > The hardware architecture of mt8192 devapc is slightly
> > different from that of the previous IC.
> > We add necessary extensions to support mt8192 and be
> > back-compatible with other ICs.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
> >  1 file changed, 156 insertions(+), 57 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
> > index f1cea04..1e40a52 100644
> > --- a/drivers/soc/mediatek/mtk-devapc.c
> > +++ b/drivers/soc/mediatek/mtk-devapc.c
> > @@ -15,6 +15,11 @@
> >  #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
> >  #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
> >  
> > +#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
> > +	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)
> 
> Not really needed, please drop.
> 
> > +#define BASE(i)			(ctx->base_list[i])
> 
> same here.
> 
> > +#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])
> 
> same here.
> 

OK, will fixed in the next version.


> > +
> >  struct mtk_devapc_vio_dbgs {
> >  	union {
> >  		u32 vio_dbg0;
> > @@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
> >  			u32 addr_h:4;
> >  			u32 resv:4;
> >  		} dbg0_bits;
> > +
> > +		/* Not used, reference only */
> > +		struct {
> > +			u32 dmnid:6;
> > +			u32 vio_w:1;
> > +			u32 vio_r:1;
> > +			u32 addr_h:4;
> > +			u32 resv:20;
> > +		} dbg0_bits_ver2;
> >  	};
> >  
> >  	u32 vio_dbg1;
> > +	u32 vio_dbg2;
> >  };
> >  
> >  struct mtk_devapc_data {
> > -	/* numbers of violation index */
> > -	u32 vio_idx_num;
> > -
> >  	/* reg offset */
> >  	u32 vio_mask_offset;
> >  	u32 vio_sta_offset;
> >  	u32 vio_dbg0_offset;
> >  	u32 vio_dbg1_offset;
> > +	u32 vio_dbg2_offset;
> >  	u32 apc_con_offset;
> >  	u32 vio_shift_sta_offset;
> >  	u32 vio_shift_sel_offset;
> > @@ -48,7 +61,10 @@ struct mtk_devapc_data {
> >  
> >  struct mtk_devapc_context {
> >  	struct device *dev;
> > -	void __iomem *infra_base;
> > +	u32 arch_ver;
> > +	u32 slave_type_num;
> > +	void __iomem **base_list;
> > +	u32 *vio_idx_num;
> >  	struct clk *infra_clk;
> >  	const struct mtk_devapc_data *data;
> >  };
> > @@ -56,39 +72,39 @@ struct mtk_devapc_context {
> >  static void clear_vio_status(struct mtk_devapc_context *ctx)
> >  {
> >  	void __iomem *reg;
> > -	int i;
> > +	int i, j;
> >  
> > -	reg = ctx->infra_base + ctx->data->vio_sta_offset;
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg = BASE(i) + ctx->data->vio_sta_offset;
> >  
> > -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> > -		writel(GENMASK(31, 0), reg + 4 * i);
> > +		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
> > +			writel(GENMASK(31, 0), reg + 4 * j);
> > +
> > +		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
> > +		       reg + 4 * j);
> > +	}
> >  
> > -	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
> > -	       reg + 4 * i);
> >  }
> >  
> > -static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> > +static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
> >  {
> > -	void __iomem *reg;
> >  	u32 val;
> >  	int i;
> >  
> > -	reg = ctx->infra_base + ctx->data->vio_mask_offset;
> > -
> >  	if (mask)
> >  		val = GENMASK(31, 0);
> >  	else
> >  		val = 0;
> >  
> > -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> > +	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
> >  		writel(val, reg + 4 * i);
> >  
> >  	val = readl(reg + 4 * i);
> >  	if (mask)
> > -		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> > +		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
> >  			       0);
> >  	else
> > -		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> > +		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
> >  				0);
> >  
> >  	writel(val, reg + 4 * i);
> > @@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> >   */
> >  static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
> >  {
> > +	int i;
> > +	void __iomem *reg_base;
> 
> Not needed.

will be removed.

> 
> >  	void __iomem *pd_vio_shift_sta_reg;
> >  	void __iomem *pd_vio_shift_sel_reg;
> >  	void __iomem *pd_vio_shift_con_reg;
> > @@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
> >  	int ret;
> >  	u32 val;
> >  
> > -	pd_vio_shift_sta_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_sta_offset;
> > -	pd_vio_shift_sel_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_sel_offset;
> > -	pd_vio_shift_con_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_con_offset;
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg_base = BASE(i);
> >  
> > -	/* Find the minimum shift group which has violation */
> > -	val = readl(pd_vio_shift_sta_reg);
> > -	if (!val)
> > -		return false;
> > +		pd_vio_shift_sta_reg = reg_base +
> > +				       ctx->data->vio_shift_sta_offset;
> > +		pd_vio_shift_sel_reg = reg_base +
> > +				       ctx->data->vio_shift_sel_offset;
> > +		pd_vio_shift_con_reg = reg_base +
> > +				       ctx->data->vio_shift_con_offset;
> >  
> > -	min_shift_group = __ffs(val);
> > +		/* Find the minimum shift group which has violation */
> > +		val = readl(pd_vio_shift_sta_reg);
> > +		if (!val)
> > +			continue;
> >  
> > -	/* Assign the group to sync */
> > -	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> > +		min_shift_group = __ffs(val);
> >  
> > -	/* Start syncing */
> > -	writel(0x1, pd_vio_shift_con_reg);
> > +		/* Assign the group to sync */
> > +		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> >  
> > -	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
> > -				 PHY_DEVAPC_TIMEOUT);
> > -	if (ret) {
> > -		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
> > -		return false;
> > -	}
> > +		/* Start syncing */
> > +		writel(0x1, pd_vio_shift_con_reg);
> > +
> > +		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
> > +					 0, PHY_DEVAPC_TIMEOUT);
> > +		if (ret) {
> > +			dev_err(ctx->dev, "%s: Shift violation info failed\n",
> > +				__func__);
> > +			return -ETIMEDOUT;
> > +		}
> >  
> > -	/* Stop syncing */
> > -	writel(0x0, pd_vio_shift_con_reg);
> > +		/* Stop syncing */
> > +		writel(0x0, pd_vio_shift_con_reg);
> >  
> > -	/* Write clear */
> > -	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> > +		/* Write clear */
> > +		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> >  
> > -	return true;
> > +		return i;
> 
> Not sure why you change that.

There is multiple devapc HW here sharing the same interrupt.
When interrupts come, all devapc HW should be checked.
I just got some advice from other reviewer.
I will drop this modification and try to have multiple nodes in DT
instead.

> 
> > +	}
> > +
> > +	/* No violation found */
> > +	return -ENODATA;
> >  }
> >  
> >  /*
> >   * devapc_extract_vio_dbg - extract full violation information after doing
> >   *                          shift mechanism.
> >   */
> > -static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> > +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
> > +				   int vio_slave_type)
> >  {
> >  	struct mtk_devapc_vio_dbgs vio_dbgs;
> >  	void __iomem *vio_dbg0_reg;
> >  	void __iomem *vio_dbg1_reg;
> > +	void __iomem *vio_dbg2_reg;
> > +	u32 vio_addr, bus_id;
> >  
> > -	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
> > -	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
> > +	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
> > +	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
> > +	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
> >  
> >  	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
> >  	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
> > +	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
> > +
> > +	switch (ctx->arch_ver) {
> > +	case 1:
> > +		bus_id = vio_dbgs.dbg0_bits.mstid;
> > +		vio_addr = vio_dbgs.vio_dbg1;
> > +		break;
> > +	case 2:
> > +		bus_id = vio_dbgs.vio_dbg1;
> > +		vio_addr = vio_dbgs.vio_dbg2;
> > +
> > +		/* To align with the bit definition of arch_ver 1 */
> > +		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
> > +		break;
> > +	default:
> > +		/* Not Supported */
> > +		return;
> > +	}
> >  
> >  	/* Print violation information */
> >  	if (vio_dbgs.dbg0_bits.vio_w)
> > @@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> >  		dev_info(ctx->dev, "Read Violation\n");
> >  
> >  	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
> > -		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
> > -		 vio_dbgs.vio_dbg1);
> > +		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
> >  }
> >  
> >  /*
> > @@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> >  static irqreturn_t devapc_violation_irq(int irq_number, void *data)
> >  {
> >  	struct mtk_devapc_context *ctx = data;
> > +	int vio_slave_type;
> >  
> > -	while (devapc_sync_vio_dbg(ctx))
> > -		devapc_extract_vio_dbg(ctx);
> > +	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
> > +		devapc_extract_vio_dbg(ctx, vio_slave_type);
> >  
> >  	clear_vio_status(ctx);
> >  
> > @@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
> >   */
> >  static void start_devapc(struct mtk_devapc_context *ctx)
> >  {
> > -	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
> > +	int i;
> > +	void __iomem *reg_base;
> >  
> > -	mask_module_irq(ctx, false);
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
> > +
> > +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> > +		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
> > +	}
> >  }
> >  
> >  /*
> > @@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
> >   */
> >  static void stop_devapc(struct mtk_devapc_context *ctx)
> >  {
> > -	mask_module_irq(ctx, true);
> > +	int i;
> > +	void __iomem *reg_base;
> > +
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> > +		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
> >  
> > -	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
> > +		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
> > +	}
> >  }
> >  
> >  static const struct mtk_devapc_data devapc_mt6779 = {
> > -	.vio_idx_num = 511,
> >  	.vio_mask_offset = 0x0,
> >  	.vio_sta_offset = 0x400,
> >  	.vio_dbg0_offset = 0x900,
> > @@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
> >  	.vio_shift_con_offset = 0xF20,
> >  };
> >  
> > +static const struct mtk_devapc_data devapc_mt8192 = {
> > +	.vio_mask_offset = 0x0,
> > +	.vio_sta_offset = 0x400,
> > +	.vio_dbg0_offset = 0x900,
> > +	.vio_dbg1_offset = 0x904,
> > +	.vio_dbg2_offset = 0x908,
> > +	.apc_con_offset = 0xF00,
> > +	.vio_shift_sta_offset = 0xF20,
> > +	.vio_shift_sel_offset = 0xF30,
> > +	.vio_shift_con_offset = 0xF10,
> > +};
> > +
> >  static const struct of_device_id mtk_devapc_dt_match[] = {
> >  	{
> >  		.compatible = "mediatek,mt6779-devapc",
> >  		.data = &devapc_mt6779,
> >  	}, {
> > +		.compatible = "mediatek,mt8192-devapc",
> > +		.data = &devapc_mt8192,
> > +	}, {
> >  	},
> >  };
> >  
> > @@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
> >  {
> >  	struct device_node *node = pdev->dev.of_node;
> >  	struct mtk_devapc_context *ctx;
> > +	int i;
> >  	u32 devapc_irq;
> >  	int ret;
> >  
> > @@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
> >  	ctx->data = of_device_get_match_data(&pdev->dev);
> >  	ctx->dev = &pdev->dev;
> >  
> > -	ctx->infra_base = of_iomap(node, 0);
> > -	if (!ctx->infra_base)
> > +	if (of_property_read_u32(node, "version", &ctx->arch_ver))
> > +		return -EINVAL;
> > +
> > +	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))
> 
> arch_ver and slave_type_num can be ddriver internal parameters set through the
> DT data instead of through a new property.
> 
> > +		return -EINVAL;
> > +
> > +	ctx->base_list = devm_kzalloc(&pdev->dev,
> > +				      sizeof(void *) * ctx->slave_type_num,
> > +				      GFP_KERNEL);
> > +	if (!ctx->base_list)
> > +		return -ENOMEM;
> > +
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		BASE(i) = of_iomap(node, i);
> > +		if (!BASE(i))
> > +			return -EINVAL;
> > +	}
> > +
> > +	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
> > +					sizeof(u32) * ctx->slave_type_num,
> > +					GFP_KERNEL);
> > +	if (!ctx->vio_idx_num)
> > +		return -ENOMEM;
> > +
> > +	if (of_property_read_u32_array(node, "vio_idx_num",
> > +				       ctx->vio_idx_num, ctx->slave_type_num))
> >  		return -EINVAL;
> >  
> >  	devapc_irq = irq_of_parse_and_map(node, 0);
> > 

Re: [PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Nina Wu]

Hi, Matthias


On Mon, 2021-03-29 at 13:16 +0200, Matthias Brugger wrote:
> As a general comment:
> 
> Please split your patch in several, one introducing changes to the existing code
> base which are needed for newer SoCs (depending on the changes more then one)
> and one which actually adds support for the new SoC.
> 
> More comments below.
> 


Thanks for the advice.
I will try to split it in the next version.

> 
> On 26/03/2021 08:31, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > The hardware architecture of mt8192 devapc is slightly
> > different from that of the previous IC.
> > We add necessary extensions to support mt8192 and be
> > back-compatible with other ICs.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
> >  1 file changed, 156 insertions(+), 57 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
> > index f1cea04..1e40a52 100644
> > --- a/drivers/soc/mediatek/mtk-devapc.c
> > +++ b/drivers/soc/mediatek/mtk-devapc.c
> > @@ -15,6 +15,11 @@
> >  #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
> >  #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
> >  
> > +#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
> > +	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)
> 
> Not really needed, please drop.
> 
> > +#define BASE(i)			(ctx->base_list[i])
> 
> same here.
> 
> > +#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])
> 
> same here.
> 

OK, will fixed in the next version.


> > +
> >  struct mtk_devapc_vio_dbgs {
> >  	union {
> >  		u32 vio_dbg0;
> > @@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
> >  			u32 addr_h:4;
> >  			u32 resv:4;
> >  		} dbg0_bits;
> > +
> > +		/* Not used, reference only */
> > +		struct {
> > +			u32 dmnid:6;
> > +			u32 vio_w:1;
> > +			u32 vio_r:1;
> > +			u32 addr_h:4;
> > +			u32 resv:20;
> > +		} dbg0_bits_ver2;
> >  	};
> >  
> >  	u32 vio_dbg1;
> > +	u32 vio_dbg2;
> >  };
> >  
> >  struct mtk_devapc_data {
> > -	/* numbers of violation index */
> > -	u32 vio_idx_num;
> > -
> >  	/* reg offset */
> >  	u32 vio_mask_offset;
> >  	u32 vio_sta_offset;
> >  	u32 vio_dbg0_offset;
> >  	u32 vio_dbg1_offset;
> > +	u32 vio_dbg2_offset;
> >  	u32 apc_con_offset;
> >  	u32 vio_shift_sta_offset;
> >  	u32 vio_shift_sel_offset;
> > @@ -48,7 +61,10 @@ struct mtk_devapc_data {
> >  
> >  struct mtk_devapc_context {
> >  	struct device *dev;
> > -	void __iomem *infra_base;
> > +	u32 arch_ver;
> > +	u32 slave_type_num;
> > +	void __iomem **base_list;
> > +	u32 *vio_idx_num;
> >  	struct clk *infra_clk;
> >  	const struct mtk_devapc_data *data;
> >  };
> > @@ -56,39 +72,39 @@ struct mtk_devapc_context {
> >  static void clear_vio_status(struct mtk_devapc_context *ctx)
> >  {
> >  	void __iomem *reg;
> > -	int i;
> > +	int i, j;
> >  
> > -	reg = ctx->infra_base + ctx->data->vio_sta_offset;
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg = BASE(i) + ctx->data->vio_sta_offset;
> >  
> > -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> > -		writel(GENMASK(31, 0), reg + 4 * i);
> > +		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
> > +			writel(GENMASK(31, 0), reg + 4 * j);
> > +
> > +		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
> > +		       reg + 4 * j);
> > +	}
> >  
> > -	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
> > -	       reg + 4 * i);
> >  }
> >  
> > -static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> > +static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
> >  {
> > -	void __iomem *reg;
> >  	u32 val;
> >  	int i;
> >  
> > -	reg = ctx->infra_base + ctx->data->vio_mask_offset;
> > -
> >  	if (mask)
> >  		val = GENMASK(31, 0);
> >  	else
> >  		val = 0;
> >  
> > -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> > +	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
> >  		writel(val, reg + 4 * i);
> >  
> >  	val = readl(reg + 4 * i);
> >  	if (mask)
> > -		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> > +		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
> >  			       0);
> >  	else
> > -		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> > +		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
> >  				0);
> >  
> >  	writel(val, reg + 4 * i);
> > @@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> >   */
> >  static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
> >  {
> > +	int i;
> > +	void __iomem *reg_base;
> 
> Not needed.

will be removed.

> 
> >  	void __iomem *pd_vio_shift_sta_reg;
> >  	void __iomem *pd_vio_shift_sel_reg;
> >  	void __iomem *pd_vio_shift_con_reg;
> > @@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
> >  	int ret;
> >  	u32 val;
> >  
> > -	pd_vio_shift_sta_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_sta_offset;
> > -	pd_vio_shift_sel_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_sel_offset;
> > -	pd_vio_shift_con_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_con_offset;
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg_base = BASE(i);
> >  
> > -	/* Find the minimum shift group which has violation */
> > -	val = readl(pd_vio_shift_sta_reg);
> > -	if (!val)
> > -		return false;
> > +		pd_vio_shift_sta_reg = reg_base +
> > +				       ctx->data->vio_shift_sta_offset;
> > +		pd_vio_shift_sel_reg = reg_base +
> > +				       ctx->data->vio_shift_sel_offset;
> > +		pd_vio_shift_con_reg = reg_base +
> > +				       ctx->data->vio_shift_con_offset;
> >  
> > -	min_shift_group = __ffs(val);
> > +		/* Find the minimum shift group which has violation */
> > +		val = readl(pd_vio_shift_sta_reg);
> > +		if (!val)
> > +			continue;
> >  
> > -	/* Assign the group to sync */
> > -	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> > +		min_shift_group = __ffs(val);
> >  
> > -	/* Start syncing */
> > -	writel(0x1, pd_vio_shift_con_reg);
> > +		/* Assign the group to sync */
> > +		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> >  
> > -	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
> > -				 PHY_DEVAPC_TIMEOUT);
> > -	if (ret) {
> > -		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
> > -		return false;
> > -	}
> > +		/* Start syncing */
> > +		writel(0x1, pd_vio_shift_con_reg);
> > +
> > +		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
> > +					 0, PHY_DEVAPC_TIMEOUT);
> > +		if (ret) {
> > +			dev_err(ctx->dev, "%s: Shift violation info failed\n",
> > +				__func__);
> > +			return -ETIMEDOUT;
> > +		}
> >  
> > -	/* Stop syncing */
> > -	writel(0x0, pd_vio_shift_con_reg);
> > +		/* Stop syncing */
> > +		writel(0x0, pd_vio_shift_con_reg);
> >  
> > -	/* Write clear */
> > -	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> > +		/* Write clear */
> > +		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> >  
> > -	return true;
> > +		return i;
> 
> Not sure why you change that.

There is multiple devapc HW here sharing the same interrupt.
When interrupts come, all devapc HW should be checked.
I just got some advice from other reviewer.
I will drop this modification and try to have multiple nodes in DT
instead.

> 
> > +	}
> > +
> > +	/* No violation found */
> > +	return -ENODATA;
> >  }
> >  
> >  /*
> >   * devapc_extract_vio_dbg - extract full violation information after doing
> >   *                          shift mechanism.
> >   */
> > -static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> > +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
> > +				   int vio_slave_type)
> >  {
> >  	struct mtk_devapc_vio_dbgs vio_dbgs;
> >  	void __iomem *vio_dbg0_reg;
> >  	void __iomem *vio_dbg1_reg;
> > +	void __iomem *vio_dbg2_reg;
> > +	u32 vio_addr, bus_id;
> >  
> > -	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
> > -	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
> > +	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
> > +	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
> > +	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
> >  
> >  	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
> >  	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
> > +	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
> > +
> > +	switch (ctx->arch_ver) {
> > +	case 1:
> > +		bus_id = vio_dbgs.dbg0_bits.mstid;
> > +		vio_addr = vio_dbgs.vio_dbg1;
> > +		break;
> > +	case 2:
> > +		bus_id = vio_dbgs.vio_dbg1;
> > +		vio_addr = vio_dbgs.vio_dbg2;
> > +
> > +		/* To align with the bit definition of arch_ver 1 */
> > +		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
> > +		break;
> > +	default:
> > +		/* Not Supported */
> > +		return;
> > +	}
> >  
> >  	/* Print violation information */
> >  	if (vio_dbgs.dbg0_bits.vio_w)
> > @@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> >  		dev_info(ctx->dev, "Read Violation\n");
> >  
> >  	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
> > -		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
> > -		 vio_dbgs.vio_dbg1);
> > +		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
> >  }
> >  
> >  /*
> > @@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> >  static irqreturn_t devapc_violation_irq(int irq_number, void *data)
> >  {
> >  	struct mtk_devapc_context *ctx = data;
> > +	int vio_slave_type;
> >  
> > -	while (devapc_sync_vio_dbg(ctx))
> > -		devapc_extract_vio_dbg(ctx);
> > +	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
> > +		devapc_extract_vio_dbg(ctx, vio_slave_type);
> >  
> >  	clear_vio_status(ctx);
> >  
> > @@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
> >   */
> >  static void start_devapc(struct mtk_devapc_context *ctx)
> >  {
> > -	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
> > +	int i;
> > +	void __iomem *reg_base;
> >  
> > -	mask_module_irq(ctx, false);
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
> > +
> > +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> > +		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
> > +	}
> >  }
> >  
> >  /*
> > @@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
> >   */
> >  static void stop_devapc(struct mtk_devapc_context *ctx)
> >  {
> > -	mask_module_irq(ctx, true);
> > +	int i;
> > +	void __iomem *reg_base;
> > +
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> > +		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
> >  
> > -	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
> > +		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
> > +	}
> >  }
> >  
> >  static const struct mtk_devapc_data devapc_mt6779 = {
> > -	.vio_idx_num = 511,
> >  	.vio_mask_offset = 0x0,
> >  	.vio_sta_offset = 0x400,
> >  	.vio_dbg0_offset = 0x900,
> > @@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
> >  	.vio_shift_con_offset = 0xF20,
> >  };
> >  
> > +static const struct mtk_devapc_data devapc_mt8192 = {
> > +	.vio_mask_offset = 0x0,
> > +	.vio_sta_offset = 0x400,
> > +	.vio_dbg0_offset = 0x900,
> > +	.vio_dbg1_offset = 0x904,
> > +	.vio_dbg2_offset = 0x908,
> > +	.apc_con_offset = 0xF00,
> > +	.vio_shift_sta_offset = 0xF20,
> > +	.vio_shift_sel_offset = 0xF30,
> > +	.vio_shift_con_offset = 0xF10,
> > +};
> > +
> >  static const struct of_device_id mtk_devapc_dt_match[] = {
> >  	{
> >  		.compatible = "mediatek,mt6779-devapc",
> >  		.data = &devapc_mt6779,
> >  	}, {
> > +		.compatible = "mediatek,mt8192-devapc",
> > +		.data = &devapc_mt8192,
> > +	}, {
> >  	},
> >  };
> >  
> > @@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
> >  {
> >  	struct device_node *node = pdev->dev.of_node;
> >  	struct mtk_devapc_context *ctx;
> > +	int i;
> >  	u32 devapc_irq;
> >  	int ret;
> >  
> > @@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
> >  	ctx->data = of_device_get_match_data(&pdev->dev);
> >  	ctx->dev = &pdev->dev;
> >  
> > -	ctx->infra_base = of_iomap(node, 0);
> > -	if (!ctx->infra_base)
> > +	if (of_property_read_u32(node, "version", &ctx->arch_ver))
> > +		return -EINVAL;
> > +
> > +	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))
> 
> arch_ver and slave_type_num can be ddriver internal parameters set through the
> DT data instead of through a new property.
> 
> > +		return -EINVAL;
> > +
> > +	ctx->base_list = devm_kzalloc(&pdev->dev,
> > +				      sizeof(void *) * ctx->slave_type_num,
> > +				      GFP_KERNEL);
> > +	if (!ctx->base_list)
> > +		return -ENOMEM;
> > +
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		BASE(i) = of_iomap(node, i);
> > +		if (!BASE(i))
> > +			return -EINVAL;
> > +	}
> > +
> > +	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
> > +					sizeof(u32) * ctx->slave_type_num,
> > +					GFP_KERNEL);
> > +	if (!ctx->vio_idx_num)
> > +		return -ENOMEM;
> > +
> > +	if (of_property_read_u32_array(node, "vio_idx_num",
> > +				       ctx->vio_idx_num, ctx->slave_type_num))
> >  		return -EINVAL;
> >  
> >  	devapc_irq = irq_of_parse_and_map(node, 0);
> > 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/2] soc: mediatek: Add mt8192 devapc driver

[Nina Wu]

Hi, Matthias


On Mon, 2021-03-29 at 13:16 +0200, Matthias Brugger wrote:
> As a general comment:
> 
> Please split your patch in several, one introducing changes to the existing code
> base which are needed for newer SoCs (depending on the changes more then one)
> and one which actually adds support for the new SoC.
> 
> More comments below.
> 


Thanks for the advice.
I will try to split it in the next version.

> 
> On 26/03/2021 08:31, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > The hardware architecture of mt8192 devapc is slightly
> > different from that of the previous IC.
> > We add necessary extensions to support mt8192 and be
> > back-compatible with other ICs.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  drivers/soc/mediatek/mtk-devapc.c | 213 ++++++++++++++++++++++++++++----------
> >  1 file changed, 156 insertions(+), 57 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-devapc.c b/drivers/soc/mediatek/mtk-devapc.c
> > index f1cea04..1e40a52 100644
> > --- a/drivers/soc/mediatek/mtk-devapc.c
> > +++ b/drivers/soc/mediatek/mtk-devapc.c
> > @@ -15,6 +15,11 @@
> >  #define VIO_MOD_TO_REG_IND(m)	((m) / 32)
> >  #define VIO_MOD_TO_REG_OFF(m)	((m) % 32)
> >  
> > +#define FOR_EACH_SLAVE_TYPE(ctx, idx) \
> > +	for ((idx) = 0; (idx) < (ctx)->slave_type_num; (idx)++)
> 
> Not really needed, please drop.
> 
> > +#define BASE(i)			(ctx->base_list[i])
> 
> same here.
> 
> > +#define VIO_IDX_NUM(i)		(ctx->vio_idx_num[i])
> 
> same here.
> 

OK, will fixed in the next version.


> > +
> >  struct mtk_devapc_vio_dbgs {
> >  	union {
> >  		u32 vio_dbg0;
> > @@ -26,20 +31,28 @@ struct mtk_devapc_vio_dbgs {
> >  			u32 addr_h:4;
> >  			u32 resv:4;
> >  		} dbg0_bits;
> > +
> > +		/* Not used, reference only */
> > +		struct {
> > +			u32 dmnid:6;
> > +			u32 vio_w:1;
> > +			u32 vio_r:1;
> > +			u32 addr_h:4;
> > +			u32 resv:20;
> > +		} dbg0_bits_ver2;
> >  	};
> >  
> >  	u32 vio_dbg1;
> > +	u32 vio_dbg2;
> >  };
> >  
> >  struct mtk_devapc_data {
> > -	/* numbers of violation index */
> > -	u32 vio_idx_num;
> > -
> >  	/* reg offset */
> >  	u32 vio_mask_offset;
> >  	u32 vio_sta_offset;
> >  	u32 vio_dbg0_offset;
> >  	u32 vio_dbg1_offset;
> > +	u32 vio_dbg2_offset;
> >  	u32 apc_con_offset;
> >  	u32 vio_shift_sta_offset;
> >  	u32 vio_shift_sel_offset;
> > @@ -48,7 +61,10 @@ struct mtk_devapc_data {
> >  
> >  struct mtk_devapc_context {
> >  	struct device *dev;
> > -	void __iomem *infra_base;
> > +	u32 arch_ver;
> > +	u32 slave_type_num;
> > +	void __iomem **base_list;
> > +	u32 *vio_idx_num;
> >  	struct clk *infra_clk;
> >  	const struct mtk_devapc_data *data;
> >  };
> > @@ -56,39 +72,39 @@ struct mtk_devapc_context {
> >  static void clear_vio_status(struct mtk_devapc_context *ctx)
> >  {
> >  	void __iomem *reg;
> > -	int i;
> > +	int i, j;
> >  
> > -	reg = ctx->infra_base + ctx->data->vio_sta_offset;
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg = BASE(i) + ctx->data->vio_sta_offset;
> >  
> > -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> > -		writel(GENMASK(31, 0), reg + 4 * i);
> > +		for (j = 0; j < VIO_MOD_TO_REG_IND(VIO_IDX_NUM(i) - 1); j++)
> > +			writel(GENMASK(31, 0), reg + 4 * j);
> > +
> > +		writel(GENMASK(VIO_MOD_TO_REG_OFF(VIO_IDX_NUM(i) - 1), 0),
> > +		       reg + 4 * j);
> > +	}
> >  
> > -	writel(GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1, 0),
> > -	       reg + 4 * i);
> >  }
> >  
> > -static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> > +static void mask_module_irq(void __iomem *reg, int vio_idx_num, bool mask)
> >  {
> > -	void __iomem *reg;
> >  	u32 val;
> >  	int i;
> >  
> > -	reg = ctx->infra_base + ctx->data->vio_mask_offset;
> > -
> >  	if (mask)
> >  		val = GENMASK(31, 0);
> >  	else
> >  		val = 0;
> >  
> > -	for (i = 0; i < VIO_MOD_TO_REG_IND(ctx->data->vio_idx_num) - 1; i++)
> > +	for (i = 0; i < VIO_MOD_TO_REG_IND(vio_idx_num - 1); i++)
> >  		writel(val, reg + 4 * i);
> >  
> >  	val = readl(reg + 4 * i);
> >  	if (mask)
> > -		val |= GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> > +		val |= GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
> >  			       0);
> >  	else
> > -		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(ctx->data->vio_idx_num) - 1,
> > +		val &= ~GENMASK(VIO_MOD_TO_REG_OFF(vio_idx_num - 1),
> >  				0);
> >  
> >  	writel(val, reg + 4 * i);
> > @@ -108,6 +124,8 @@ static void mask_module_irq(struct mtk_devapc_context *ctx, bool mask)
> >   */
> >  static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
> >  {
> > +	int i;
> > +	void __iomem *reg_base;
> 
> Not needed.

will be removed.

> 
> >  	void __iomem *pd_vio_shift_sta_reg;
> >  	void __iomem *pd_vio_shift_sel_reg;
> >  	void __iomem *pd_vio_shift_con_reg;
> > @@ -115,57 +133,87 @@ static int devapc_sync_vio_dbg(struct mtk_devapc_context *ctx)
> >  	int ret;
> >  	u32 val;
> >  
> > -	pd_vio_shift_sta_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_sta_offset;
> > -	pd_vio_shift_sel_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_sel_offset;
> > -	pd_vio_shift_con_reg = ctx->infra_base +
> > -			       ctx->data->vio_shift_con_offset;
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg_base = BASE(i);
> >  
> > -	/* Find the minimum shift group which has violation */
> > -	val = readl(pd_vio_shift_sta_reg);
> > -	if (!val)
> > -		return false;
> > +		pd_vio_shift_sta_reg = reg_base +
> > +				       ctx->data->vio_shift_sta_offset;
> > +		pd_vio_shift_sel_reg = reg_base +
> > +				       ctx->data->vio_shift_sel_offset;
> > +		pd_vio_shift_con_reg = reg_base +
> > +				       ctx->data->vio_shift_con_offset;
> >  
> > -	min_shift_group = __ffs(val);
> > +		/* Find the minimum shift group which has violation */
> > +		val = readl(pd_vio_shift_sta_reg);
> > +		if (!val)
> > +			continue;
> >  
> > -	/* Assign the group to sync */
> > -	writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> > +		min_shift_group = __ffs(val);
> >  
> > -	/* Start syncing */
> > -	writel(0x1, pd_vio_shift_con_reg);
> > +		/* Assign the group to sync */
> > +		writel(0x1 << min_shift_group, pd_vio_shift_sel_reg);
> >  
> > -	ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3, 0,
> > -				 PHY_DEVAPC_TIMEOUT);
> > -	if (ret) {
> > -		dev_err(ctx->dev, "%s: Shift violation info failed\n", __func__);
> > -		return false;
> > -	}
> > +		/* Start syncing */
> > +		writel(0x1, pd_vio_shift_con_reg);
> > +
> > +		ret = readl_poll_timeout(pd_vio_shift_con_reg, val, val == 0x3,
> > +					 0, PHY_DEVAPC_TIMEOUT);
> > +		if (ret) {
> > +			dev_err(ctx->dev, "%s: Shift violation info failed\n",
> > +				__func__);
> > +			return -ETIMEDOUT;
> > +		}
> >  
> > -	/* Stop syncing */
> > -	writel(0x0, pd_vio_shift_con_reg);
> > +		/* Stop syncing */
> > +		writel(0x0, pd_vio_shift_con_reg);
> >  
> > -	/* Write clear */
> > -	writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> > +		/* Write clear */
> > +		writel(0x1 << min_shift_group, pd_vio_shift_sta_reg);
> >  
> > -	return true;
> > +		return i;
> 
> Not sure why you change that.

There is multiple devapc HW here sharing the same interrupt.
When interrupts come, all devapc HW should be checked.
I just got some advice from other reviewer.
I will drop this modification and try to have multiple nodes in DT
instead.

> 
> > +	}
> > +
> > +	/* No violation found */
> > +	return -ENODATA;
> >  }
> >  
> >  /*
> >   * devapc_extract_vio_dbg - extract full violation information after doing
> >   *                          shift mechanism.
> >   */
> > -static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> > +static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx,
> > +				   int vio_slave_type)
> >  {
> >  	struct mtk_devapc_vio_dbgs vio_dbgs;
> >  	void __iomem *vio_dbg0_reg;
> >  	void __iomem *vio_dbg1_reg;
> > +	void __iomem *vio_dbg2_reg;
> > +	u32 vio_addr, bus_id;
> >  
> > -	vio_dbg0_reg = ctx->infra_base + ctx->data->vio_dbg0_offset;
> > -	vio_dbg1_reg = ctx->infra_base + ctx->data->vio_dbg1_offset;
> > +	vio_dbg0_reg = BASE(vio_slave_type) + ctx->data->vio_dbg0_offset;
> > +	vio_dbg1_reg = BASE(vio_slave_type) + ctx->data->vio_dbg1_offset;
> > +	vio_dbg2_reg = BASE(vio_slave_type) + ctx->data->vio_dbg2_offset;
> >  
> >  	vio_dbgs.vio_dbg0 = readl(vio_dbg0_reg);
> >  	vio_dbgs.vio_dbg1 = readl(vio_dbg1_reg);
> > +	vio_dbgs.vio_dbg2 = readl(vio_dbg2_reg);
> > +
> > +	switch (ctx->arch_ver) {
> > +	case 1:
> > +		bus_id = vio_dbgs.dbg0_bits.mstid;
> > +		vio_addr = vio_dbgs.vio_dbg1;
> > +		break;
> > +	case 2:
> > +		bus_id = vio_dbgs.vio_dbg1;
> > +		vio_addr = vio_dbgs.vio_dbg2;
> > +
> > +		/* To align with the bit definition of arch_ver 1 */
> > +		vio_dbgs.vio_dbg0 = (vio_dbgs.vio_dbg0 << 16);
> > +		break;
> > +	default:
> > +		/* Not Supported */
> > +		return;
> > +	}
> >  
> >  	/* Print violation information */
> >  	if (vio_dbgs.dbg0_bits.vio_w)
> > @@ -174,8 +222,7 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> >  		dev_info(ctx->dev, "Read Violation\n");
> >  
> >  	dev_info(ctx->dev, "Bus ID:0x%x, Dom ID:0x%x, Vio Addr:0x%x\n",
> > -		 vio_dbgs.dbg0_bits.mstid, vio_dbgs.dbg0_bits.dmnid,
> > -		 vio_dbgs.vio_dbg1);
> > +		 bus_id, vio_dbgs.dbg0_bits.dmnid, vio_addr);
> >  }
> >  
> >  /*
> > @@ -186,9 +233,10 @@ static void devapc_extract_vio_dbg(struct mtk_devapc_context *ctx)
> >  static irqreturn_t devapc_violation_irq(int irq_number, void *data)
> >  {
> >  	struct mtk_devapc_context *ctx = data;
> > +	int vio_slave_type;
> >  
> > -	while (devapc_sync_vio_dbg(ctx))
> > -		devapc_extract_vio_dbg(ctx);
> > +	while ((vio_slave_type = devapc_sync_vio_dbg(ctx)) >= 0)
> > +		devapc_extract_vio_dbg(ctx, vio_slave_type);
> >  
> >  	clear_vio_status(ctx);
> >  
> > @@ -200,9 +248,15 @@ static irqreturn_t devapc_violation_irq(int irq_number, void *data)
> >   */
> >  static void start_devapc(struct mtk_devapc_context *ctx)
> >  {
> > -	writel(BIT(31), ctx->infra_base + ctx->data->apc_con_offset);
> > +	int i;
> > +	void __iomem *reg_base;
> >  
> > -	mask_module_irq(ctx, false);
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		writel(BIT(31), BASE(i) + ctx->data->apc_con_offset);
> > +
> > +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> > +		mask_module_irq(reg_base, VIO_IDX_NUM(i), false);
> > +	}
> >  }
> >  
> >  /*
> > @@ -210,13 +264,18 @@ static void start_devapc(struct mtk_devapc_context *ctx)
> >   */
> >  static void stop_devapc(struct mtk_devapc_context *ctx)
> >  {
> > -	mask_module_irq(ctx, true);
> > +	int i;
> > +	void __iomem *reg_base;
> > +
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		reg_base = BASE(i) + ctx->data->vio_mask_offset;
> > +		mask_module_irq(reg_base, VIO_IDX_NUM(i), true);
> >  
> > -	writel(BIT(2), ctx->infra_base + ctx->data->apc_con_offset);
> > +		writel(BIT(2), BASE(i) + ctx->data->apc_con_offset);
> > +	}
> >  }
> >  
> >  static const struct mtk_devapc_data devapc_mt6779 = {
> > -	.vio_idx_num = 511,
> >  	.vio_mask_offset = 0x0,
> >  	.vio_sta_offset = 0x400,
> >  	.vio_dbg0_offset = 0x900,
> > @@ -227,11 +286,26 @@ static const struct mtk_devapc_data devapc_mt6779 = {
> >  	.vio_shift_con_offset = 0xF20,
> >  };
> >  
> > +static const struct mtk_devapc_data devapc_mt8192 = {
> > +	.vio_mask_offset = 0x0,
> > +	.vio_sta_offset = 0x400,
> > +	.vio_dbg0_offset = 0x900,
> > +	.vio_dbg1_offset = 0x904,
> > +	.vio_dbg2_offset = 0x908,
> > +	.apc_con_offset = 0xF00,
> > +	.vio_shift_sta_offset = 0xF20,
> > +	.vio_shift_sel_offset = 0xF30,
> > +	.vio_shift_con_offset = 0xF10,
> > +};
> > +
> >  static const struct of_device_id mtk_devapc_dt_match[] = {
> >  	{
> >  		.compatible = "mediatek,mt6779-devapc",
> >  		.data = &devapc_mt6779,
> >  	}, {
> > +		.compatible = "mediatek,mt8192-devapc",
> > +		.data = &devapc_mt8192,
> > +	}, {
> >  	},
> >  };
> >  
> > @@ -239,6 +313,7 @@ static int mtk_devapc_probe(struct platform_device *pdev)
> >  {
> >  	struct device_node *node = pdev->dev.of_node;
> >  	struct mtk_devapc_context *ctx;
> > +	int i;
> >  	u32 devapc_irq;
> >  	int ret;
> >  
> > @@ -252,8 +327,32 @@ static int mtk_devapc_probe(struct platform_device *pdev)
> >  	ctx->data = of_device_get_match_data(&pdev->dev);
> >  	ctx->dev = &pdev->dev;
> >  
> > -	ctx->infra_base = of_iomap(node, 0);
> > -	if (!ctx->infra_base)
> > +	if (of_property_read_u32(node, "version", &ctx->arch_ver))
> > +		return -EINVAL;
> > +
> > +	if (of_property_read_u32(node, "slave_type_num", &ctx->slave_type_num))
> 
> arch_ver and slave_type_num can be ddriver internal parameters set through the
> DT data instead of through a new property.
> 
> > +		return -EINVAL;
> > +
> > +	ctx->base_list = devm_kzalloc(&pdev->dev,
> > +				      sizeof(void *) * ctx->slave_type_num,
> > +				      GFP_KERNEL);
> > +	if (!ctx->base_list)
> > +		return -ENOMEM;
> > +
> > +	FOR_EACH_SLAVE_TYPE(ctx, i) {
> > +		BASE(i) = of_iomap(node, i);
> > +		if (!BASE(i))
> > +			return -EINVAL;
> > +	}
> > +
> > +	ctx->vio_idx_num = devm_kzalloc(&pdev->dev,
> > +					sizeof(u32) * ctx->slave_type_num,
> > +					GFP_KERNEL);
> > +	if (!ctx->vio_idx_num)
> > +		return -ENOMEM;
> > +
> > +	if (of_property_read_u32_array(node, "vio_idx_num",
> > +				       ctx->vio_idx_num, ctx->slave_type_num))
> >  		return -EINVAL;
> >  
> >  	devapc_irq = irq_of_parse_and_map(node, 0);
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

Hi, Matthias

On Mon, 2021-03-29 at 13:16 +0200, Matthias Brugger wrote:
> 
> On 26/03/2021 08:31, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > To support newer hardware architecture of devapc,
> > update device tree bindings.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > index 31e4d3c..489f6a9 100644
> > --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > @@ -20,9 +20,27 @@ properties:
> >    compatible:
> >      enum:
> >        - mediatek,mt6779-devapc
> > +      - mediatek,mt8192-devapc
> > +
> > +  version:
> > +    description: The version of the hardware architecture
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2]
> > +    maxItems: 1
> > +
> > +  slave_type_num:
> > +    description: The number of the devapc set
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 4]
> > +    maxItems: 1
> >  
> >    reg:
> >      description: The base address of devapc register bank
> > +    maxItems: 4
> > +
> > +  vio_idx_num:
> > +    description: The number of the devices controlled by devapc
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> This can all per compatible DT data objects in the driver. Don't add new
> properties here.

I will try to remove it in the next version
Thanks


> 
> Regards,
> Matthias
> 
> >      maxItems: 1
> >  
> >    interrupts:
> > @@ -39,7 +57,10 @@ properties:
> >  
> >  required:
> >    - compatible
> > +  - version
> > +  - slave_type_num
> >    - reg
> > +  - vio_idx_num
> >    - interrupts
> >    - clocks
> >    - clock-names
> > @@ -53,8 +74,28 @@ examples:
> >  
> >      devapc: devapc@10207000 {
> >        compatible = "mediatek,mt6779-devapc";
> > +      version = <1>;
> > +      slave_type_num = <1>;
> >        reg = <0x10207000 0x1000>;
> > +      vio_idx_num = <511>;
> >        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
> >        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> >        clock-names = "devapc-infra-clock";
> >      };
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/mt8192-clk.h>
> > +
> > +    devapc: devapc@10207000 {
> > +        compatible = "mediatek,mt8192-devapc";
> > +        version = <2>;
> > +        slave_type_num = <4>;
> > +        reg = <0 0x10207000 0 0x1000>,
> > +            <0 0x10274000 0 0x1000>,
> > +            <0 0x10275000 0 0x1000>,
> > +            <0 0x11020000 0 0x1000>;
> > +        vio_idx_num = <367 292 242 58>;
> > +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> > +        clock-names = "devapc-infra-clock";
> > +    };
> > 

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

Hi, Matthias

On Mon, 2021-03-29 at 13:16 +0200, Matthias Brugger wrote:
> 
> On 26/03/2021 08:31, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > To support newer hardware architecture of devapc,
> > update device tree bindings.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > index 31e4d3c..489f6a9 100644
> > --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > @@ -20,9 +20,27 @@ properties:
> >    compatible:
> >      enum:
> >        - mediatek,mt6779-devapc
> > +      - mediatek,mt8192-devapc
> > +
> > +  version:
> > +    description: The version of the hardware architecture
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2]
> > +    maxItems: 1
> > +
> > +  slave_type_num:
> > +    description: The number of the devapc set
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 4]
> > +    maxItems: 1
> >  
> >    reg:
> >      description: The base address of devapc register bank
> > +    maxItems: 4
> > +
> > +  vio_idx_num:
> > +    description: The number of the devices controlled by devapc
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> This can all per compatible DT data objects in the driver. Don't add new
> properties here.

I will try to remove it in the next version
Thanks


> 
> Regards,
> Matthias
> 
> >      maxItems: 1
> >  
> >    interrupts:
> > @@ -39,7 +57,10 @@ properties:
> >  
> >  required:
> >    - compatible
> > +  - version
> > +  - slave_type_num
> >    - reg
> > +  - vio_idx_num
> >    - interrupts
> >    - clocks
> >    - clock-names
> > @@ -53,8 +74,28 @@ examples:
> >  
> >      devapc: devapc@10207000 {
> >        compatible = "mediatek,mt6779-devapc";
> > +      version = <1>;
> > +      slave_type_num = <1>;
> >        reg = <0x10207000 0x1000>;
> > +      vio_idx_num = <511>;
> >        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
> >        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> >        clock-names = "devapc-infra-clock";
> >      };
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/mt8192-clk.h>
> > +
> > +    devapc: devapc@10207000 {
> > +        compatible = "mediatek,mt8192-devapc";
> > +        version = <2>;
> > +        slave_type_num = <4>;
> > +        reg = <0 0x10207000 0 0x1000>,
> > +            <0 0x10274000 0 0x1000>,
> > +            <0 0x10275000 0 0x1000>,
> > +            <0 0x11020000 0 0x1000>;
> > +        vio_idx_num = <367 292 242 58>;
> > +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> > +        clock-names = "devapc-infra-clock";
> > +    };
> > 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/2] dt-bindings: devapc: Update bindings

[Nina Wu]

Hi, Matthias

On Mon, 2021-03-29 at 13:16 +0200, Matthias Brugger wrote:
> 
> On 26/03/2021 08:31, Nina Wu wrote:
> > From: Nina Wu <Nina-CM.Wu@mediatek.com>
> > 
> > To support newer hardware architecture of devapc,
> > update device tree bindings.
> > 
> > Signed-off-by: Nina Wu <Nina-CM.Wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/devapc.yaml   | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > index 31e4d3c..489f6a9 100644
> > --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> > @@ -20,9 +20,27 @@ properties:
> >    compatible:
> >      enum:
> >        - mediatek,mt6779-devapc
> > +      - mediatek,mt8192-devapc
> > +
> > +  version:
> > +    description: The version of the hardware architecture
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2]
> > +    maxItems: 1
> > +
> > +  slave_type_num:
> > +    description: The number of the devapc set
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 4]
> > +    maxItems: 1
> >  
> >    reg:
> >      description: The base address of devapc register bank
> > +    maxItems: 4
> > +
> > +  vio_idx_num:
> > +    description: The number of the devices controlled by devapc
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> This can all per compatible DT data objects in the driver. Don't add new
> properties here.

I will try to remove it in the next version
Thanks


> 
> Regards,
> Matthias
> 
> >      maxItems: 1
> >  
> >    interrupts:
> > @@ -39,7 +57,10 @@ properties:
> >  
> >  required:
> >    - compatible
> > +  - version
> > +  - slave_type_num
> >    - reg
> > +  - vio_idx_num
> >    - interrupts
> >    - clocks
> >    - clock-names
> > @@ -53,8 +74,28 @@ examples:
> >  
> >      devapc: devapc@10207000 {
> >        compatible = "mediatek,mt6779-devapc";
> > +      version = <1>;
> > +      slave_type_num = <1>;
> >        reg = <0x10207000 0x1000>;
> > +      vio_idx_num = <511>;
> >        interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_LOW>;
> >        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> >        clock-names = "devapc-infra-clock";
> >      };
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/mt8192-clk.h>
> > +
> > +    devapc: devapc@10207000 {
> > +        compatible = "mediatek,mt8192-devapc";
> > +        version = <2>;
> > +        slave_type_num = <4>;
> > +        reg = <0 0x10207000 0 0x1000>,
> > +            <0 0x10274000 0 0x1000>,
> > +            <0 0x10275000 0 0x1000>,
> > +            <0 0x11020000 0 0x1000>;
> > +        vio_idx_num = <367 292 242 58>;
> > +        interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&infracfg_ao CLK_INFRA_DEVICE_APC>;
> > +        clock-names = "devapc-infra-clock";
> > +    };
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel