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

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

[Friday Yang]

Based on tag: next-20250521, 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 v8:
- 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 I 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.

v7:
https://lore.kernel.org/lkml/20250430094545.23932-2-friday.yang@mediatek.com/
https://lore.kernel.org/lkml/20250430094545.23932-3-friday.yang@mediatek.com/

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                      | 133 ++++++++++++++++++
 3 files changed, 154 insertions(+)

--
2.46.0

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

[Friday Yang]

From: "Friday Yang" <friday.yang@mediatek.com>

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 v8 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188

[Friday Yang]

From: "Friday Yang" <friday.yang@mediatek.com>

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>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Acked-by: Rob Herring <robh@kernel.org>

---
 drivers/memory/mtk-smi.c | 133 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index c086c22511f7..f3745f549629 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>
@@ -36,6 +40,12 @@
 #define SMI_DCM				0x300
 #define SMI_DUMMY			0x444

+#define SMI_COMMON_CLAMP_EN		0x3c0
+#define SMI_COMMON_CLAMP_EN_SET		0x3c4
+#define SMI_COMMON_CLAMP_EN_CLR		0x3c8
+
+#define SMI_SUB_COMM_INPORT_NR		(8)
+
 /* SMI LARB */
 #define SMI_LARB_SLP_CON                0xc
 #define SLP_PROT_EN                     BIT(0)
@@ -134,6 +144,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 +161,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 +169,10 @@ struct mtk_smi_larb { /* larb: local arbiter */
 	int				larbid;
 	u32				*mmu;
 	unsigned char			*bank;
+	struct regmap			*sub_comm_syscon;
+	u8				sub_comm_inport;
+	struct notifier_block		nb;
+	struct reset_control		*rst_con;
 };

 static int
@@ -446,6 +462,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,
@@ -498,6 +527,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 = {
@@ -549,6 +579,46 @@ 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;
+
+	/* sub_comm_syscon could be NULL if larb directly linked to SMI common */
+	if (!larb->sub_comm_syscon)
+		return -EINVAL;
+
+	reg = enable ? SMI_COMMON_CLAMP_EN_SET : SMI_COMMON_CLAMP_EN_CLR;
+
+	ret = regmap_write(larb->sub_comm_syscon, reg,
+			   larb->sub_comm_inport);
+	if (ret)
+		dev_err(dev, "Unable to %s clamp for input port %d: %d\n",
+			enable ? "enable" : "disable",
+			larb->sub_comm_inport, 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;
@@ -605,6 +675,53 @@ 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)
+		return -EINVAL;
+	if (!larb_gen->clamp_port || !larb_gen->clamp_port[larb_id])
+		return 0;
+
+	larb->sub_comm_inport = larb_gen->clamp_port[larb_id];
+	larb->sub_comm_syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
+								"mediatek,smi");
+	if (IS_ERR(larb->sub_comm_syscon)) {
+		larb->sub_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;
@@ -615,6 +732,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))
@@ -631,6 +749,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);
@@ -640,6 +766,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;
 }
@@ -770,6 +897,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,
@@ -814,6 +946,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 v8 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188

[Krzysztof Kozlowski]

On 21/05/2025 08:33, Friday Yang wrote:
> From: "Friday Yang" <friday.yang@mediatek.com>
> 
> 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>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Acked-by: Rob Herring <robh@kernel.org>

You did not respond to previous review. Sending the same while ignoring
previous review is obvious NAK.

Best regards,
Krzysztof

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

[Friday Yang (杨阳)]

On Thu, 2025-06-12 at 17:16 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 21/05/2025 08:33, Friday Yang wrote:
> > From: "Friday Yang" <friday.yang@mediatek.com>
> > 
> > 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>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > Acked-by: Rob Herring <robh@kernel.org>
> 
> You did not respond to previous review. Sending the same while
> ignoring
> previous review is obvious NAK.
> 

Apologies for missing the message. In the v6 patch, I replaced
'pm_runtime_enable' with 'devm_pm_runtime_enable'. You pointed out that
this change might alter the cleanup order and potentially introduce
errors.

v6:
https://lore.kernel.org/lkml/20250408033206.12176-3-friday.yang@mediatek.com/

Therefore, in the v7 patch, I reverted this change and continued using
'pm_runtime_enable' in the SMI driver. However, I did not include a
clear description of the changes between v6 and v7 in the cover letter.

v7:
https://lore.kernel.org/lkml/9f01a9a4-89b2-4bfc-97cd-827be989ef16@kernel.org/

In the v8 patch, I have added a description in the cover letter.
This series just add functions for SMI clamp and not change anything
else. Is this acceptable to you?


> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 24/07/2025 03:49, Friday Yang (杨阳) wrote:
> On Thu, 2025-06-12 at 17:16 +0200, Krzysztof Kozlowski wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 21/05/2025 08:33, Friday Yang wrote:
>>> From: "Friday Yang" <friday.yang@mediatek.com>
>>>
>>> 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>
>>> Reviewed-by: AngeloGioacchino Del Regno <
>>> angelogioacchino.delregno@collabora.com>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>
>> You did not respond to previous review. Sending the same while
>> ignoring
>> previous review is obvious NAK.
>>
> 
> Apologies for missing the message. In the v6 patch, I replaced
> 'pm_runtime_enable' with 'devm_pm_runtime_enable'. You pointed out that
> this change might alter the cleanup order and potentially introduce
> errors.
> 
> v6:
> https://lore.kernel.org/lkml/20250408033206.12176-3-friday.yang@mediatek.com/
> 
> Therefore, in the v7 patch, I reverted this change and continued using
> 'pm_runtime_enable' in the SMI driver. However, I did not include a
> clear description of the changes between v6 and v7 in the cover letter.
> 
> v7:
> https://lore.kernel.org/lkml/9f01a9a4-89b2-4bfc-97cd-827be989ef16@kernel.org/
> 
> In the v8 patch, I have added a description in the cover letter.
> This series just add functions for SMI clamp and not change anything
> else. Is this acceptable to you?

That was month ago. Nothing form this thread is in my memory, nothing is
in the mailbox. There is no cover letter to find anymore.

Anyway, you did not respond to the actual comment and you send the same.
Now you respond... to something else still ignoring the comment about
fake tags.

I will not be wasting more time on this patchset.

Best regards,
Krzysztof

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

[Friday Yang (杨阳)]

On Thu, 2025-07-24 at 09:08 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 24/07/2025 03:49, Friday Yang (杨阳) wrote:
> > On Thu, 2025-06-12 at 17:16 +0200, Krzysztof Kozlowski wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On 21/05/2025 08:33, Friday Yang wrote:
> > > > From: "Friday Yang" <friday.yang@mediatek.com>
> > > > 
> > > > 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>
> > > > Reviewed-by: AngeloGioacchino Del Regno <
> > > > angelogioacchino.delregno@collabora.com>
> > > > Acked-by: Rob Herring <robh@kernel.org>
> > > 
> > > You did not respond to previous review. Sending the same while
> > > ignoring
> > > previous review is obvious NAK.
> > > 
> > 
> > Apologies for missing the message. In the v6 patch, I replaced
> > 'pm_runtime_enable' with 'devm_pm_runtime_enable'. You pointed out
> > that
> > this change might alter the cleanup order and potentially introduce
> > errors.
> > 
> > v6:
> > 
https://lore.kernel.org/lkml/20250408033206.12176-3-friday.yang@mediatek.com/
> > 
> > Therefore, in the v7 patch, I reverted this change and continued
> > using
> > 'pm_runtime_enable' in the SMI driver. However, I did not include a
> > clear description of the changes between v6 and v7 in the cover
> > letter.
> > 
> > v7:
> > 
https://lore.kernel.org/lkml/9f01a9a4-89b2-4bfc-97cd-827be989ef16@kernel.org/
> > 
> > In the v8 patch, I have added a description in the cover letter.
> > This series just add functions for SMI clamp and not change
> > anything
> > else. Is this acceptable to you?
> 
> That was month ago. Nothing form this thread is in my memory, nothing
> is
> in the mailbox. There is no cover letter to find anymore.
> 
> Anyway, you did not respond to the actual comment and you send the
> same.
> Now you respond... to something else still ignoring the comment about
> fake tags.
> 
> I will not be wasting more time on this patchset.
> 

Changes v7:
- Remove the 'devm_pm_runtime_enable' change.

Above is the tag you ask. You are right, the 'devm_pm_runtime_enable'
tag I mentionded in the patchset v6 is truly a fake tag. So please
ignore this tag. What I intended to explain here was that I decided not
to use 'devm_pm_runtime_enable' to replace 'pm_runtime_enable'
functions. Unfortunately, the fake tag didn't explain this clearly in
the changelog, which was my fault. To address this, I updated patchset
v8 to include an explanation.

In patchset v6, I replaced 'pm_runtime_enable' with
'devm_pm_runtime_enable'. However, in patchset v8, I reverted this
change and included the reason for this decision in the changelog.
Apologize for the delay and the trouble again.


> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 24/07/2025 09:59, Friday Yang (杨阳) wrote:
> On Thu, 2025-07-24 at 09:08 +0200, Krzysztof Kozlowski wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 24/07/2025 03:49, Friday Yang (杨阳) wrote:
>>> On Thu, 2025-06-12 at 17:16 +0200, Krzysztof Kozlowski wrote:
>>>> External email : Please do not click links or open attachments
>>>> until
>>>> you have verified the sender or the content.
>>>>
>>>>
>>>> On 21/05/2025 08:33, Friday Yang wrote:
>>>>> From: "Friday Yang" <friday.yang@mediatek.com>
>>>>>
>>>>> 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>
>>>>> Reviewed-by: AngeloGioacchino Del Regno <
>>>>> angelogioacchino.delregno@collabora.com>
>>>>> Acked-by: Rob Herring <robh@kernel.org>
>>>>
>>>> You did not respond to previous review. Sending the same while
>>>> ignoring
>>>> previous review is obvious NAK.
>>>>
>>>
>>> Apologies for missing the message. In the v6 patch, I replaced
>>> 'pm_runtime_enable' with 'devm_pm_runtime_enable'. You pointed out
>>> that
>>> this change might alter the cleanup order and potentially introduce
>>> errors.
>>>
>>> v6:
>>>
> https://lore.kernel.org/lkml/20250408033206.12176-3-friday.yang@mediatek.com/
>>>
>>> Therefore, in the v7 patch, I reverted this change and continued
>>> using
>>> 'pm_runtime_enable' in the SMI driver. However, I did not include a
>>> clear description of the changes between v6 and v7 in the cover
>>> letter.
>>>
>>> v7:
>>>
> https://lore.kernel.org/lkml/9f01a9a4-89b2-4bfc-97cd-827be989ef16@kernel.org/
>>>
>>> In the v8 patch, I have added a description in the cover letter.
>>> This series just add functions for SMI clamp and not change
>>> anything
>>> else. Is this acceptable to you?
>>
>> That was month ago. Nothing form this thread is in my memory, nothing
>> is
>> in the mailbox. There is no cover letter to find anymore.
>>
>> Anyway, you did not respond to the actual comment and you send the
>> same.
>> Now you respond... to something else still ignoring the comment about
>> fake tags.
>>
>> I will not be wasting more time on this patchset.
>>
> 
> Changes v7:
> - Remove the 'devm_pm_runtime_enable' change.
> 
> Above is the tag you ask. You are right, the 'devm_pm_runtime_enable'
> tag I mentionded in the patchset v6 is truly a fake tag. So please

This is a function. Not a tag. I asked about tag.

> ignore this tag. What I intended to explain here was that I decided not
> to use 'devm_pm_runtime_enable' to replace 'pm_runtime_enable'
> functions. Unfortunately, the fake tag didn't explain this clearly in
> the changelog, which was my fault. To address this, I updated patchset
> v8 to include an explanation.
> 
> In patchset v6, I replaced 'pm_runtime_enable' with
> 'devm_pm_runtime_enable'. However, in patchset v8, I reverted this
> change and included the reason for this decision in the changelog.
> Apologize for the delay and the trouble again.

Nothing above is related to my question about the
fake/invented/questioned tag.

Best regards,
Krzysztof

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

[Friday Yang (杨阳)]

On Thu, 2025-07-24 at 10:09 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 24/07/2025 09:59, Friday Yang (杨阳) wrote:
> > On Thu, 2025-07-24 at 09:08 +0200, Krzysztof Kozlowski wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On 24/07/2025 03:49, Friday Yang (杨阳) wrote:
> > > > On Thu, 2025-06-12 at 17:16 +0200, Krzysztof Kozlowski wrote:
> > > > > External email : Please do not click links or open
> > > > > attachments
> > > > > until
> > > > > you have verified the sender or the content.
> > > > > 
> > > > > 
> > > > > On 21/05/2025 08:33, Friday Yang wrote:
> > > > > > From: "Friday Yang" <friday.yang@mediatek.com>
> > > > > > 
> > > > > > 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>
> > > > > > Reviewed-by: AngeloGioacchino Del Regno <
> > > > > > angelogioacchino.delregno@collabora.com>
> > > > > > Acked-by: Rob Herring <robh@kernel.org>
> > > > > 
> > > > > You did not respond to previous review. Sending the same
> > > > > while
> > > > > ignoring
> > > > > previous review is obvious NAK.
> > > > > 
> > > > 
> > > > Apologies for missing the message. In the v6 patch, I replaced
> > > > 'pm_runtime_enable' with 'devm_pm_runtime_enable'. You pointed
> > > > out
> > > > that
> > > > this change might alter the cleanup order and potentially
> > > > introduce
> > > > errors.
> > > > 
> > > > v6:
> > > > 
> > 
> > 
https://lore.kernel.org/lkml/20250408033206.12176-3-friday.yang@mediatek.com/
> > > > 
> > > > Therefore, in the v7 patch, I reverted this change and
> > > > continued
> > > > using
> > > > 'pm_runtime_enable' in the SMI driver. However, I did not
> > > > include a
> > > > clear description of the changes between v6 and v7 in the cover
> > > > letter.
> > > > 
> > > > v7:
> > > > 
> > 
> > 
https://lore.kernel.org/lkml/9f01a9a4-89b2-4bfc-97cd-827be989ef16@kernel.org/
> > > > 
> > > > In the v8 patch, I have added a description in the cover
> > > > letter.
> > > > This series just add functions for SMI clamp and not change
> > > > anything
> > > > else. Is this acceptable to you?
> > > 
> > > That was month ago. Nothing form this thread is in my memory,
> > > nothing
> > > is
> > > in the mailbox. There is no cover letter to find anymore.
> > > 
> > > Anyway, you did not respond to the actual comment and you send
> > > the
> > > same.
> > > Now you respond... to something else still ignoring the comment
> > > about
> > > fake tags.
> > > 
> > > I will not be wasting more time on this patchset.
> > > 
> > 
> > Changes v7:
> > - Remove the 'devm_pm_runtime_enable' change.
> > 
> > Above is the tag you ask. You are right, the
> > 'devm_pm_runtime_enable'
> > tag I mentionded in the patchset v6 is truly a fake tag. So please
> 
> This is a function. Not a tag. I asked about tag.
> 
> > ignore this tag. What I intended to explain here was that I decided
> > not
> > to use 'devm_pm_runtime_enable' to replace 'pm_runtime_enable'
> > functions. Unfortunately, the fake tag didn't explain this clearly
> > in
> > the changelog, which was my fault. To address this, I updated
> > patchset
> > v8 to include an explanation.
> > 
> > In patchset v6, I replaced 'pm_runtime_enable' with
> > 'devm_pm_runtime_enable'. However, in patchset v8, I reverted this
> > change and included the reason for this decision in the changelog.
> > Apologize for the delay and the trouble again.
> 
> Nothing above is related to my question about the
> fake/invented/questioned tag.
> 

I got your point, you refer to the 'reviewed-by' and 'acked-by' tag in
this patch.
These are the tags from two reviewers.

https://lore.kernel.org/lkml/174172361378.44650.15345202042780383326.robh@kernel.org/


https://lore.kernel.org/lkml/9dca8772-6bc0-4105-98d7-e4f804b0d637@collabora.com/

And tag from 'Yong.Wu@mediatek.com' should be deleted. He just reviewed
this patch in mtk internal gerrit and did not make it pubilc. So I
delete this tag from the patch. I could ask him to help reply a
'reviewed-by' tag if necessary.
Thanks for all.


> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 24/07/2025 10:43, Friday Yang (杨阳) wrote:
>>
>>> ignore this tag. What I intended to explain here was that I decided
>>> not
>>> to use 'devm_pm_runtime_enable' to replace 'pm_runtime_enable'
>>> functions. Unfortunately, the fake tag didn't explain this clearly
>>> in
>>> the changelog, which was my fault. To address this, I updated
>>> patchset
>>> v8 to include an explanation.
>>>
>>> In patchset v6, I replaced 'pm_runtime_enable' with
>>> 'devm_pm_runtime_enable'. However, in patchset v8, I reverted this
>>> change and included the reason for this decision in the changelog.
>>> Apologize for the delay and the trouble again.
>>
>> Nothing above is related to my question about the
>> fake/invented/questioned tag.
>>
> 
> I got your point, you refer to the 'reviewed-by' and 'acked-by' tag in
> this patch.
> These are the tags from two reviewers.
> 
> https://lore.kernel.org/lkml/174172361378.44650.15345202042780383326.robh@kernel.org/


You are really not responding to my initial comments and keep dragging
this discussion in some confused directions. Do we talk here about that
patch? No.

Best regards,
Krzysztof

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

[Friday Yang (杨阳)]

On Thu, 2025-07-24 at 10:55 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 24/07/2025 10:43, Friday Yang (杨阳) wrote:
> > > 
> > > > ignore this tag. What I intended to explain here was that I
> > > > decided
> > > > not
> > > > to use 'devm_pm_runtime_enable' to replace 'pm_runtime_enable'
> > > > functions. Unfortunately, the fake tag didn't explain this
> > > > clearly
> > > > in
> > > > the changelog, which was my fault. To address this, I updated
> > > > patchset
> > > > v8 to include an explanation.
> > > > 
> > > > In patchset v6, I replaced 'pm_runtime_enable' with
> > > > 'devm_pm_runtime_enable'. However, in patchset v8, I reverted
> > > > this
> > > > change and included the reason for this decision in the
> > > > changelog.
> > > > Apologize for the delay and the trouble again.
> > > 
> > > Nothing above is related to my question about the
> > > fake/invented/questioned tag.
> > > 
> > 
> > I got your point, you refer to the 'reviewed-by' and 'acked-by' tag
> > in
> > this patch.
> > These are the tags from two reviewers.
> > 
> > 
https://lore.kernel.org/lkml/174172361378.44650.15345202042780383326.robh@kernel.org/
> 
> 
> You are really not responding to my initial comments and keep
> dragging
> this discussion in some confused directions. Do we talk here about
> that
> patch? No.
> 

I understand what you're referring to. For the 'dt-binding' patch, I
can keep the tags. However, for the 'smi driver' patch, I should remove
the tags because there was a change after v6.
I will update v9 soon. Thank you!


> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 28/07/2025 04:59, Friday Yang (杨阳) wrote:
> On Thu, 2025-07-24 at 10:55 +0200, Krzysztof Kozlowski wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 24/07/2025 10:43, Friday Yang (杨阳) wrote:
>>>>
>>>>> ignore this tag. What I intended to explain here was that I
>>>>> decided
>>>>> not
>>>>> to use 'devm_pm_runtime_enable' to replace 'pm_runtime_enable'
>>>>> functions. Unfortunately, the fake tag didn't explain this
>>>>> clearly
>>>>> in
>>>>> the changelog, which was my fault. To address this, I updated
>>>>> patchset
>>>>> v8 to include an explanation.
>>>>>
>>>>> In patchset v6, I replaced 'pm_runtime_enable' with
>>>>> 'devm_pm_runtime_enable'. However, in patchset v8, I reverted
>>>>> this
>>>>> change and included the reason for this decision in the
>>>>> changelog.
>>>>> Apologize for the delay and the trouble again.
>>>>
>>>> Nothing above is related to my question about the
>>>> fake/invented/questioned tag.
>>>>
>>>
>>> I got your point, you refer to the 'reviewed-by' and 'acked-by' tag
>>> in
>>> this patch.
>>> These are the tags from two reviewers.
>>>
>>>
> https://lore.kernel.org/lkml/174172361378.44650.15345202042780383326.robh@kernel.org/
>>
>>
>> You are really not responding to my initial comments and keep
>> dragging
>> this discussion in some confused directions. Do we talk here about
>> that
>> patch? No.
>>
> 
> I understand what you're referring to. For the 'dt-binding' patch, I
> can keep the tags. However, for the 'smi driver' patch, I should remove
> the tags because there was a change after v6.
> I will update v9 soon. Thank you!


No. You never responded to v7 and never addressed the actual comments I
raised.


Best regards,
Krzysztof

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

[Yong Wu (吴勇)]

On Wed, 2025-05-21 at 14:33 +0800, Friday Yang wrote:
> From: "Friday Yang" <friday.yang@mediatek.com>
> 
> 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>
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Acked-by: Rob Herring <robh@kernel.org>
> 
> ---
>  drivers/memory/mtk-smi.c | 133
> +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 133 insertions(+)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index c086c22511f7..f3745f549629 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>
> @@ -36,6 +40,12 @@
>  #define SMI_DCM				0x300
>  #define SMI_DUMMY			0x444
> 
> +#define SMI_COMMON_CLAMP_EN		0x3c0

Not used. remove this.

> +#define SMI_COMMON_CLAMP_EN_SET		0x3c4
> +#define SMI_COMMON_CLAMP_EN_CLR		0x3c8

Sort by the address. put above 0x444.

> +
> +#define SMI_SUB_COMM_INPORT_NR		(8)

Not used. remove this.

> +
>  /* SMI LARB */
>  #define SMI_LARB_SLP_CON                0xc
>  #define SLP_PROT_EN                     BIT(0)
> @@ -134,6 +144,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 +161,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 +169,10 @@ struct mtk_smi_larb { /* larb: local arbiter */
>  	int				larbid;
>  	u32				*mmu;
>  	unsigned char			*bank;
> +	struct regmap			*sub_comm_syscon;

This is also ok when the parent is smi-common, not just smi-sub-comm.
thus, personaly I'd like to rename to something like:

       struct regmap *smi_comm_syscon; /* smi-comm or sub_comm */

> +	u8				sub_comm_inport;
      
also rename to smi_comm_inport_id.

> +	struct notifier_block		nb;
> +	struct reset_control		*rst_con;
>  };
> 
>  static int
> @@ -446,6 +462,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,
> @@ -498,6 +527,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 = {
> @@ -549,6 +579,46 @@ 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;
> +
> +	/* sub_comm_syscon could be NULL if larb directly linked to SMI
> common */
> +	if (!larb->sub_comm_syscon)
> +		return -EINVAL;
> +
> +	reg = enable ? SMI_COMMON_CLAMP_EN_SET :
> SMI_COMMON_CLAMP_EN_CLR;
> +
> +	ret = regmap_write(larb->sub_comm_syscon, reg,
> +			   larb->sub_comm_inport);

one line.

> +	if (ret)
> +		dev_err(dev, "Unable to %s clamp for input port %d:
> %d\n",
> +			enable ? "enable" : "disable",
> +			larb->sub_comm_inport, 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;
> @@ -605,6 +675,53 @@ 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)
> +		return -EINVAL;
> +	if (!larb_gen->clamp_port || !larb_gen->clamp_port[larb_id])
> +		return 0;
> +
> +	larb->sub_comm_inport = larb_gen->clamp_port[larb_id];
> +	larb->sub_comm_syscon = syscon_regmap_lookup_by_phandle(dev-
> >of_node,
> +								"mediat
> ek,smi");
> +	if (IS_ERR(larb->sub_comm_syscon)) {
> +		larb->sub_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);

Add dev_pm_genpd_remove_notifier in larb_remove.

Thanks.

> +	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;
> @@ -615,6 +732,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))
> @@ -631,6 +749,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);
> @@ -640,6 +766,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;
>  }
> @@ -770,6 +897,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,
> @@ -814,6 +946,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 v8 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188

[Friday Yang (杨阳)]

On Sat, 2025-08-02 at 01:25 +0000, Yong Wu (吴勇) wrote:
> On Wed, 2025-05-21 at 14:33 +0800, Friday Yang wrote:
> > From: "Friday Yang" <friday.yang@mediatek.com>
> > 
> > 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>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > Acked-by: Rob Herring <robh@kernel.org>
> > 
> > ---
> >  drivers/memory/mtk-smi.c | 133
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 133 insertions(+)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index c086c22511f7..f3745f549629 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>
> > @@ -36,6 +40,12 @@
> >  #define SMI_DCM				0x300
> >  #define SMI_DUMMY			0x444
> > 
> > +#define SMI_COMMON_CLAMP_EN		0x3c0
> 
> Not used. remove this.
> 

Thanks, I will remove this.

> > +#define SMI_COMMON_CLAMP_EN_SET		0x3c4
> > +#define SMI_COMMON_CLAMP_EN_CLR		0x3c8
> 
> Sort by the address. put above 0x444.
> 

ACK

> > +
> > +#define SMI_SUB_COMM_INPORT_NR		(8)
> 
> Not used. remove this.
> 
ACK

> > +
> >  /* SMI LARB */
> >  #define SMI_LARB_SLP_CON                0xc
> >  #define SLP_PROT_EN                     BIT(0)
> > @@ -134,6 +144,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 +161,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 +169,10 @@ struct mtk_smi_larb { /* larb: local arbiter
> > */
> >  	int				larbid;
> >  	u32				*mmu;
> >  	unsigned char			*bank;
> > +	struct regmap			*sub_comm_syscon;
> 
> This is also ok when the parent is smi-common, not just smi-sub-comm.
> thus, personaly I'd like to rename to something like:
> 
>        struct regmap *smi_comm_syscon; /* smi-comm or sub_comm */
> 
ACK

> > +	u8				sub_comm_inport;
> 
>       
> also rename to smi_comm_inport_id.
ACK

> 
> > +	struct notifier_block		nb;
> > +	struct reset_control		*rst_con;
> >  };
> > 
> >  static int
> > @@ -446,6 +462,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,
> > @@ -498,6 +527,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 = {
> > @@ -549,6 +579,46 @@ 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;
> > +
> > +	/* sub_comm_syscon could be NULL if larb directly linked to SMI
> > common */
> > +	if (!larb->sub_comm_syscon)
> > +		return -EINVAL;
> > +
> > +	reg = enable ? SMI_COMMON_CLAMP_EN_SET :
> > SMI_COMMON_CLAMP_EN_CLR;
> > +
> > +	ret = regmap_write(larb->sub_comm_syscon, reg,
> > +			   larb->sub_comm_inport);
> 
> one line.
ACK

> 
> > +	if (ret)
> > +		dev_err(dev, "Unable to %s clamp for input port %d:
> > %d\n",
> > +			enable ? "enable" : "disable",
> > +			larb->sub_comm_inport, 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;
> > @@ -605,6 +675,53 @@ 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)
> > +		return -EINVAL;
> > +	if (!larb_gen->clamp_port || !larb_gen->clamp_port[larb_id])
> > +		return 0;
> > +
> > +	larb->sub_comm_inport = larb_gen->clamp_port[larb_id];
> > +	larb->sub_comm_syscon = syscon_regmap_lookup_by_phandle(dev-
> > > of_node,
> > 
> > +								"mediat
> > ek,smi");
> > +	if (IS_ERR(larb->sub_comm_syscon)) {
> > +		larb->sub_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);
> 
> Add dev_pm_genpd_remove_notifier in larb_remove.
> 
> Thanks.
> 
You are right, the genpd callback is required to be removed here.
In order to avoid calling it again when smi larb removing. This may
result in the use of NULL pointers or wild pointers, which could cause
system crashes. Thanks.

> > +	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;
> > @@ -615,6 +732,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))
> > @@ -631,6 +749,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);
> > @@ -640,6 +766,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;
> >  }
> > @@ -770,6 +897,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,
> > @@ -814,6 +946,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
> >