* [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC
@ 2025-02-21 7:48 Friday Yang
2025-02-21 7:48 ` [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188 Friday Yang
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Friday Yang @ 2025-02-21 7:48 UTC (permalink / raw)
To: Yong Wu, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Philipp Zabel
Cc: Friday Yang, linux-mediatek, linux-kernel, devicetree,
linux-arm-kernel, Project_Global_Chrome_Upstream_Group
Based on tag: next-20250220, 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 v4:
- Use 'devm_reset_control_get_optional_exclusive' instead of
'devm_reset_control_get'.
v3:
https://lore.kernel.org/lkml/20250121064934.13482-2-
friday.yang@mediatek.com/
https://lore.kernel.org/lkml/20250121064934.13482-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 | 20 +++
drivers/memory/mtk-smi.c | 141 +++++++++++++++++-
3 files changed, 159 insertions(+), 4 deletions(-)
--
2.46.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
2025-02-21 7:48 [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC Friday Yang
@ 2025-02-21 7:48 ` Friday Yang
2025-02-24 8:41 ` AngeloGioacchino Del Regno
2025-02-21 7:48 ` [PATCH v4 2/2] memory: mtk-smi: mt8188: " Friday Yang
2025-02-22 9:45 ` [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC Krzysztof Kozlowski
2 siblings, 1 reply; 14+ messages in thread
From: Friday Yang @ 2025-02-21 7:48 UTC (permalink / raw)
To: Yong Wu, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Philipp Zabel
Cc: Friday Yang, linux-mediatek, linux-kernel, devicetree,
linux-arm-kernel, Project_Global_Chrome_Upstream_Group
From: "Friday Yang" <friday.yang@mediatek.com>
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
For previous discussion on the direction of the code modifications,
please refer to:
https://lore.kernel.org/all/CAFGrd9qZhObQXvm2_abqaX83xMLqxjQETB2=
wXpobDWU1CnvkA@mail.gmail.com/
https://lore.kernel.org/all/CAPDyKFpokXV2gJDgowbixTvOH_5VL3B5H8ey
hP+KJ5Fasm2rFg@mail.gmail.com/
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 this
binding. Without these patches, the SMI becomes unstable during camera
stress tests.
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.
This patch primarily add two changes:
1. Add compatible for SMI sub-common on MT8188 SoC.
2. Add 'resets' and 'reset-names' properties for SMI LARBs to
support SMI reset operations.
Signed-off-by: Friday Yang <friday.yang@mediatek.com>
---
.../mediatek,smi-common.yaml | 2 ++
.../memory-controllers/mediatek,smi-larb.yaml | 20 +++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
index 2f36ac23604c..4392d349878c 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
@@ -39,6 +39,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
@@ -107,6 +108,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 2381660b324c..2e86bb3455f9 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
@@ -69,6 +69,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
@@ -125,6 +131,20 @@ allOf:
required:
- mediatek,larb-id
+ - if: # only for image, camera and ipe subsys
+ properties:
+ compatible:
+ const: mediatek,mt8188-smi-larb
+ mediatek,larb-id:
+ oneOf:
+ - enum:
+ [ 9, 10, 11, 12, 13, 16, 17, 18, 19, 20 ]
+
+ then:
+ required:
+ - resets
+ - reset-names
+
additionalProperties: false
examples:
--
2.46.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188
2025-02-21 7:48 [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC Friday Yang
2025-02-21 7:48 ` [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188 Friday Yang
@ 2025-02-21 7:48 ` Friday Yang
2025-02-24 8:41 ` AngeloGioacchino Del Regno
2025-02-22 9:45 ` [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC Krzysztof Kozlowski
2 siblings, 1 reply; 14+ messages in thread
From: Friday Yang @ 2025-02-21 7:48 UTC (permalink / raw)
To: Yong Wu, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Philipp Zabel
Cc: Friday Yang, linux-mediatek, linux-kernel, devicetree,
linux-arm-kernel, Project_Global_Chrome_Upstream_Group
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>
---
drivers/memory/mtk-smi.c | 141 +++++++++++++++++++++++++++++++++++++--
1 file changed, 137 insertions(+), 4 deletions(-)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 5710348f72f6..51235c2a2081 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
@@ -377,6 +393,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,
@@ -423,6 +452,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 = {
@@ -472,6 +502,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;
@@ -528,6 +598,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;
@@ -538,6 +655,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))
@@ -554,15 +672,24 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- pm_runtime_enable(dev);
+ 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;
+
platform_set_drvdata(pdev, larb);
ret = component_add(dev, &mtk_smi_larb_component_ops);
if (ret)
- goto err_pm_disable;
+ goto err_link_remove;
+
+ pm_runtime_enable(dev);
+
return 0;
-err_pm_disable:
- pm_runtime_disable(dev);
+err_link_remove:
device_link_remove(dev, larb->smi_common_dev);
return ret;
}
@@ -686,6 +813,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,
@@ -729,6 +861,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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC
2025-02-21 7:48 [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC Friday Yang
2025-02-21 7:48 ` [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188 Friday Yang
2025-02-21 7:48 ` [PATCH v4 2/2] memory: mtk-smi: mt8188: " Friday Yang
@ 2025-02-22 9:45 ` Krzysztof Kozlowski
2025-02-24 6:11 ` Friday Yang (杨阳)
2 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-02-22 9:45 UTC (permalink / raw)
To: Friday Yang
Cc: Yong Wu, Rob Herring, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Philipp Zabel, linux-mediatek,
linux-kernel, devicetree, linux-arm-kernel,
Project_Global_Chrome_Upstream_Group
On Fri, Feb 21, 2025 at 03:48:30PM +0800, Friday Yang wrote:
> Based on tag: next-20250220, 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 v4:
> - Use 'devm_reset_control_get_optional_exclusive' instead of
> 'devm_reset_control_get'.
>
> v3:
> https://lore.kernel.org/lkml/20250121064934.13482-2-
That's not a valid link. Fix your email client.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC
2025-02-22 9:45 ` [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC Krzysztof Kozlowski
@ 2025-02-24 6:11 ` Friday Yang (杨阳)
0 siblings, 0 replies; 14+ messages in thread
From: Friday Yang (杨阳) @ 2025-02-24 6:11 UTC (permalink / raw)
To: krzk@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
devicetree@vger.kernel.org, p.zabel@pengutronix.de,
Yong Wu (吴勇), conor+dt@kernel.org, robh@kernel.org,
Project_Global_Chrome_Upstream_Group,
linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
AngeloGioacchino Del Regno
On Sat, 2025-02-22 at 10:45 +0100, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> On Fri, Feb 21, 2025 at 03:48:30PM +0800, Friday Yang wrote:
> > Based on tag: next-20250220, 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 v4:
> > - Use 'devm_reset_control_get_optional_exclusive' instead of
> > 'devm_reset_control_get'.
> >
> > v3:
> > https://lore.kernel.org/lkml/20250121064934.13482-2-
>
> That's not a valid link. Fix your email client.
>
Sorry for the inconvenience. The Web link is quite long, so I have
split it into two lines. Please copy the entire link and paste it into
the browser, it sholuld work.
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188
2025-02-21 7:48 ` [PATCH v4 2/2] memory: mtk-smi: mt8188: " Friday Yang
@ 2025-02-24 8:41 ` AngeloGioacchino Del Regno
2025-03-06 12:46 ` Friday Yang (杨阳)
0 siblings, 1 reply; 14+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-02-24 8:41 UTC (permalink / raw)
To: Friday Yang, Yong Wu, Krzysztof Kozlowski, Rob Herring,
Conor Dooley, Matthias Brugger, Philipp Zabel
Cc: linux-mediatek, linux-kernel, devicetree, linux-arm-kernel,
Project_Global_Chrome_Upstream_Group
Il 21/02/25 08:48, Friday Yang ha scritto:
> 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>
> ---
> drivers/memory/mtk-smi.c | 141 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 137 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index 5710348f72f6..51235c2a2081 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
> @@ -377,6 +393,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,
> @@ -423,6 +452,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 = {
> @@ -472,6 +502,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;
> @@ -528,6 +598,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;
> @@ -538,6 +655,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))
> @@ -554,15 +672,24 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - pm_runtime_enable(dev);
> + 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;
> +
To avoid changing functionality, please call pm_runtime_enable() here, before
component_add().
If you want to remove the goto, you could (please do it!) instead call
devm_pm_runtime_enable() and error check.
Everything else looks good!
Cheers,
Angelo
> platform_set_drvdata(pdev, larb);
> ret = component_add(dev, &mtk_smi_larb_component_ops);
> if (ret)
> - goto err_pm_disable;
> + goto err_link_remove;
> +
> + pm_runtime_enable(dev);
> +
> return 0;
>
> -err_pm_disable:
> - pm_runtime_disable(dev);
> +err_link_remove:
> device_link_remove(dev, larb->smi_common_dev);
> return ret;
> }
> @@ -686,6 +813,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,
> @@ -729,6 +861,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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
2025-02-21 7:48 ` [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188 Friday Yang
@ 2025-02-24 8:41 ` AngeloGioacchino Del Regno
2025-03-06 12:45 ` Friday Yang (杨阳)
0 siblings, 1 reply; 14+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-02-24 8:41 UTC (permalink / raw)
To: Friday Yang, Yong Wu, Krzysztof Kozlowski, Rob Herring,
Conor Dooley, Matthias Brugger, Philipp Zabel
Cc: linux-mediatek, linux-kernel, devicetree, linux-arm-kernel,
Project_Global_Chrome_Upstream_Group
Il 21/02/25 08:48, Friday Yang ha scritto:
> From: "Friday Yang" <friday.yang@mediatek.com>
>
> 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
>
> For previous discussion on the direction of the code modifications,
> please refer to:
> https://lore.kernel.org/all/CAFGrd9qZhObQXvm2_abqaX83xMLqxjQETB2=
> wXpobDWU1CnvkA@mail.gmail.com/
> https://lore.kernel.org/all/CAPDyKFpokXV2gJDgowbixTvOH_5VL3B5H8ey
> hP+KJ5Fasm2rFg@mail.gmail.com/
>
> 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 this
> binding. Without these patches, the SMI becomes unstable during camera
> stress tests.
>
> 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.
>
> This patch primarily add two changes:
> 1. Add compatible for SMI sub-common on MT8188 SoC.
> 2. Add 'resets' and 'reset-names' properties for SMI LARBs to
> support SMI reset operations.
>
> Signed-off-by: Friday Yang <friday.yang@mediatek.com>
> ---
> .../mediatek,smi-common.yaml | 2 ++
> .../memory-controllers/mediatek,smi-larb.yaml | 20 +++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> index 2f36ac23604c..4392d349878c 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> @@ -39,6 +39,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
> @@ -107,6 +108,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 2381660b324c..2e86bb3455f9 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> @@ -69,6 +69,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
> @@ -125,6 +131,20 @@ allOf:
> required:
> - mediatek,larb-id
>
> + - if: # only for image, camera and ipe subsys
> + properties:
> + compatible:
> + const: mediatek,mt8188-smi-larb
> + mediatek,larb-id:
> + oneOf:
Are you really sure that you need 'oneOf' here? :-)
Regards,
Angelo
> + - enum:
> + [ 9, 10, 11, 12, 13, 16, 17, 18, 19, 20 ]
> +
> + then:
> + required:
> + - resets
> + - reset-names
> +
> additionalProperties: false
>
> examples:
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
2025-02-24 8:41 ` AngeloGioacchino Del Regno
@ 2025-03-06 12:45 ` Friday Yang (杨阳)
2025-03-06 12:48 ` Krzysztof Kozlowski
0 siblings, 1 reply; 14+ messages in thread
From: Friday Yang (杨阳) @ 2025-03-06 12:45 UTC (permalink / raw)
To: robh@kernel.org, matthias.bgg@gmail.com,
Yong Wu (吴勇), p.zabel@pengutronix.de,
AngeloGioacchino Del Regno, krzk@kernel.org, conor+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
devicetree@vger.kernel.org, Project_Global_Chrome_Upstream_Group
On Mon, 2025-02-24 at 09:41 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Il 21/02/25 08:48, Friday Yang ha scritto:
> > From: "Friday Yang" <friday.yang@mediatek.com>
> >
> > 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
> >
> > For previous discussion on the direction of the code modifications,
> > please refer to:
> > https://lore.kernel.org/all/CAFGrd9qZhObQXvm2_abqaX83xMLqxjQETB2=
> > wXpobDWU1CnvkA@mail.gmail.com/
> > https://lore.kernel.org/all/CAPDyKFpokXV2gJDgowbixTvOH_5VL3B5H8ey
> > hP+KJ5Fasm2rFg@mail.gmail.com/
> >
> > 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 this
> > binding. Without these patches, the SMI becomes unstable during
> > camera
> > stress tests.
> >
> > 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.
> >
> > This patch primarily add two changes:
> > 1. Add compatible for SMI sub-common on MT8188 SoC.
> > 2. Add 'resets' and 'reset-names' properties for SMI LARBs to
> > support SMI reset operations.
> >
> > Signed-off-by: Friday Yang <friday.yang@mediatek.com>
> > ---
> > .../mediatek,smi-common.yaml | 2 ++
> > .../memory-controllers/mediatek,smi-larb.yaml | 20
> > +++++++++++++++++++
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/memory-
> > controllers/mediatek,smi-common.yaml
> > b/Documentation/devicetree/bindings/memory-
> > controllers/mediatek,smi-common.yaml
> > index 2f36ac23604c..4392d349878c 100644
> > --- a/Documentation/devicetree/bindings/memory-
> > controllers/mediatek,smi-common.yaml
> > +++ b/Documentation/devicetree/bindings/memory-
> > controllers/mediatek,smi-common.yaml
> > @@ -39,6 +39,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
> > @@ -107,6 +108,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 2381660b324c..2e86bb3455f9 100644
> > --- a/Documentation/devicetree/bindings/memory-
> > controllers/mediatek,smi-larb.yaml
> > +++ b/Documentation/devicetree/bindings/memory-
> > controllers/mediatek,smi-larb.yaml
> > @@ -69,6 +69,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
> > @@ -125,6 +131,20 @@ allOf:
> > required:
> > - mediatek,larb-id
> >
> > + - if: # only for image, camera and ipe subsys
> > + properties:
> > + compatible:
> > + const: mediatek,mt8188-smi-larb
> > + mediatek,larb-id:
> > + oneOf:
>
> Are you really sure that you need 'oneOf' here? :-)
>
> Regards,
> Angelo
Yes, I have tested it. If I try to modify the 'examples'
like this. That is:
change the compatible to "mediatek,mt8188-smi-larb",
add 'mediatek,larb-id = <10>;'
examples:
- |+
#include <dt-bindings/clock/mt8173-clk.h>
#includ
e <dt-bindings/power/mt8173-power.h>
larb1: larb@16010000 {
compatible = "mediatek,mt8188-smi-larb";
reg = <0x16010000 0x1000>;
mediatek,smi = <&smi_common>;
mediatek,larb-id = <10>;
power-domains = <&scpsys MT8188_POWER_DOMAIN_VDEC>;
clocks = <&vdecsys CLK_VDEC_CKEN>,
<&vdecsys CLK_VDEC_LARB_CKEN>;
clock-names = "apb", "smi";
};
The 'dt_binding_check' could give the following
errors:
Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
larb.example.dtb: larb@16010000: 'resets' is a required property
from schema $id:
http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml#
Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
larb.example.dtb: larb@16010000: 'reset-names' is a required property
from schema $id:
http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml#
And this is what I want to achieve. On the MediaTek MT8188 SoC
platform, 'resets' and 'reset-names' are only required for SMI LARBs
located in image, camera and ipe subsys. Others can be ignored. And the
'larb-id' of these SMI LARBs are shown in this array: [ 9, 10, 11, 12,
13, 16, 17, 18, 19, 20 ].
Please feel free to let me know if you have any doubts.
>
> > + - enum:
> > + [ 9, 10, 11, 12, 13, 16, 17, 18, 19, 20 ]
> > +
> > + then:
> > + required:
> > + - resets
> > + - reset-names
> > +
> > additionalProperties: false
> >
> > examples:
> > --
> > 2.46.0
> >
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/2] memory: mtk-smi: mt8188: Add SMI reset and clamp for MT8188
2025-02-24 8:41 ` AngeloGioacchino Del Regno
@ 2025-03-06 12:46 ` Friday Yang (杨阳)
0 siblings, 0 replies; 14+ messages in thread
From: Friday Yang (杨阳) @ 2025-03-06 12:46 UTC (permalink / raw)
To: robh@kernel.org, matthias.bgg@gmail.com,
Yong Wu (吴勇), p.zabel@pengutronix.de,
AngeloGioacchino Del Regno, krzk@kernel.org, conor+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
devicetree@vger.kernel.org, Project_Global_Chrome_Upstream_Group
On Mon, 2025-02-24 at 09:41 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Il 21/02/25 08:48, Friday Yang ha scritto:
> > 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>
> > ---
> > drivers/memory/mtk-smi.c | 141
> > +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 137 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index 5710348f72f6..51235c2a2081 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
> > @@ -377,6 +393,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,
> > @@ -423,6 +452,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 = {
> > @@ -472,6 +502,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;
> > @@ -528,6 +598,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,
> > + "medi
> > atek,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;
> > @@ -538,6 +655,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))
> > @@ -554,15 +672,24 @@ static int mtk_smi_larb_probe(struct
> > platform_device *pdev)
> > if (ret < 0)
> > return ret;
> >
> > - pm_runtime_enable(dev);
> > + 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;
> > +
>
> To avoid changing functionality, please call pm_runtime_enable()
> here, before
> component_add().
>
> If you want to remove the goto, you could (please do it!) instead
> call
> devm_pm_runtime_enable() and error check.
>
> Everything else looks good!
>
> Cheers,
> Angelo
Thanks for your comments. I will replace pm_runtime_enable with
devm_pm_runtime_enable and delete the pm_runtime_disable in
mtk_smi_larb_remove and mtk_smi_common_remove. Just like this:
ret = mtk_smi_larb_parse_reset_optional(larb);
if (ret)
goto err_link_remove;
ret = devm_pm_runtime_enable(dev);
if (ret)
goto err_link_remove;
platform_set_drvdata(pdev, larb);
ret = component_add(dev, &mtk_smi_larb_component_ops);
if (ret)
goto err_link_remove;
return 0;
err_link_remove:
device_link_remove(dev, larb->smi_common_dev);
And I will update v5 as soon as possible.
>
> > platform_set_drvdata(pdev, larb);
> > ret = component_add(dev, &mtk_smi_larb_component_ops);
> > if (ret)
> > - goto err_pm_disable;
> > + goto err_link_remove;
> > +
> > + pm_runtime_enable(dev);
> > +
> > return 0;
> >
> > -err_pm_disable:
> > - pm_runtime_disable(dev);
> > +err_link_remove:
> > device_link_remove(dev, larb->smi_common_dev);
> > return ret;
> > }
> > @@ -686,6 +813,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,
> > @@ -729,6 +861,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
> >
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
2025-03-06 12:45 ` Friday Yang (杨阳)
@ 2025-03-06 12:48 ` Krzysztof Kozlowski
2025-03-07 6:38 ` Friday Yang (杨阳)
0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-06 12:48 UTC (permalink / raw)
To: Friday Yang (杨阳), robh@kernel.org,
matthias.bgg@gmail.com, Yong Wu (吴勇),
p.zabel@pengutronix.de, AngeloGioacchino Del Regno,
conor+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
devicetree@vger.kernel.org, Project_Global_Chrome_Upstream_Group
On 06/03/2025 13:45, Friday Yang (杨阳) wrote:
>>> + const: mediatek,mt8188-smi-larb
>>> + mediatek,larb-id:
>>> + oneOf:
>>
>> Are you really sure that you need 'oneOf' here? :-)
>>
>> Regards,
>> Angelo
>
> Yes, I have tested it. If I try to modify the 'examples'
> like this. That is:
> change the compatible to "mediatek,mt8188-smi-larb",
> add 'mediatek,larb-id = <10>;'
>
> examples:
> - |+
> #include <dt-bindings/clock/mt8173-clk.h>
> #includ
> e <dt-bindings/power/mt8173-power.h>
>
> larb1: larb@16010000 {
> compatible = "mediatek,mt8188-smi-larb";
> reg = <0x16010000 0x1000>;
> mediatek,smi = <&smi_common>;
> mediatek,larb-id = <10>;
> power-domains = <&scpsys MT8188_POWER_DOMAIN_VDEC>;
> clocks = <&vdecsys CLK_VDEC_CKEN>,
> <&vdecsys CLK_VDEC_LARB_CKEN>;
> clock-names = "apb", "smi";
> };
>
> The 'dt_binding_check' could give the following
> errors:
>
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
> larb.example.dtb: larb@16010000: 'resets' is a required property
> from schema $id:
> http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml#
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
> larb.example.dtb: larb@16010000: 'reset-names' is a required property
> from schema $id:
> http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml#
>
> And this is what I want to achieve. On the MediaTek MT8188 SoC
> platform, 'resets' and 'reset-names' are only required for SMI LARBs
> located in image, camera and ipe subsys. Others can be ignored. And the
> 'larb-id' of these SMI LARBs are shown in this array: [ 9, 10, 11, 12,
> 13, 16, 17, 18, 19, 20 ].
>
> Please feel free to let me know if you have any doubts.
You did not really answer the question. Where is anything about oneOf in
your reply?
I am dropping this patchset from my queue.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
2025-03-06 12:48 ` Krzysztof Kozlowski
@ 2025-03-07 6:38 ` Friday Yang (杨阳)
2025-03-07 7:04 ` Krzysztof Kozlowski
0 siblings, 1 reply; 14+ messages in thread
From: Friday Yang (杨阳) @ 2025-03-07 6:38 UTC (permalink / raw)
To: robh@kernel.org, matthias.bgg@gmail.com,
Yong Wu (吴勇), p.zabel@pengutronix.de,
AngeloGioacchino Del Regno, krzk@kernel.org, conor+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
devicetree@vger.kernel.org, Project_Global_Chrome_Upstream_Group
On Thu, 2025-03-06 at 13:48 +0100, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> On 06/03/2025 13:45, Friday Yang (杨阳) wrote:
> > > > + const: mediatek,mt8188-smi-larb
> > > > + mediatek,larb-id:
> > > > + oneOf:
> > >
> > > Are you really sure that you need 'oneOf' here? :-)
> > >
> > > Regards,
> > > Angelo
> >
> > Yes, I have tested it. If I try to modify the 'examples'
> > like this. That is:
> > change the compatible to "mediatek,mt8188-smi-larb",
> > add 'mediatek,larb-id = <10>;'
> >
> > examples:
> > - |+
> > #include <dt-bindings/clock/mt8173-clk.h>
> > #includ
> > e <dt-bindings/power/mt8173-power.h>
> >
> > larb1: larb@16010000 {
> > compatible = "mediatek,mt8188-smi-larb";
> > reg = <0x16010000 0x1000>;
> > mediatek,smi = <&smi_common>;
> > mediatek,larb-id = <10>;
> > power-domains = <&scpsys MT8188_POWER_DOMAIN_VDEC>;
> > clocks = <&vdecsys CLK_VDEC_CKEN>,
> > <&vdecsys CLK_VDEC_LARB_CKEN>;
> > clock-names = "apb", "smi";
> > };
> >
> > The 'dt_binding_check' could give the following
> > errors:
> >
> > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
> > larb.example.dtb: larb@16010000: 'resets' is a required property
> > from schema $id:
> >
https://urldefense.com/v3/__http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml*__;Iw!!CTRNKA9wMg0ARbw!kEwWhxyfjVtuHKBHazZGRaFdlmrU2bcIsiVDcsUDzEIManMw2XIG9RgOzq773vtmqlR9_sWZDFhU09SV$
> > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
> > larb.example.dtb: larb@16010000: 'reset-names' is a required
> > property
> > from schema $id:
> >
https://urldefense.com/v3/__http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml*__;Iw!!CTRNKA9wMg0ARbw!kEwWhxyfjVtuHKBHazZGRaFdlmrU2bcIsiVDcsUDzEIManMw2XIG9RgOzq773vtmqlR9_sWZDFhU09SV$
> >
> > And this is what I want to achieve. On the MediaTek MT8188 SoC
> > platform, 'resets' and 'reset-names' are only required for SMI
> > LARBs
> > located in image, camera and ipe subsys. Others can be ignored. And
> > the
> > 'larb-id' of these SMI LARBs are shown in this array: [ 9, 10, 11,
> > 12,
> > 13, 16, 17, 18, 19, 20 ].
> >
> > Please feel free to let me know if you have any doubts.
>
> You did not really answer the question. Where is anything about oneOf
> in
> your reply?
>
> I am dropping this patchset from my queue.
>
In this SoC, we encountered power-off failures and SMI bus hang issues
during camera stress tests. SMI LARBs located in the image, IPE, and
camera subsystems need to implement a reset, while other LARBs do not
require it.
The 'mediatek,larb-id' for SMI LARBs in the image, IPE, and camera
subsystems are as follows:
- image subsystem: 9, 10, 11, 12, 16
- IPE subsystem: 13
- camera subsystem: 17, 18, 19, 20
Therefore, we believe that 'mediatek,larb-id' should be 'oneOf' the
values in the array. Could this answer the question, or is there
another property I should use in this situation?
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
2025-03-07 6:38 ` Friday Yang (杨阳)
@ 2025-03-07 7:04 ` Krzysztof Kozlowski
2025-03-07 8:14 ` Friday Yang (杨阳)
0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-07 7:04 UTC (permalink / raw)
To: Friday Yang (杨阳), robh@kernel.org,
matthias.bgg@gmail.com, Yong Wu (吴勇),
p.zabel@pengutronix.de, AngeloGioacchino Del Regno,
conor+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
devicetree@vger.kernel.org, Project_Global_Chrome_Upstream_Group
On 07/03/2025 07:38, Friday Yang (杨阳) wrote:
> On Thu, 2025-03-06 at 13:48 +0100, Krzysztof Kozlowski wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 06/03/2025 13:45, Friday Yang (杨阳) wrote:
>>>>> + const: mediatek,mt8188-smi-larb
>>>>> + mediatek,larb-id:
>>>>> + oneOf:
>>>>
>>>> Are you really sure that you need 'oneOf' here? :-)
>>>>
>>>> Regards,
>>>> Angelo
>>>
>>> Yes, I have tested it. If I try to modify the 'examples'
>>> like this. That is:
>>> change the compatible to "mediatek,mt8188-smi-larb",
>>> add 'mediatek,larb-id = <10>;'
>>>
>>> examples:
>>> - |+
>>> #include <dt-bindings/clock/mt8173-clk.h>
>>> #includ
>>> e <dt-bindings/power/mt8173-power.h>
>>>
>>> larb1: larb@16010000 {
>>> compatible = "mediatek,mt8188-smi-larb";
>>> reg = <0x16010000 0x1000>;
>>> mediatek,smi = <&smi_common>;
>>> mediatek,larb-id = <10>;
>>> power-domains = <&scpsys MT8188_POWER_DOMAIN_VDEC>;
>>> clocks = <&vdecsys CLK_VDEC_CKEN>,
>>> <&vdecsys CLK_VDEC_LARB_CKEN>;
>>> clock-names = "apb", "smi";
>>> };
>>>
>>> The 'dt_binding_check' could give the following
>>> errors:
>>>
>>> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
>>> larb.example.dtb: larb@16010000: 'resets' is a required property
>>> from schema $id:
>>>
> https://urldefense.com/v3/__http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml*__;Iw!!CTRNKA9wMg0ARbw!kEwWhxyfjVtuHKBHazZGRaFdlmrU2bcIsiVDcsUDzEIManMw2XIG9RgOzq773vtmqlR9_sWZDFhU09SV$
>>> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
>>> larb.example.dtb: larb@16010000: 'reset-names' is a required
>>> property
>>> from schema $id:
>>>
> https://urldefense.com/v3/__http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml*__;Iw!!CTRNKA9wMg0ARbw!kEwWhxyfjVtuHKBHazZGRaFdlmrU2bcIsiVDcsUDzEIManMw2XIG9RgOzq773vtmqlR9_sWZDFhU09SV$
>>>
>>> And this is what I want to achieve. On the MediaTek MT8188 SoC
>>> platform, 'resets' and 'reset-names' are only required for SMI
>>> LARBs
>>> located in image, camera and ipe subsys. Others can be ignored. And
>>> the
>>> 'larb-id' of these SMI LARBs are shown in this array: [ 9, 10, 11,
>>> 12,
>>> 13, 16, 17, 18, 19, 20 ].
>>>
>>> Please feel free to let me know if you have any doubts.
>>
>> You did not really answer the question. Where is anything about oneOf
>> in
>> your reply?
>>
>> I am dropping this patchset from my queue.
>>
>
> In this SoC, we encountered power-off failures and SMI bus hang issues
> during camera stress tests. SMI LARBs located in the image, IPE, and
> camera subsystems need to implement a reset, while other LARBs do not
> require it.
>
> The 'mediatek,larb-id' for SMI LARBs in the image, IPE, and camera
> subsystems are as follows:
> - image subsystem: 9, 10, 11, 12, 16
> - IPE subsystem: 13
> - camera subsystem: 17, 18, 19, 20
>
> Therefore, we believe that 'mediatek,larb-id' should be 'oneOf' the
So explain me where is the second condition?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
2025-03-07 7:04 ` Krzysztof Kozlowski
@ 2025-03-07 8:14 ` Friday Yang (杨阳)
2025-03-07 8:37 ` Krzysztof Kozlowski
0 siblings, 1 reply; 14+ messages in thread
From: Friday Yang (杨阳) @ 2025-03-07 8:14 UTC (permalink / raw)
To: robh@kernel.org, matthias.bgg@gmail.com,
Yong Wu (吴勇), p.zabel@pengutronix.de,
AngeloGioacchino Del Regno, krzk@kernel.org, conor+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
devicetree@vger.kernel.org, Project_Global_Chrome_Upstream_Group
On Fri, 2025-03-07 at 08:04 +0100, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> On 07/03/2025 07:38, Friday Yang (杨阳) wrote:
> > On Thu, 2025-03-06 at 13:48 +0100, Krzysztof Kozlowski wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > >
> > >
> > > On 06/03/2025 13:45, Friday Yang (杨阳) wrote:
> > > > > > + const: mediatek,mt8188-smi-larb
> > > > > > + mediatek,larb-id:
> > > > > > + oneOf:
> > > > >
> > > > > Are you really sure that you need 'oneOf' here? :-)
> > > > >
> > > > > Regards,
> > > > > Angelo
> > > >
> > > > Yes, I have tested it. If I try to modify the 'examples'
> > > > like this. That is:
> > > > change the compatible to "mediatek,mt8188-smi-larb",
> > > > add 'mediatek,larb-id = <10>;'
> > > >
> > > > examples:
> > > > - |+
> > > > #include <dt-bindings/clock/mt8173-clk.h>
> > > > #includ
> > > > e <dt-bindings/power/mt8173-power.h>
> > > >
> > > > larb1: larb@16010000 {
> > > > compatible = "mediatek,mt8188-smi-larb";
> > > > reg = <0x16010000 0x1000>;
> > > > mediatek,smi = <&smi_common>;
> > > > mediatek,larb-id = <10>;
> > > > power-domains = <&scpsys MT8188_POWER_DOMAIN_VDEC>;
> > > > clocks = <&vdecsys CLK_VDEC_CKEN>,
> > > > <&vdecsys CLK_VDEC_LARB_CKEN>;
> > > > clock-names = "apb", "smi";
> > > > };
> > > >
> > > > The 'dt_binding_check' could give the following
> > > > errors:
> > > >
> > > > Documentation/devicetree/bindings/memory-
> > > > controllers/mediatek,smi-
> > > > larb.example.dtb: larb@16010000: 'resets' is a required
> > > > property
> > > > from schema $id:
> > > >
> >
> >
https://urldefense.com/v3/__http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml*__;Iw!!CTRNKA9wMg0ARbw!kEwWhxyfjVtuHKBHazZGRaFdlmrU2bcIsiVDcsUDzEIManMw2XIG9RgOzq773vtmqlR9_sWZDFhU09SV$
> > > > Documentation/devicetree/bindings/memory-
> > > > controllers/mediatek,smi-
> > > > larb.example.dtb: larb@16010000: 'reset-names' is a required
> > > > property
> > > > from schema $id:
> > > >
> >
> >
https://urldefense.com/v3/__http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml*__;Iw!!CTRNKA9wMg0ARbw!kEwWhxyfjVtuHKBHazZGRaFdlmrU2bcIsiVDcsUDzEIManMw2XIG9RgOzq773vtmqlR9_sWZDFhU09SV$
> > > >
> > > > And this is what I want to achieve. On the MediaTek MT8188 SoC
> > > > platform, 'resets' and 'reset-names' are only required for SMI
> > > > LARBs
> > > > located in image, camera and ipe subsys. Others can be ignored.
> > > > And
> > > > the
> > > > 'larb-id' of these SMI LARBs are shown in this array: [ 9, 10,
> > > > 11,
> > > > 12,
> > > > 13, 16, 17, 18, 19, 20 ].
> > > >
> > > > Please feel free to let me know if you have any doubts.
> > >
> > > You did not really answer the question. Where is anything about
> > > oneOf
> > > in
> > > your reply?
> > >
> > > I am dropping this patchset from my queue.
> > >
> >
> > In this SoC, we encountered power-off failures and SMI bus hang
> > issues
> > during camera stress tests. SMI LARBs located in the image, IPE,
> > and
> > camera subsystems need to implement a reset, while other LARBs do
> > not
> > require it.
> >
> > The 'mediatek,larb-id' for SMI LARBs in the image, IPE, and camera
> > subsystems are as follows:
> > - image subsystem: 9, 10, 11, 12, 16
> > - IPE subsystem: 13
> > - camera subsystem: 17, 18, 19, 20
> >
> > Therefore, we believe that 'mediatek,larb-id' should be 'oneOf' the
>
>
> So explain me where is the second condition?
>
Sorry for the misunderstanding. I should fix it like this:
just delete 'oneOf' and use 'enum'
- if: # only for camera and image subsys
properties:
com
patible:
const: mediatek,mt8188-smi-larb
mediatek,larb-
id:
enum:
[ 9, 10, 11, 12, 13, 16, 17, 18, 19, 20 ]
then:
required:
- resets
- reset-names
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188
2025-03-07 8:14 ` Friday Yang (杨阳)
@ 2025-03-07 8:37 ` Krzysztof Kozlowski
0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-07 8:37 UTC (permalink / raw)
To: Friday Yang (杨阳), robh@kernel.org,
matthias.bgg@gmail.com, Yong Wu (吴勇),
p.zabel@pengutronix.de, AngeloGioacchino Del Regno,
conor+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
devicetree@vger.kernel.org, Project_Global_Chrome_Upstream_Group
On 07/03/2025 09:14, Friday Yang (杨阳) wrote:
>>> The 'mediatek,larb-id' for SMI LARBs in the image, IPE, and camera
>>> subsystems are as follows:
>>> - image subsystem: 9, 10, 11, 12, 16
>>> - IPE subsystem: 13
>>> - camera subsystem: 17, 18, 19, 20
>>>
>>> Therefore, we believe that 'mediatek,larb-id' should be 'oneOf' the
>>
>>
>> So explain me where is the second condition?
>>
>
> Sorry for the misunderstanding. I should fix it like this:
> just delete 'oneOf' and use 'enum'
>
> - if: # only for camera and image subsys
> properties:
> com
> patible:
> const: mediatek,mt8188-smi-larb
> mediatek,larb-
> id:
> enum:
> [ 9, 10, 11, 12, 13, 16, 17, 18, 19, 20 ]
Yes
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-03-07 8:37 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-21 7:48 [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC Friday Yang
2025-02-21 7:48 ` [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188 Friday Yang
2025-02-24 8:41 ` AngeloGioacchino Del Regno
2025-03-06 12:45 ` Friday Yang (杨阳)
2025-03-06 12:48 ` Krzysztof Kozlowski
2025-03-07 6:38 ` Friday Yang (杨阳)
2025-03-07 7:04 ` Krzysztof Kozlowski
2025-03-07 8:14 ` Friday Yang (杨阳)
2025-03-07 8:37 ` Krzysztof Kozlowski
2025-02-21 7:48 ` [PATCH v4 2/2] memory: mtk-smi: mt8188: " Friday Yang
2025-02-24 8:41 ` AngeloGioacchino Del Regno
2025-03-06 12:46 ` Friday Yang (杨阳)
2025-02-22 9:45 ` [PATCH v4 0/2] Add SMI reset and clamp for MediaTek MT8188 SoC Krzysztof Kozlowski
2025-02-24 6:11 ` Friday Yang (杨阳)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox