[PATCH v10 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC

[PATCH v10 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC

[Friday Yang]

Based on tag: next-20250805, linux-next/master

On the MediaTek MT8188 SoC platform, we encountered power-off failures
and SMI bus hang issues during camera stress tests. The issue arises
because bus glitches are sometimes produced when MTCMOS powers on or
off. While this is fairly normal, the software must handle these
glitches to avoid mistaking them for transaction signals. What's
more, this issue emerged only after the initial upstreaming of SMI
driver.

The software solutions can be summarized as follows:

1. Use CLAMP to disable the SMI sub-common port after turning off the
   LARB CG and before turning off the LARB MTCMOS.
2. Use CLAMP to disable/enable the SMI sub-common port.
3. Implement an AXI reset for SMI LARBs.

---
Changes in v10:
- Rename 'smi_comm_inport_id' to 'smi_comm_in_port_id'.
- Return 0 when it fails to get 'larb_id' in
  'mtk_smi_larb_parse_clamp_optional'.

Changes in v9:
- Add 'dev_pm_genpd_remove_notifier' in 'mtk_smi_larb_remove'.
- Remove unused macros.
- Rename 'sub_comm_syscon' to 'smi_comm_syscon'.
- Rename 'sub_comm_inport_id' to 'smi_comm_inport_id'.
- Add more detailed descriptions in change log.
- Fix incorrect tags.

Changes in v8:
- Fix incorrect tags.

Changes in v7:
- We replaced 'pm_runtime_enable' with 'devm_pm_runtime_enable' in the
  v6 patch. This changed the order of cleanup, and reviewers expressed
  concerns that it could introduce unexpected issues. So v7 discard this
  change and continue using 'pm_runtime_enable'. We need to conduct
  further investigation to determine if there are any issues related
  to the cleanup order. This might be resolved in the future, but for
  now, we just maintain the current status.

Changes in v6:
- Fix coding style.
- Add another patch to replace 'pm_runtime_enable' with
  'devm_pm_runtime_enable'.

Changes in v5:
- Use 'devm_pm_runtime_enable' instead of 'pm_runtime_enable'.
- Remove 'pm_runtime_disable' in 'mtk_smi_common_remove' and
  'mtk_smi_larb_remove'.

Changes in v4:
- Use 'devm_reset_control_get_optional_exclusive' instead of
  'devm_reset_control_get'.

Changes in v3:
- Remove redundant descriptions for 'resets' and 'reset-names'.
- Modify the requirements for 'resets' and 'reset-names'.
- Rename 'mtk_smi_larb_parse_clamp' to 'mtk_smi_larb_parse_clamp_optional'.
- Rename 'mtk_smi_larb_parse_reset' to 'mtk_smi_larb_parse_reset_optional'.
- Merge 'mtk_smi_larb_clamp_protect_enable' and
  'mtk_smi_larb_clamp_protect_disble' into one function.
- Modify the definition for mtk_smi_larb_clamp_port_mt8188,
  use 'larbid' as the index of the array.
- Use 'syscon_regmap_lookup_by_phandle' instead of 'device_node_to_regmap'.
- Do Not parse 'resets', just check the return value of
  'devm_reset_control_get'.
- Add 'has_gals' flag for 'mtk_smi_sub_common_mt8188'.

Changes in v2:
- According to previous discussions in v1, divided these four
  patches into two topic separately.
- Modify the description for 'resets' in binding.
- Add const value 'larb' for 'reset-names' in binding.
- Modify requirement for 'resets' and 'reset-names' in binding.
- Delete 'mediatek,smi-sub-comm' in binding.
- Delete 'mediatek,smi-sub-comm-in-portid' in binding.
- Modify the example in binding.
- Add 'mtk_smi_larb_clamp_port_mt8188' definition in SMI driver.
- Change the way to parse the 'resets' in driver.
- Change label from 'err_pm_disable' to 'err_link_remove'.

---

Friday Yang (2):
  dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
  memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188

 .../mediatek,smi-common.yaml                  |   2 +
 .../memory-controllers/mediatek,smi-larb.yaml |  19 +++
 drivers/memory/mtk-smi.c                      | 129 ++++++++++++++++++
 3 files changed, 150 insertions(+)

-- 
2.46.0

[PATCH v10 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188

[Friday Yang]

Add 'resets' and 'reset-names' properties for SMI LARBs to support
SMI reset operations.
On the MediaTek platform, some SMI LARBs are directly connected to
the SMI Common, while others are connected to the SMI Sub-Common,
which in turn is connected to the SMI Common. The hardware block
diagram can be described as follows.

             SMI-Common(Smart Multimedia Interface Common)
                 |
         +----------------+------------------+
         |                |                  |
         |                |                  |
         |                |                  |
         |                |                  |
         |                |                  |
       larb0       SMI-Sub-Common0     SMI-Sub-Common1
                   |      |     |      |             |
                  larb1  larb2 larb3  larb7       larb9

Signed-off-by: Friday Yang <friday.yang@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../mediatek,smi-common.yaml                  |  2 ++
 .../memory-controllers/mediatek,smi-larb.yaml | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
index 0762e0ff66ef..3d98c08b2149 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
@@ -40,6 +40,7 @@ properties:
           - mediatek,mt8186-smi-common
           - mediatek,mt8188-smi-common-vdo
           - mediatek,mt8188-smi-common-vpp
+          - mediatek,mt8188-smi-sub-common
           - mediatek,mt8192-smi-common
           - mediatek,mt8195-smi-common-vdo
           - mediatek,mt8195-smi-common-vpp
@@ -108,6 +109,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - mediatek,mt8188-smi-sub-common
               - mediatek,mt8195-smi-sub-common
     then:
       required:
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
index 2e7fac4b5094..fc5feb2eac1f 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
@@ -70,6 +70,12 @@ properties:
     description: the hardware id of this larb. It's only required when this
       hardware id is not consecutive from its M4U point of view.
 
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: larb
+
 required:
   - compatible
   - reg
@@ -126,6 +132,19 @@ allOf:
       required:
         - mediatek,larb-id
 
+  - if:  # only for image, camera and ipe subsys
+      properties:
+        compatible:
+          const: mediatek,mt8188-smi-larb
+        mediatek,larb-id:
+          enum:
+            [ 9, 10, 11, 12, 13, 16, 17, 18, 19, 20 ]
+
+    then:
+      required:
+        - resets
+        - reset-names
+
 additionalProperties: false
 
 examples:
-- 
2.46.0

[PATCH v10 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188

[Friday Yang]

To prevent handling glitch signals during MTCMOS on/off transitions,
SMI requires clamp and reset operations. Parse the reset settings for
SMI LARBs and the clamp settings for the SMI Sub-Common. Register
genpd callback for the SMI LARBs located in image, camera and IPE
subsystems, and apply reset and clamp operations within the callback.

Signed-off-by: Friday Yang <friday.yang@mediatek.com>
---
 drivers/memory/mtk-smi.c | 129 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 733e22f695ab..acc8904dd117 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -10,11 +10,15 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/soc/mediatek/mtk_sip_svc.h>
 #include <soc/mediatek/smi.h>
 #include <dt-bindings/memory/mt2701-larb-port.h>
@@ -34,6 +38,8 @@
 #define SMI_FIFO_TH1			0x238
 #define SMI_FIFO_TH2			0x23c
 #define SMI_DCM				0x300
+#define SMI_COMMON_CLAMP_EN_SET		0x3c4
+#define SMI_COMMON_CLAMP_EN_CLR		0x3c8
 #define SMI_DUMMY			0x444
 
 /* SMI LARB */
@@ -134,6 +140,7 @@ struct mtk_smi_larb_gen {
 	unsigned int			larb_direct_to_common_mask;
 	unsigned int			flags_general;
 	const u8			(*ostd)[SMI_LARB_PORT_NR_MAX];
+	const u8			*clamp_port;
 };
 
 struct mtk_smi {
@@ -150,6 +157,7 @@ struct mtk_smi {
 };
 
 struct mtk_smi_larb { /* larb: local arbiter */
+	struct device			*dev;
 	struct mtk_smi			smi;
 	void __iomem			*base;
 	struct device			*smi_common_dev; /* common or sub-common dev */
@@ -157,6 +165,10 @@ struct mtk_smi_larb { /* larb: local arbiter */
 	int				larbid;
 	u32				*mmu;
 	unsigned char			*bank;
+	struct regmap			*smi_comm_syscon; /* smi-comm or sub-comm */
+	u8				smi_comm_in_port_id; /* smi-comm or sub-comm */
+	struct notifier_block		nb;
+	struct reset_control		*rst_con;
 };
 
 static int
@@ -478,6 +490,19 @@ static const u8 mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX] = {
 	[28] = {0x1a, 0x0e, 0x0a, 0x0a, 0x0c, 0x0e, 0x10,},
 };
 
+static const u8 mtk_smi_larb_clamp_port_mt8188[MTK_LARB_NR_MAX] = {
+	[9]	= BIT(1), /* larb10 */
+	[10]	= BIT(2), /* larb11a */
+	[11]	= BIT(2), /* larb11b */
+	[12]	= BIT(3), /* larb11c */
+	[13]	= BIT(0), /* larb12 */
+	[16]	= BIT(1), /* larb15 */
+	[17]	= BIT(2), /* larb16a */
+	[18]	= BIT(2), /* larb16b */
+	[19]	= BIT(3), /* larb17a */
+	[20]	= BIT(3), /* larb17b */
+};
+
 static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
 	.port_in_larb = {
 		LARB0_PORT_OFFSET, LARB1_PORT_OFFSET,
@@ -531,6 +556,7 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt8188 = {
 	.flags_general	            = MTK_SMI_FLAG_THRT_UPDATE | MTK_SMI_FLAG_SW_FLAG |
 				      MTK_SMI_FLAG_SLEEP_CTL | MTK_SMI_FLAG_CFG_PORT_SEC_CTL,
 	.ostd		            = mtk_smi_larb_mt8188_ostd,
+	.clamp_port                 = mtk_smi_larb_clamp_port_mt8188,
 };
 
 static const struct mtk_smi_larb_gen mtk_smi_larb_mt8192 = {
@@ -582,6 +608,45 @@ static void mtk_smi_larb_sleep_ctrl_disable(struct mtk_smi_larb *larb)
 	writel_relaxed(0, larb->base + SMI_LARB_SLP_CON);
 }
 
+static int mtk_smi_larb_clamp_protect_enable(struct device *dev, bool enable)
+{
+	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
+	u32 reg;
+	int ret;
+
+	/* smi_comm_syscon may be NULL if the subsys doesn't have bus glitch issues */
+	if (!larb->smi_comm_syscon)
+		return -EINVAL;
+
+	reg = enable ? SMI_COMMON_CLAMP_EN_SET : SMI_COMMON_CLAMP_EN_CLR;
+
+	ret = regmap_write(larb->smi_comm_syscon, reg, larb->smi_comm_in_port_id);
+	if (ret)
+		dev_err(dev, "Unable to %s clamp for input port %d: %d\n",
+			enable ? "enable" : "disable",
+			larb->smi_comm_in_port_id, ret);
+
+	return ret;
+}
+
+static int mtk_smi_genpd_callback(struct notifier_block *nb,
+				  unsigned long flags, void *data)
+{
+	struct mtk_smi_larb *larb = container_of(nb, struct mtk_smi_larb, nb);
+	struct device *dev = larb->dev;
+
+	if (flags == GENPD_NOTIFY_PRE_ON || flags == GENPD_NOTIFY_PRE_OFF) {
+		/* disable related SMI sub-common port */
+		mtk_smi_larb_clamp_protect_enable(dev, true);
+	} else if (flags == GENPD_NOTIFY_ON) {
+		/* enable related SMI sub-common port */
+		reset_control_reset(larb->rst_con);
+		mtk_smi_larb_clamp_protect_enable(dev, false);
+	}
+
+	return NOTIFY_OK;
+}
+
 static int mtk_smi_device_link_common(struct device *dev, struct device **com_dev)
 {
 	struct platform_device *smi_com_pdev;
@@ -638,6 +703,51 @@ static int mtk_smi_dts_clk_init(struct device *dev, struct mtk_smi *smi,
 	return ret;
 }
 
+static int mtk_smi_larb_parse_clamp_optional(struct mtk_smi_larb *larb)
+{
+	struct device *dev = larb->dev;
+	const struct mtk_smi_larb_gen *larb_gen = larb->larb_gen;
+	u32 larb_id;
+	int ret;
+
+	/*
+	 * Only SMI LARBs in camera, image and IPE subsys need to
+	 * apply clamp and reset operations, others can be skipped.
+	 */
+	ret = of_property_read_u32(dev->of_node, "mediatek,larb-id", &larb_id);
+	if (ret || !larb_gen->clamp_port || !larb_gen->clamp_port[larb_id])
+		return 0;
+
+	larb->smi_comm_in_port_id = larb_gen->clamp_port[larb_id];
+	larb->smi_comm_syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
+								"mediatek,smi");
+	if (IS_ERR(larb->smi_comm_syscon)) {
+		larb->smi_comm_syscon = NULL;
+		return dev_err_probe(dev, -EINVAL,
+				     "Unknown clamp port for larb %d\n", larb_id);
+	}
+
+	return 0;
+}
+
+static int mtk_smi_larb_parse_reset_optional(struct mtk_smi_larb *larb)
+{
+	struct device *dev = larb->dev;
+	int ret;
+
+	larb->rst_con = devm_reset_control_get_optional_exclusive(dev, "larb");
+	if (!larb->rst_con)
+		return 0;
+
+	larb->nb.notifier_call = mtk_smi_genpd_callback;
+	ret = dev_pm_genpd_add_notifier(dev, &larb->nb);
+	if (ret)
+		return dev_err_probe(dev, -EINVAL,
+				     "Failed to add genpd callback %d\n", ret);
+
+	return 0;
+}
+
 static int mtk_smi_larb_probe(struct platform_device *pdev)
 {
 	struct mtk_smi_larb *larb;
@@ -648,6 +758,7 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
 	if (!larb)
 		return -ENOMEM;
 
+	larb->dev = dev;
 	larb->larb_gen = of_device_get_match_data(dev);
 	larb->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(larb->base))
@@ -664,6 +775,14 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	ret = mtk_smi_larb_parse_clamp_optional(larb);
+	if (ret)
+		goto err_link_remove;
+
+	ret = mtk_smi_larb_parse_reset_optional(larb);
+	if (ret)
+		goto err_link_remove;
+
 	pm_runtime_enable(dev);
 	platform_set_drvdata(pdev, larb);
 	ret = component_add(dev, &mtk_smi_larb_component_ops);
@@ -673,6 +792,7 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
 
 err_pm_disable:
 	pm_runtime_disable(dev);
+err_link_remove:
 	device_link_remove(dev, larb->smi_common_dev);
 	return ret;
 }
@@ -681,6 +801,9 @@ static void mtk_smi_larb_remove(struct platform_device *pdev)
 {
 	struct mtk_smi_larb *larb = platform_get_drvdata(pdev);
 
+	if (larb->nb.notifier_call)
+		dev_pm_genpd_remove_notifier(&pdev->dev);
+
 	device_link_remove(&pdev->dev, larb->smi_common_dev);
 	pm_runtime_disable(&pdev->dev);
 	component_del(&pdev->dev, &mtk_smi_larb_component_ops);
@@ -803,6 +926,11 @@ static const struct mtk_smi_common_plat mtk_smi_common_mt8188_vpp = {
 	.init     = mtk_smi_common_mt8195_init,
 };
 
+static const struct mtk_smi_common_plat mtk_smi_sub_common_mt8188 = {
+	.type     = MTK_SMI_GEN2_SUB_COMM,
+	.has_gals = true,
+};
+
 static const struct mtk_smi_common_plat mtk_smi_common_mt8192 = {
 	.type     = MTK_SMI_GEN2,
 	.has_gals = true,
@@ -847,6 +975,7 @@ static const struct of_device_id mtk_smi_common_of_ids[] = {
 	{.compatible = "mediatek,mt8186-smi-common", .data = &mtk_smi_common_mt8186},
 	{.compatible = "mediatek,mt8188-smi-common-vdo", .data = &mtk_smi_common_mt8188_vdo},
 	{.compatible = "mediatek,mt8188-smi-common-vpp", .data = &mtk_smi_common_mt8188_vpp},
+	{.compatible = "mediatek,mt8188-smi-sub-common", .data = &mtk_smi_sub_common_mt8188},
 	{.compatible = "mediatek,mt8192-smi-common", .data = &mtk_smi_common_mt8192},
 	{.compatible = "mediatek,mt8195-smi-common-vdo", .data = &mtk_smi_common_mt8195_vdo},
 	{.compatible = "mediatek,mt8195-smi-common-vpp", .data = &mtk_smi_common_mt8195_vpp},
-- 
2.46.0

Re: [PATCH v10 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188

[Yong Wu (吴勇)]

On Wed, 2025-08-06 at 16:59 +0800, Friday Yang wrote:
> To prevent handling glitch signals during MTCMOS on/off transitions,
> SMI requires clamp and reset operations. Parse the reset settings for
> SMI LARBs and the clamp settings for the SMI Sub-Common. Register
> genpd callback for the SMI LARBs located in image, camera and IPE
> subsystems, and apply reset and clamp operations within the callback.
> 
> Signed-off-by: Friday Yang <friday.yang@mediatek.com>
> ---
>  drivers/memory/mtk-smi.c | 129
> +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 129 insertions(+)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index 733e22f695ab..acc8904dd117 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -10,11 +10,15 @@
>  #include <linux/err.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
>  #include <linux/soc/mediatek/mtk_sip_svc.h>
>  #include <soc/mediatek/smi.h>
>  #include <dt-bindings/memory/mt2701-larb-port.h>
> @@ -34,6 +38,8 @@
>  #define SMI_FIFO_TH1			0x238
>  #define SMI_FIFO_TH2			0x23c
>  #define SMI_DCM				0x300
> +#define SMI_COMMON_CLAMP_EN_SET		0x3c4
> +#define SMI_COMMON_CLAMP_EN_CLR		0x3c8
>  #define SMI_DUMMY			0x444
>  
>  /* SMI LARB */
> @@ -134,6 +140,7 @@ struct mtk_smi_larb_gen {
>  	unsigned int			larb_direct_to_common_mask;
>  	unsigned int			flags_general;
>  	const u8			(*ostd)[SMI_LARB_PORT_NR_MAX];
> +	const u8			*clamp_port;
>  };
>  
>  struct mtk_smi {
> @@ -150,6 +157,7 @@ struct mtk_smi {
>  };
>  
>  struct mtk_smi_larb { /* larb: local arbiter */
> +	struct device			*dev;
>  	struct mtk_smi			smi;
>  	void __iomem			*base;
>  	struct device			*smi_common_dev; /* common or
> sub-common dev */
> @@ -157,6 +165,10 @@ struct mtk_smi_larb { /* larb: local arbiter */
>  	int				larbid;
>  	u32				*mmu;
>  	unsigned char			*bank;
> +	struct regmap			*smi_comm_syscon; /* smi-comm 
> or sub-comm */
> +	u8				smi_comm_in_port_id; /* smi-
> comm or sub-comm */
> +	struct notifier_block		nb;
> +	struct reset_control		*rst_con;
>  };
>  
>  static int
> @@ -478,6 +490,19 @@ static const u8
> mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX] = {
>  	[28] = {0x1a, 0x0e, 0x0a, 0x0a, 0x0c, 0x0e, 0x10,},
>  };
>  
> +static const u8 mtk_smi_larb_clamp_port_mt8188[MTK_LARB_NR_MAX] = {
> +	[9]	= BIT(1), /* larb10 */
> +	[10]	= BIT(2), /* larb11a */
> +	[11]	= BIT(2), /* larb11b */
> +	[12]	= BIT(3), /* larb11c */
> +	[13]	= BIT(0), /* larb12 */
> +	[16]	= BIT(1), /* larb15 */
> +	[17]	= BIT(2), /* larb16a */
> +	[18]	= BIT(2), /* larb16b */
> +	[19]	= BIT(3), /* larb17a */
> +	[20]	= BIT(3), /* larb17b */
> +};
> +
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
>  	.port_in_larb = {
>  		LARB0_PORT_OFFSET, LARB1_PORT_OFFSET,
> @@ -531,6 +556,7 @@ static const struct mtk_smi_larb_gen
> mtk_smi_larb_mt8188 = {
>  	.flags_general	            = MTK_SMI_FLAG_THRT_UPDATE |
> MTK_SMI_FLAG_SW_FLAG |
>  				      MTK_SMI_FLAG_SLEEP_CTL |
> MTK_SMI_FLAG_CFG_PORT_SEC_CTL,
>  	.ostd		            = mtk_smi_larb_mt8188_ostd,
> +	.clamp_port                 = mtk_smi_larb_clamp_port_mt8188,
>  };
>  
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt8192 = {
> @@ -582,6 +608,45 @@ static void
> mtk_smi_larb_sleep_ctrl_disable(struct mtk_smi_larb *larb)
>  	writel_relaxed(0, larb->base + SMI_LARB_SLP_CON);
>  }
>  
> +static int mtk_smi_larb_clamp_protect_enable(struct device *dev,
> bool enable)
> +{
> +	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> +	u32 reg;
> +	int ret;
> +
> +	/* smi_comm_syscon may be NULL if the subsys doesn't have bus
> glitch issues */
> +	if (!larb->smi_comm_syscon)
> +		return -EINVAL;
> +
> +	reg = enable ? SMI_COMMON_CLAMP_EN_SET :
> SMI_COMMON_CLAMP_EN_CLR;
> +
> +	ret = regmap_write(larb->smi_comm_syscon, reg, larb-
> >smi_comm_in_port_id);
> +	if (ret)
> +		dev_err(dev, "Unable to %s clamp for input port %d:
> %d\n",
> +			enable ? "enable" : "disable",
> +			larb->smi_comm_in_port_id, ret);
> +
> +	return ret;
> +}
> +
> +static int mtk_smi_genpd_callback(struct notifier_block *nb,
> +				  unsigned long flags, void *data)
> +{
> +	struct mtk_smi_larb *larb = container_of(nb, struct
> mtk_smi_larb, nb);
> +	struct device *dev = larb->dev;
> +
> +	if (flags == GENPD_NOTIFY_PRE_ON || flags ==
> GENPD_NOTIFY_PRE_OFF) {
> +		/* disable related SMI sub-common port */
> +		mtk_smi_larb_clamp_protect_enable(dev, true);
> +	} else if (flags == GENPD_NOTIFY_ON) {
> +		/* enable related SMI sub-common port */
> +		reset_control_reset(larb->rst_con);
> +		mtk_smi_larb_clamp_protect_enable(dev, false);
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
>  static int mtk_smi_device_link_common(struct device *dev, struct
> device **com_dev)
>  {
>  	struct platform_device *smi_com_pdev;
> @@ -638,6 +703,51 @@ static int mtk_smi_dts_clk_init(struct device
> *dev, struct mtk_smi *smi,
>  	return ret;
>  }
>  
> +static int mtk_smi_larb_parse_clamp_optional(struct mtk_smi_larb
> *larb)
> +{
> +	struct device *dev = larb->dev;
> +	const struct mtk_smi_larb_gen *larb_gen = larb->larb_gen;
> +	u32 larb_id;
> +	int ret;
> +
> +	/*
> +	 * Only SMI LARBs in camera, image and IPE subsys need to
> +	 * apply clamp and reset operations, others can be skipped.
> +	 */
> +	ret = of_property_read_u32(dev->of_node, "mediatek,larb-id",
> &larb_id);
> +	if (ret || !larb_gen->clamp_port || !larb_gen-
> >clamp_port[larb_id])
> +		return 0;
> +
> +	larb->smi_comm_in_port_id = larb_gen->clamp_port[larb_id];
> +	larb->smi_comm_syscon = syscon_regmap_lookup_by_phandle(dev-
> >of_node,
> +								"mediat
> ek,smi");
> +	if (IS_ERR(larb->smi_comm_syscon)) {
> +		larb->smi_comm_syscon = NULL;
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Unknown clamp port for larb
> %d\n", larb_id);
> +	}
> +
> +	return 0;
> +}
> +
> +static int mtk_smi_larb_parse_reset_optional(struct mtk_smi_larb
> *larb)
> +{
> +	struct device *dev = larb->dev;
> +	int ret;
> +
> +	larb->rst_con = devm_reset_control_get_optional_exclusive(dev,
> "larb");
> +	if (!larb->rst_con)
> +		return 0;
> +
> +	larb->nb.notifier_call = mtk_smi_genpd_callback;
> +	ret = dev_pm_genpd_add_notifier(dev, &larb->nb);
> +	if (ret)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Failed to add genpd callback
> %d\n", ret);
> +
> +	return 0;
> +}
> +
>  static int mtk_smi_larb_probe(struct platform_device *pdev)
>  {
>  	struct mtk_smi_larb *larb;
> @@ -648,6 +758,7 @@ static int mtk_smi_larb_probe(struct
> platform_device *pdev)
>  	if (!larb)
>  		return -ENOMEM;
>  
> +	larb->dev = dev;
>  	larb->larb_gen = of_device_get_match_data(dev);
>  	larb->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(larb->base))
> @@ -664,6 +775,14 @@ static int mtk_smi_larb_probe(struct
> platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = mtk_smi_larb_parse_clamp_optional(larb);
> +	if (ret)
> +		goto err_link_remove;
> +
> +	ret = mtk_smi_larb_parse_reset_optional(larb);
> +	if (ret)
> +		goto err_link_remove;


If we have "larb_gen->clamp_port[larb_id]", we must have the reset
function, is this right? if yes,

if (larb->smi_comm_syscon && larb_gen->clamp_port[larb_id]) {
   ret = mtk_smi_larb_parse_reset(larb);  // Not optional.
}

and then we could remove the checking of larb->smi_comm_syscon in 
mtk_smi_larb_clamp_protect_enable, it will be simpler.

Thanks.

> +
>  	pm_runtime_enable(dev);
>  	platform_set_drvdata(pdev, larb);
>  	ret = component_add(dev, &mtk_smi_larb_component_ops);
> @@ -673,6 +792,7 @@ static int mtk_smi_larb_probe(struct
> platform_device *pdev)
>  
>  err_pm_disable:
>  	pm_runtime_disable(dev);
> +err_link_remove:
>  	device_link_remove(dev, larb->smi_common_dev);
>  	return ret;
>  }
> @@ -681,6 +801,9 @@ static void mtk_smi_larb_remove(struct
> platform_device *pdev)
>  {
>  	struct mtk_smi_larb *larb = platform_get_drvdata(pdev);
>  
> +	if (larb->nb.notifier_call)
> +		dev_pm_genpd_remove_notifier(&pdev->dev);
> +
>  	device_link_remove(&pdev->dev, larb->smi_common_dev);
>  	pm_runtime_disable(&pdev->dev);
>  	component_del(&pdev->dev, &mtk_smi_larb_component_ops);
> @@ -803,6 +926,11 @@ static const struct mtk_smi_common_plat
> mtk_smi_common_mt8188_vpp = {
>  	.init     = mtk_smi_common_mt8195_init,
>  };
>  
> +static const struct mtk_smi_common_plat mtk_smi_sub_common_mt8188 =
> {
> +	.type     = MTK_SMI_GEN2_SUB_COMM,
> +	.has_gals = true,
> +};
> +
>  static const struct mtk_smi_common_plat mtk_smi_common_mt8192 = {
>  	.type     = MTK_SMI_GEN2,
>  	.has_gals = true,
> @@ -847,6 +975,7 @@ static const struct of_device_id
> mtk_smi_common_of_ids[] = {
>  	{.compatible = "mediatek,mt8186-smi-common", .data =
> &mtk_smi_common_mt8186},
>  	{.compatible = "mediatek,mt8188-smi-common-vdo", .data =
> &mtk_smi_common_mt8188_vdo},
>  	{.compatible = "mediatek,mt8188-smi-common-vpp", .data =
> &mtk_smi_common_mt8188_vpp},
> +	{.compatible = "mediatek,mt8188-smi-sub-common", .data =
> &mtk_smi_sub_common_mt8188},
>  	{.compatible = "mediatek,mt8192-smi-common", .data =
> &mtk_smi_common_mt8192},
>  	{.compatible = "mediatek,mt8195-smi-common-vdo", .data =
> &mtk_smi_common_mt8195_vdo},
>  	{.compatible = "mediatek,mt8195-smi-common-vpp", .data =
> &mtk_smi_common_mt8195_vpp},

Re: [PATCH v10 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188

[Philipp Zabel]

On Mi, 2025-08-06 at 16:59 +0800, Friday Yang wrote:
> To prevent handling glitch signals during MTCMOS on/off transitions,
> SMI requires clamp and reset operations. Parse the reset settings for
> SMI LARBs and the clamp settings for the SMI Sub-Common. Register
> genpd callback for the SMI LARBs located in image, camera and IPE
> subsystems, and apply reset and clamp operations within the callback.
> 
> Signed-off-by: Friday Yang <friday.yang@mediatek.com>
> ---
>  drivers/memory/mtk-smi.c | 129 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 129 insertions(+)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index 733e22f695ab..acc8904dd117 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -10,11 +10,15 @@
>  #include <linux/err.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
>  #include <linux/soc/mediatek/mtk_sip_svc.h>
>  #include <soc/mediatek/smi.h>
>  #include <dt-bindings/memory/mt2701-larb-port.h>
> @@ -34,6 +38,8 @@
>  #define SMI_FIFO_TH1			0x238
>  #define SMI_FIFO_TH2			0x23c
>  #define SMI_DCM				0x300
> +#define SMI_COMMON_CLAMP_EN_SET		0x3c4
> +#define SMI_COMMON_CLAMP_EN_CLR		0x3c8
>  #define SMI_DUMMY			0x444
>  
>  /* SMI LARB */
> @@ -134,6 +140,7 @@ struct mtk_smi_larb_gen {
>  	unsigned int			larb_direct_to_common_mask;
>  	unsigned int			flags_general;
>  	const u8			(*ostd)[SMI_LARB_PORT_NR_MAX];
> +	const u8			*clamp_port;
>  };
>  
>  struct mtk_smi {
> @@ -150,6 +157,7 @@ struct mtk_smi {
>  };
>  
>  struct mtk_smi_larb { /* larb: local arbiter */
> +	struct device			*dev;
>  	struct mtk_smi			smi;
>  	void __iomem			*base;
>  	struct device			*smi_common_dev; /* common or sub-common dev */
> @@ -157,6 +165,10 @@ struct mtk_smi_larb { /* larb: local arbiter */
>  	int				larbid;
>  	u32				*mmu;
>  	unsigned char			*bank;
> +	struct regmap			*smi_comm_syscon; /* smi-comm or sub-comm */
> +	u8				smi_comm_in_port_id; /* smi-comm or sub-comm */
> +	struct notifier_block		nb;
> +	struct reset_control		*rst_con;
>  };
>  
>  static int
> @@ -478,6 +490,19 @@ static const u8 mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX] = {
>  	[28] = {0x1a, 0x0e, 0x0a, 0x0a, 0x0c, 0x0e, 0x10,},
>  };
>  
> +static const u8 mtk_smi_larb_clamp_port_mt8188[MTK_LARB_NR_MAX] = {
> +	[9]	= BIT(1), /* larb10 */
> +	[10]	= BIT(2), /* larb11a */
> +	[11]	= BIT(2), /* larb11b */
> +	[12]	= BIT(3), /* larb11c */
> +	[13]	= BIT(0), /* larb12 */
> +	[16]	= BIT(1), /* larb15 */
> +	[17]	= BIT(2), /* larb16a */
> +	[18]	= BIT(2), /* larb16b */
> +	[19]	= BIT(3), /* larb17a */
> +	[20]	= BIT(3), /* larb17b */
> +};
> +
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
>  	.port_in_larb = {
>  		LARB0_PORT_OFFSET, LARB1_PORT_OFFSET,
> @@ -531,6 +556,7 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt8188 = {
>  	.flags_general	            = MTK_SMI_FLAG_THRT_UPDATE | MTK_SMI_FLAG_SW_FLAG |
>  				      MTK_SMI_FLAG_SLEEP_CTL | MTK_SMI_FLAG_CFG_PORT_SEC_CTL,
>  	.ostd		            = mtk_smi_larb_mt8188_ostd,
> +	.clamp_port                 = mtk_smi_larb_clamp_port_mt8188,
>  };
>  
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt8192 = {
> @@ -582,6 +608,45 @@ static void mtk_smi_larb_sleep_ctrl_disable(struct mtk_smi_larb *larb)
>  	writel_relaxed(0, larb->base + SMI_LARB_SLP_CON);
>  }
>  
> +static int mtk_smi_larb_clamp_protect_enable(struct device *dev, bool enable)
> +{
> +	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> +	u32 reg;
> +	int ret;
> +
> +	/* smi_comm_syscon may be NULL if the subsys doesn't have bus glitch issues */
> +	if (!larb->smi_comm_syscon)
> +		return -EINVAL;
> +
> +	reg = enable ? SMI_COMMON_CLAMP_EN_SET : SMI_COMMON_CLAMP_EN_CLR;
> +
> +	ret = regmap_write(larb->smi_comm_syscon, reg, larb->smi_comm_in_port_id);
> +	if (ret)
> +		dev_err(dev, "Unable to %s clamp for input port %d: %d\n",
> +			enable ? "enable" : "disable",
> +			larb->smi_comm_in_port_id, ret);
> +
> +	return ret;
> +}
> +
> +static int mtk_smi_genpd_callback(struct notifier_block *nb,
> +				  unsigned long flags, void *data)
> +{
> +	struct mtk_smi_larb *larb = container_of(nb, struct mtk_smi_larb, nb);
> +	struct device *dev = larb->dev;
> +
> +	if (flags == GENPD_NOTIFY_PRE_ON || flags == GENPD_NOTIFY_PRE_OFF) {
> +		/* disable related SMI sub-common port */
> +		mtk_smi_larb_clamp_protect_enable(dev, true);
> +	} else if (flags == GENPD_NOTIFY_ON) {
> +		/* enable related SMI sub-common port */
> +		reset_control_reset(larb->rst_con);
> +		mtk_smi_larb_clamp_protect_enable(dev, false);
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
>  static int mtk_smi_device_link_common(struct device *dev, struct device **com_dev)
>  {
>  	struct platform_device *smi_com_pdev;
> @@ -638,6 +703,51 @@ static int mtk_smi_dts_clk_init(struct device *dev, struct mtk_smi *smi,
>  	return ret;
>  }
>  
> +static int mtk_smi_larb_parse_clamp_optional(struct mtk_smi_larb *larb)
> +{
> +	struct device *dev = larb->dev;
> +	const struct mtk_smi_larb_gen *larb_gen = larb->larb_gen;
> +	u32 larb_id;
> +	int ret;
> +
> +	/*
> +	 * Only SMI LARBs in camera, image and IPE subsys need to
> +	 * apply clamp and reset operations, others can be skipped.
> +	 */
> +	ret = of_property_read_u32(dev->of_node, "mediatek,larb-id", &larb_id);
> +	if (ret || !larb_gen->clamp_port || !larb_gen->clamp_port[larb_id])
> +		return 0;
> +
> +	larb->smi_comm_in_port_id = larb_gen->clamp_port[larb_id];
> +	larb->smi_comm_syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
> +								"mediatek,smi");
> +	if (IS_ERR(larb->smi_comm_syscon)) {
> +		larb->smi_comm_syscon = NULL;
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Unknown clamp port for larb %d\n", larb_id);
> +	}
> +
> +	return 0;
> +}
> +
> +static int mtk_smi_larb_parse_reset_optional(struct mtk_smi_larb *larb)
> +{
> +	struct device *dev = larb->dev;
> +	int ret;
> +
> +	larb->rst_con = devm_reset_control_get_optional_exclusive(dev, "larb");
> +	if (!larb->rst_con)
> +		return 0;

Please don't ignore errors.

regards
Philipp