2021-09-14 11:38:58

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 00/13] MT8195 SMI support

This patchset mainly adds SMI support for mt8195.

Comparing with the previous version, add two new functions:
a) add smi sub common
b) add initial setting for smi-common and smi-larb.

Change note:
v4:1) base on v5.15-rc1
2) In the dt-binding:
a. add "else mediatek,smi: false." in the yaml.
b. Remove mediatek,smi_sub_common. since we have only 2 level currently,
It should be smi-sub-common if that node has "mediatek,smi". otherwise,
it is smi-common.

v3: https://lore.kernel.org/linux-mediatek/[email protected]/
1)in the dt-binding:
a. change mediatek,smi type from phandle-array to phandle from Rob.
b. Add a new bool property (mediatek,smi_sub_common)
to indicate if this is smi-sub-common.
2)change the clock using bulk parting.
keep the smi-common's flag. more strict.
3) more comment about larb initial setting.

v2: https://lore.kernel.org/linux-mediatek/[email protected]/
rebase on v5.14-rc1
1) Adjust clk_bulk flow: use devm_clk_bulk_get for necessary clocks.
2) Add two new little patches:
a) use devm_platform_ioremap_resource
b) Add error handle for smi_probe

v1: https://lore.kernel.org/linux-mediatek/[email protected]/

Yong Wu (13):
dt-bindings: memory: mediatek: Add mt8195 smi binding
dt-bindings: memory: mediatek: Add mt8195 smi sub common
memory: mtk-smi: Use clk_bulk clock ops
memory: mtk-smi: Rename smi_gen to smi_type
memory: mtk-smi: Adjust some code position
memory: mtk-smi: Add error handle for smi_probe
memory: mtk-smi: Add device link for smi-sub-common
memory: mtk-smi: Add clocks for smi-sub-common
memory: mtk-smi: Use devm_platform_ioremap_resource
memory: mtk-smi: mt8195: Add smi support
memory: mtk-smi: mt8195: Add initial setting for smi-common
memory: mtk-smi: mt8195: Add initial setting for smi-larb
MAINTAINERS: Add entry for MediaTek SMI

.../mediatek,smi-common.yaml | 34 +-
.../memory-controllers/mediatek,smi-larb.yaml | 3 +
MAINTAINERS | 8 +
drivers/memory/mtk-smi.c | 596 ++++++++++--------
4 files changed, 393 insertions(+), 248 deletions(-)

--
2.18.0



2021-09-14 11:39:21

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 01/13] dt-bindings: memory: mediatek: Add mt8195 smi binding

Add mt8195 smi supporting in the bindings.

In mt8195, there are two smi-common HW, one is for vdo(video output),
the other is for vpp(video processing pipe). They connect with different
smi-larbs, then some setting(bus_sel) is different. Differentiate them
with the compatible string.

Something like this:

IOMMU(VDO) IOMMU(VPP)
| |
SMI_COMMON_VDO SMI_COMMON_VPP
---------------- ----------------
| | ... | | ...
larb0 larb2 ... larb1 larb3 ...

Signed-off-by: Yong Wu <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
.../bindings/memory-controllers/mediatek,smi-common.yaml | 6 +++++-
.../bindings/memory-controllers/mediatek,smi-larb.yaml | 3 +++
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
index e87e4382807c..602592b6c3f5 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
@@ -16,7 +16,7 @@ description: |
MediaTek SMI have two generations of HW architecture, here is the list
which generation the SoCs use:
generation 1: mt2701 and mt7623.
- generation 2: mt2712, mt6779, mt8167, mt8173, mt8183 and mt8192.
+ generation 2: mt2712, mt6779, mt8167, mt8173, mt8183, mt8192 and mt8195.

There's slight differences between the two SMI, for generation 2, the
register which control the iommu port is at each larb's register base. But
@@ -36,6 +36,8 @@ properties:
- mediatek,mt8173-smi-common
- mediatek,mt8183-smi-common
- mediatek,mt8192-smi-common
+ - mediatek,mt8195-smi-common-vdo
+ - mediatek,mt8195-smi-common-vpp

- description: for mt7623
items:
@@ -98,6 +100,8 @@ allOf:
- mediatek,mt6779-smi-common
- mediatek,mt8183-smi-common
- mediatek,mt8192-smi-common
+ - mediatek,mt8195-smi-common-vdo
+ - mediatek,mt8195-smi-common-vpp

then:
properties:
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
index 2353f6cf3c80..eaeff1ada7f8 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
@@ -24,6 +24,7 @@ properties:
- mediatek,mt8173-smi-larb
- mediatek,mt8183-smi-larb
- mediatek,mt8192-smi-larb
+ - mediatek,mt8195-smi-larb

- description: for mt7623
items:
@@ -74,6 +75,7 @@ allOf:
compatible:
enum:
- mediatek,mt8183-smi-larb
+ - mediatek,mt8195-smi-larb

then:
properties:
@@ -108,6 +110,7 @@ allOf:
- mediatek,mt6779-smi-larb
- mediatek,mt8167-smi-larb
- mediatek,mt8192-smi-larb
+ - mediatek,mt8195-smi-larb

then:
required:
--
2.18.0

2021-09-14 11:40:02

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 02/13] dt-bindings: memory: mediatek: Add mt8195 smi sub common

Add the binding for smi-sub-common. The SMI block diagram like this:

IOMMU
| |
smi-common
------------------
| .... |
larb0 larb7 <-max is 8

The smi-common connects with smi-larb and IOMMU. The maximum larbs number
that connects with a smi-common is 8. If the engines number is over 8,
sometimes we use a smi-sub-common which is nearly same with smi-common.
It supports up to 8 input and 1 output(smi-common has 2 output)

Something like:

IOMMU
| |
smi-common
---------------------
| | ...
larb0 sub-common ... <-max is 8
-----------
| | ... <-max is 8 too.
larb2 larb5

We don't need extra SW setting for smi-sub-common, only the sub-common has
special clocks need to enable when the engines access dram.

If it is sub-common, it should have a "mediatek,smi" phandle to point to
its smi-common. meanwhile the sub-common only has one gals clock.

Signed-off-by: Yong Wu <[email protected]>
---
change note: add "else mediatek,smi: false".
---
.../mediatek,smi-common.yaml | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
index 602592b6c3f5..3a82b0b27fa0 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
@@ -38,6 +38,7 @@ properties:
- mediatek,mt8192-smi-common
- mediatek,mt8195-smi-common-vdo
- mediatek,mt8195-smi-common-vpp
+ - mediatek,mt8195-smi-sub-common

- description: for mt7623
items:
@@ -67,6 +68,10 @@ properties:
minItems: 2
maxItems: 4

+ mediatek,smi:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: a phandle to the smi-common node above. Only for sub-common.
+
required:
- compatible
- reg
@@ -93,6 +98,29 @@ allOf:
- const: smi
- const: async

+ - if: # only for sub common
+ properties:
+ compatible:
+ contains:
+ enum:
+ - mediatek,mt8195-smi-sub-common
+ then:
+ required:
+ - mediatek,smi
+ properties:
+ clock:
+ items:
+ minItems: 3
+ maxItems: 3
+ clock-names:
+ items:
+ - const: apb
+ - const: smi
+ - const: gals0
+ else:
+ properties:
+ mediatek,smi: false
+
- if: # for gen2 HW that have gals
properties:
compatible:
--
2.18.0

2021-09-14 11:40:13

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 04/13] memory: mtk-smi: Rename smi_gen to smi_type

Prepare for adding smi sub common. Only rename from smi_gen to smi_type.
No functional change.

About the current "smi_gen", we have gen1/gen2 that stand for the
generation number for HW. I plan to add a new type(sub_common), then the
name "gen" is not proper.

Signed-off-by: Yong Wu <[email protected]>
Reviewed-by: Ikjoon Jang <[email protected]>
---
drivers/memory/mtk-smi.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index f91eaf5c3ab0..02a584dfb9b1 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -55,7 +55,7 @@
/* All are MMU0 defaultly. Only specialize mmu1 here. */
#define F_MMU1_LARB(larbid) (0x1 << SMI_BUS_LARB_SHIFT(larbid))

-enum mtk_smi_gen {
+enum mtk_smi_type {
MTK_SMI_GEN1,
MTK_SMI_GEN2
};
@@ -75,9 +75,9 @@ static const char * const mtk_smi_common_clks[] = {"apb", "smi", "gals0", "gals1
#define MTK_SMI_COM_GALS_REQ_CLK_NR MTK_SMI_CLK_NR_MAX

struct mtk_smi_common_plat {
- enum mtk_smi_gen gen;
- bool has_gals;
- u32 bus_sel; /* Balance some larbs to enter mmu0 or mmu1 */
+ enum mtk_smi_type type;
+ bool has_gals;
+ u32 bus_sel; /* Balance some larbs to enter mmu0 or mmu1 */
};

struct mtk_smi_larb_gen {
@@ -409,29 +409,29 @@ static struct platform_driver mtk_smi_larb_driver = {
};

static const struct mtk_smi_common_plat mtk_smi_common_gen1 = {
- .gen = MTK_SMI_GEN1,
+ .type = MTK_SMI_GEN1,
};

static const struct mtk_smi_common_plat mtk_smi_common_gen2 = {
- .gen = MTK_SMI_GEN2,
+ .type = MTK_SMI_GEN2,
};

static const struct mtk_smi_common_plat mtk_smi_common_mt6779 = {
- .gen = MTK_SMI_GEN2,
- .has_gals = true,
- .bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(4) |
- F_MMU1_LARB(5) | F_MMU1_LARB(6) | F_MMU1_LARB(7),
+ .type = MTK_SMI_GEN2,
+ .has_gals = true,
+ .bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(4) |
+ F_MMU1_LARB(5) | F_MMU1_LARB(6) | F_MMU1_LARB(7),
};

static const struct mtk_smi_common_plat mtk_smi_common_mt8183 = {
- .gen = MTK_SMI_GEN2,
+ .type = MTK_SMI_GEN2,
.has_gals = true,
.bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(5) |
F_MMU1_LARB(7),
};

static const struct mtk_smi_common_plat mtk_smi_common_mt8192 = {
- .gen = MTK_SMI_GEN2,
+ .type = MTK_SMI_GEN2,
.has_gals = true,
.bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(5) |
F_MMU1_LARB(6),
@@ -494,7 +494,7 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
* clock into emi clock domain, but for mtk smi gen2, there's no smi ao
* base.
*/
- if (common->plat->gen == MTK_SMI_GEN1) {
+ if (common->plat->type == MTK_SMI_GEN1) {
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
common->smi_ao_base = devm_ioremap_resource(dev, res);
if (IS_ERR(common->smi_ao_base))
@@ -534,7 +534,7 @@ static int __maybe_unused mtk_smi_common_resume(struct device *dev)
if (ret)
return ret;

- if (common->plat->gen == MTK_SMI_GEN2 && bus_sel)
+ if (common->plat->type == MTK_SMI_GEN2 && bus_sel)
writel(bus_sel, common->base + SMI_BUS_SEL);
return 0;
}
--
2.18.0

2021-09-14 11:40:14

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 03/13] memory: mtk-smi: Use clk_bulk clock ops

Use clk_bulk interface instead of the orginal one to simplify the code.

For SMI larbs: Require apb/smi clocks while gals is optional.
For SMI common: Require apb/smi/gals0/gal1 in has_gals case. Otherwise,
also only require apb/smi, No optional clk here.

About the "has_gals" flag, for smi larbs, the gals clock also may be
optional even this platform support it. thus it always use
*_bulk_get_optional, then the flag has_gals is unnecessary. Remove it.
The smi_common's has_gals still keep it.

Also remove clk fail logs since bulk interface already output fail log.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/memory/mtk-smi.c | 143 +++++++++++++++------------------------
1 file changed, 55 insertions(+), 88 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index c5fb51f73b34..f91eaf5c3ab0 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -60,6 +60,20 @@ enum mtk_smi_gen {
MTK_SMI_GEN2
};

+#define MTK_SMI_CLK_NR_MAX 4
+
+/* larbs: Require apb/smi clocks while gals is optional. */
+static const char * const mtk_smi_larb_clks[] = {"apb", "smi", "gals"};
+#define MTK_SMI_LARB_REQ_CLK_NR 2
+#define MTK_SMI_LARB_OPT_CLK_NR 1
+
+/*
+ * common: Require these four clocks in has_gals case. Otherwise, only apb/smi are required.
+ */
+static const char * const mtk_smi_common_clks[] = {"apb", "smi", "gals0", "gals1"};
+#define MTK_SMI_COM_REQ_CLK_NR 2
+#define MTK_SMI_COM_GALS_REQ_CLK_NR MTK_SMI_CLK_NR_MAX
+
struct mtk_smi_common_plat {
enum mtk_smi_gen gen;
bool has_gals;
@@ -70,13 +84,12 @@ struct mtk_smi_larb_gen {
int port_in_larb[MTK_LARB_NR_MAX + 1];
void (*config_port)(struct device *dev);
unsigned int larb_direct_to_common_mask;
- bool has_gals;
};

struct mtk_smi {
struct device *dev;
- struct clk *clk_apb, *clk_smi;
- struct clk *clk_gals0, *clk_gals1;
+ unsigned int clk_num;
+ struct clk_bulk_data clks[MTK_SMI_CLK_NR_MAX];
struct clk *clk_async; /*only needed by mt2701*/
union {
void __iomem *smi_ao_base; /* only for gen1 */
@@ -95,45 +108,6 @@ struct mtk_smi_larb { /* larb: local arbiter */
unsigned char *bank;
};

-static int mtk_smi_clk_enable(const struct mtk_smi *smi)
-{
- int ret;
-
- ret = clk_prepare_enable(smi->clk_apb);
- if (ret)
- return ret;
-
- ret = clk_prepare_enable(smi->clk_smi);
- if (ret)
- goto err_disable_apb;
-
- ret = clk_prepare_enable(smi->clk_gals0);
- if (ret)
- goto err_disable_smi;
-
- ret = clk_prepare_enable(smi->clk_gals1);
- if (ret)
- goto err_disable_gals0;
-
- return 0;
-
-err_disable_gals0:
- clk_disable_unprepare(smi->clk_gals0);
-err_disable_smi:
- clk_disable_unprepare(smi->clk_smi);
-err_disable_apb:
- clk_disable_unprepare(smi->clk_apb);
- return ret;
-}
-
-static void mtk_smi_clk_disable(const struct mtk_smi *smi)
-{
- clk_disable_unprepare(smi->clk_gals1);
- clk_disable_unprepare(smi->clk_gals0);
- clk_disable_unprepare(smi->clk_smi);
- clk_disable_unprepare(smi->clk_apb);
-}
-
int mtk_smi_larb_get(struct device *larbdev)
{
int ret = pm_runtime_resume_and_get(larbdev);
@@ -270,7 +244,6 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt6779 = {
};

static const struct mtk_smi_larb_gen mtk_smi_larb_mt8183 = {
- .has_gals = true,
.config_port = mtk_smi_larb_config_port_gen2_general,
.larb_direct_to_common_mask = BIT(2) | BIT(3) | BIT(7),
/* IPU0 | IPU1 | CCU */
@@ -312,6 +285,27 @@ static const struct of_device_id mtk_smi_larb_of_ids[] = {
{}
};

+static int mtk_smi_dts_clk_init(struct device *dev, struct mtk_smi *smi,
+ const char * const clks[],
+ unsigned int clk_nr_required,
+ unsigned int clk_nr_optional)
+{
+ int i, ret;
+
+ for (i = 0; i < clk_nr_required; i++)
+ smi->clks[i].id = clks[i];
+ ret = devm_clk_bulk_get(dev, clk_nr_required, smi->clks);
+ if (ret)
+ return ret;
+
+ for (i = clk_nr_required; i < clk_nr_required + clk_nr_optional; i++)
+ smi->clks[i].id = clks[i];
+ ret = devm_clk_bulk_get_optional(dev, clk_nr_optional,
+ smi->clks + clk_nr_required);
+ smi->clk_num = clk_nr_required + clk_nr_optional;
+ return ret;
+}
+
static int mtk_smi_larb_probe(struct platform_device *pdev)
{
struct mtk_smi_larb *larb;
@@ -320,6 +314,7 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
struct device_node *smi_node;
struct platform_device *smi_pdev;
struct device_link *link;
+ int ret;

larb = devm_kzalloc(dev, sizeof(*larb), GFP_KERNEL);
if (!larb)
@@ -331,24 +326,12 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
if (IS_ERR(larb->base))
return PTR_ERR(larb->base);

- larb->smi.clk_apb = devm_clk_get(dev, "apb");
- if (IS_ERR(larb->smi.clk_apb))
- return PTR_ERR(larb->smi.clk_apb);
-
- larb->smi.clk_smi = devm_clk_get(dev, "smi");
- if (IS_ERR(larb->smi.clk_smi))
- return PTR_ERR(larb->smi.clk_smi);
-
- if (larb->larb_gen->has_gals) {
- /* The larbs may still haven't gals even if the SoC support.*/
- larb->smi.clk_gals0 = devm_clk_get(dev, "gals");
- if (PTR_ERR(larb->smi.clk_gals0) == -ENOENT)
- larb->smi.clk_gals0 = NULL;
- else if (IS_ERR(larb->smi.clk_gals0))
- return PTR_ERR(larb->smi.clk_gals0);
- }
- larb->smi.dev = dev;
+ ret = mtk_smi_dts_clk_init(dev, &larb->smi, mtk_smi_larb_clks,
+ MTK_SMI_LARB_REQ_CLK_NR, MTK_SMI_LARB_OPT_CLK_NR);
+ if (ret)
+ return ret;

+ larb->smi.dev = dev;
smi_node = of_parse_phandle(dev->of_node, "mediatek,smi", 0);
if (!smi_node)
return -EINVAL;
@@ -391,11 +374,9 @@ static int __maybe_unused mtk_smi_larb_resume(struct device *dev)
const struct mtk_smi_larb_gen *larb_gen = larb->larb_gen;
int ret;

- ret = mtk_smi_clk_enable(&larb->smi);
- if (ret < 0) {
- dev_err(dev, "Failed to enable clock(%d).\n", ret);
+ ret = clk_bulk_prepare_enable(larb->smi.clk_num, larb->smi.clks);
+ if (ret < 0)
return ret;
- }

/* Configure the basic setting for this larb */
larb_gen->config_port(dev);
@@ -407,7 +388,7 @@ static int __maybe_unused mtk_smi_larb_suspend(struct device *dev)
{
struct mtk_smi_larb *larb = dev_get_drvdata(dev);

- mtk_smi_clk_disable(&larb->smi);
+ clk_bulk_disable_unprepare(larb->smi.clk_num, larb->smi.clks);
return 0;
}

@@ -493,7 +474,7 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct mtk_smi *common;
struct resource *res;
- int ret;
+ int ret, clk_required = MTK_SMI_COM_REQ_CLK_NR;

common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL);
if (!common)
@@ -501,23 +482,11 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
common->dev = dev;
common->plat = of_device_get_match_data(dev);

- common->clk_apb = devm_clk_get(dev, "apb");
- if (IS_ERR(common->clk_apb))
- return PTR_ERR(common->clk_apb);
-
- common->clk_smi = devm_clk_get(dev, "smi");
- if (IS_ERR(common->clk_smi))
- return PTR_ERR(common->clk_smi);
-
- if (common->plat->has_gals) {
- common->clk_gals0 = devm_clk_get(dev, "gals0");
- if (IS_ERR(common->clk_gals0))
- return PTR_ERR(common->clk_gals0);
-
- common->clk_gals1 = devm_clk_get(dev, "gals1");
- if (IS_ERR(common->clk_gals1))
- return PTR_ERR(common->clk_gals1);
- }
+ if (common->plat->has_gals)
+ clk_required = MTK_SMI_COM_GALS_REQ_CLK_NR;
+ ret = mtk_smi_dts_clk_init(dev, common, mtk_smi_common_clks, clk_required, 0);
+ if (ret)
+ return ret;

/*
* for mtk smi gen 1, we need to get the ao(always on) base to config
@@ -561,11 +530,9 @@ static int __maybe_unused mtk_smi_common_resume(struct device *dev)
u32 bus_sel = common->plat->bus_sel;
int ret;

- ret = mtk_smi_clk_enable(common);
- if (ret) {
- dev_err(common->dev, "Failed to enable clock(%d).\n", ret);
+ ret = clk_bulk_prepare_enable(common->clk_num, common->clks);
+ if (ret)
return ret;
- }

if (common->plat->gen == MTK_SMI_GEN2 && bus_sel)
writel(bus_sel, common->base + SMI_BUS_SEL);
@@ -576,7 +543,7 @@ static int __maybe_unused mtk_smi_common_suspend(struct device *dev)
{
struct mtk_smi *common = dev_get_drvdata(dev);

- mtk_smi_clk_disable(common);
+ clk_bulk_disable_unprepare(common->clk_num, common->clks);
return 0;
}

--
2.18.0

2021-09-14 11:40:21

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 05/13] memory: mtk-smi: Adjust some code position

No functional change. Only move the code position to make the code more
readable.
1. Put the register smi-common above smi-larb. Prepare to add some others
register setting.
2. Put mtk_smi_larb_unbind around larb_bind.
3. Sort the SoC data alphabetically. and put them in one line as the
current kernel allow it.

Signed-off-by: Yong Wu <[email protected]>
Reviewed-by: Ikjoon Jang <[email protected]>
---
drivers/memory/mtk-smi.c | 188 ++++++++++++++++-----------------------
1 file changed, 75 insertions(+), 113 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 02a584dfb9b1..33b6c5efe102 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -17,13 +17,16 @@
#include <dt-bindings/memory/mt2701-larb-port.h>
#include <dt-bindings/memory/mtk-memory-port.h>

-/* mt8173 */
-#define SMI_LARB_MMU_EN 0xf00
+/* SMI COMMON */
+#define SMI_BUS_SEL 0x220
+#define SMI_BUS_LARB_SHIFT(larbid) ((larbid) << 1)
+/* All are MMU0 defaultly. Only specialize mmu1 here. */
+#define F_MMU1_LARB(larbid) (0x1 << SMI_BUS_LARB_SHIFT(larbid))

-/* mt8167 */
-#define MT8167_SMI_LARB_MMU_EN 0xfc0
+/* SMI LARB */

-/* mt2701 */
+/* Below are about mmu enable registers, they are different in SoCs */
+/* gen1: mt2701 */
#define REG_SMI_SECUR_CON_BASE 0x5c0

/* every register control 8 port, register offset 0x4 */
@@ -41,20 +44,21 @@
/* mt2701 domain should be set to 3 */
#define SMI_SECUR_CON_VAL_DOMAIN(id) (0x3 << ((((id) & 0x7) << 2) + 1))

-/* mt2712 */
-#define SMI_LARB_NONSEC_CON(id) (0x380 + ((id) * 4))
-#define F_MMU_EN BIT(0)
-#define BANK_SEL(id) ({ \
+/* gen2: */
+/* mt8167 */
+#define MT8167_SMI_LARB_MMU_EN 0xfc0
+
+/* mt8173 */
+#define MT8173_SMI_LARB_MMU_EN 0xf00
+
+/* general */
+#define SMI_LARB_NONSEC_CON(id) (0x380 + ((id) * 4))
+#define F_MMU_EN BIT(0)
+#define BANK_SEL(id) ({ \
u32 _id = (id) & 0x3; \
(_id << 8 | _id << 10 | _id << 12 | _id << 14); \
})

-/* SMI COMMON */
-#define SMI_BUS_SEL 0x220
-#define SMI_BUS_LARB_SHIFT(larbid) ((larbid) << 1)
-/* All are MMU0 defaultly. Only specialize mmu1 here. */
-#define F_MMU1_LARB(larbid) (0x1 << SMI_BUS_LARB_SHIFT(larbid))
-
enum mtk_smi_type {
MTK_SMI_GEN1,
MTK_SMI_GEN2
@@ -140,36 +144,16 @@ mtk_smi_larb_bind(struct device *dev, struct device *master, void *data)
return -ENODEV;
}

-static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
-{
- struct mtk_smi_larb *larb = dev_get_drvdata(dev);
- u32 reg;
- int i;
-
- if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
- return;
-
- for_each_set_bit(i, (unsigned long *)larb->mmu, 32) {
- reg = readl_relaxed(larb->base + SMI_LARB_NONSEC_CON(i));
- reg |= F_MMU_EN;
- reg |= BANK_SEL(larb->bank[i]);
- writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
- }
-}
-
-static void mtk_smi_larb_config_port_mt8173(struct device *dev)
+static void
+mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
{
- struct mtk_smi_larb *larb = dev_get_drvdata(dev);
-
- writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
+ /* Do nothing as the iommu is always enabled. */
}

-static void mtk_smi_larb_config_port_mt8167(struct device *dev)
-{
- struct mtk_smi_larb *larb = dev_get_drvdata(dev);
-
- writel(*larb->mmu, larb->base + MT8167_SMI_LARB_MMU_EN);
-}
+static const struct component_ops mtk_smi_larb_component_ops = {
+ .bind = mtk_smi_larb_bind,
+ .unbind = mtk_smi_larb_unbind,
+};

static void mtk_smi_larb_config_port_gen1(struct device *dev)
{
@@ -202,26 +186,36 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
}
}

-static void
-mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
+static void mtk_smi_larb_config_port_mt8167(struct device *dev)
{
- /* Do nothing as the iommu is always enabled. */
+ struct mtk_smi_larb *larb = dev_get_drvdata(dev);
+
+ writel(*larb->mmu, larb->base + MT8167_SMI_LARB_MMU_EN);
}

-static const struct component_ops mtk_smi_larb_component_ops = {
- .bind = mtk_smi_larb_bind,
- .unbind = mtk_smi_larb_unbind,
-};
+static void mtk_smi_larb_config_port_mt8173(struct device *dev)
+{
+ struct mtk_smi_larb *larb = dev_get_drvdata(dev);

-static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
- /* mt8173 do not need the port in larb */
- .config_port = mtk_smi_larb_config_port_mt8173,
-};
+ writel(*larb->mmu, larb->base + MT8173_SMI_LARB_MMU_EN);
+}

-static const struct mtk_smi_larb_gen mtk_smi_larb_mt8167 = {
- /* mt8167 do not need the port in larb */
- .config_port = mtk_smi_larb_config_port_mt8167,
-};
+static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
+{
+ struct mtk_smi_larb *larb = dev_get_drvdata(dev);
+ u32 reg;
+ int i;
+
+ if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
+ return;
+
+ for_each_set_bit(i, (unsigned long *)larb->mmu, 32) {
+ reg = readl_relaxed(larb->base + SMI_LARB_NONSEC_CON(i));
+ reg |= F_MMU_EN;
+ reg |= BANK_SEL(larb->bank[i]);
+ writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
+ }
+}

static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
.port_in_larb = {
@@ -243,6 +237,16 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt6779 = {
/* DUMMY | IPU0 | IPU1 | CCU | MDLA */
};

+static const struct mtk_smi_larb_gen mtk_smi_larb_mt8167 = {
+ /* mt8167 do not need the port in larb */
+ .config_port = mtk_smi_larb_config_port_mt8167,
+};
+
+static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
+ /* mt8173 do not need the port in larb */
+ .config_port = mtk_smi_larb_config_port_mt8173,
+};
+
static const struct mtk_smi_larb_gen mtk_smi_larb_mt8183 = {
.config_port = mtk_smi_larb_config_port_gen2_general,
.larb_direct_to_common_mask = BIT(2) | BIT(3) | BIT(7),
@@ -254,34 +258,13 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt8192 = {
};

static const struct of_device_id mtk_smi_larb_of_ids[] = {
- {
- .compatible = "mediatek,mt8167-smi-larb",
- .data = &mtk_smi_larb_mt8167
- },
- {
- .compatible = "mediatek,mt8173-smi-larb",
- .data = &mtk_smi_larb_mt8173
- },
- {
- .compatible = "mediatek,mt2701-smi-larb",
- .data = &mtk_smi_larb_mt2701
- },
- {
- .compatible = "mediatek,mt2712-smi-larb",
- .data = &mtk_smi_larb_mt2712
- },
- {
- .compatible = "mediatek,mt6779-smi-larb",
- .data = &mtk_smi_larb_mt6779
- },
- {
- .compatible = "mediatek,mt8183-smi-larb",
- .data = &mtk_smi_larb_mt8183
- },
- {
- .compatible = "mediatek,mt8192-smi-larb",
- .data = &mtk_smi_larb_mt8192
- },
+ {.compatible = "mediatek,mt2701-smi-larb", .data = &mtk_smi_larb_mt2701},
+ {.compatible = "mediatek,mt2712-smi-larb", .data = &mtk_smi_larb_mt2712},
+ {.compatible = "mediatek,mt6779-smi-larb", .data = &mtk_smi_larb_mt6779},
+ {.compatible = "mediatek,mt8167-smi-larb", .data = &mtk_smi_larb_mt8167},
+ {.compatible = "mediatek,mt8173-smi-larb", .data = &mtk_smi_larb_mt8173},
+ {.compatible = "mediatek,mt8183-smi-larb", .data = &mtk_smi_larb_mt8183},
+ {.compatible = "mediatek,mt8192-smi-larb", .data = &mtk_smi_larb_mt8192},
{}
};

@@ -438,34 +421,13 @@ static const struct mtk_smi_common_plat mtk_smi_common_mt8192 = {
};

static const struct of_device_id mtk_smi_common_of_ids[] = {
- {
- .compatible = "mediatek,mt8173-smi-common",
- .data = &mtk_smi_common_gen2,
- },
- {
- .compatible = "mediatek,mt8167-smi-common",
- .data = &mtk_smi_common_gen2,
- },
- {
- .compatible = "mediatek,mt2701-smi-common",
- .data = &mtk_smi_common_gen1,
- },
- {
- .compatible = "mediatek,mt2712-smi-common",
- .data = &mtk_smi_common_gen2,
- },
- {
- .compatible = "mediatek,mt6779-smi-common",
- .data = &mtk_smi_common_mt6779,
- },
- {
- .compatible = "mediatek,mt8183-smi-common",
- .data = &mtk_smi_common_mt8183,
- },
- {
- .compatible = "mediatek,mt8192-smi-common",
- .data = &mtk_smi_common_mt8192,
- },
+ {.compatible = "mediatek,mt2701-smi-common", .data = &mtk_smi_common_gen1},
+ {.compatible = "mediatek,mt2712-smi-common", .data = &mtk_smi_common_gen2},
+ {.compatible = "mediatek,mt6779-smi-common", .data = &mtk_smi_common_mt6779},
+ {.compatible = "mediatek,mt8167-smi-common", .data = &mtk_smi_common_gen2},
+ {.compatible = "mediatek,mt8173-smi-common", .data = &mtk_smi_common_gen2},
+ {.compatible = "mediatek,mt8183-smi-common", .data = &mtk_smi_common_mt8183},
+ {.compatible = "mediatek,mt8192-smi-common", .data = &mtk_smi_common_mt8192},
{}
};

--
2.18.0

2021-09-14 11:40:41

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 06/13] memory: mtk-smi: Add error handle for smi_probe

Add error handle while component_add fail.

Signed-off-by: Yong Wu <[email protected]>
Reviewed-by: Ikjoon Jang <[email protected]>
---
drivers/memory/mtk-smi.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 33b6c5efe102..b362d528944e 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -338,7 +338,15 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)

pm_runtime_enable(dev);
platform_set_drvdata(pdev, larb);
- return component_add(dev, &mtk_smi_larb_component_ops);
+ ret = component_add(dev, &mtk_smi_larb_component_ops);
+ if (ret)
+ goto err_pm_disable;
+ return 0;
+
+err_pm_disable:
+ pm_runtime_disable(dev);
+ device_link_remove(dev, larb->smi_common_dev);
+ return ret;
}

static int mtk_smi_larb_remove(struct platform_device *pdev)
--
2.18.0

2021-09-14 11:40:57

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 07/13] memory: mtk-smi: Add device link for smi-sub-common

In mt8195, there are some larbs connect with the smi-sub-common, then
connect with smi-common.

Before we create device link between smi-larb with smi-common. If we have
sub-common, we should use device link the smi-larb and smi-sub-common,
then use device link between the smi-sub-common with smi-common. This is
for enabling clock/power automatically.

Move the device link code to a new interface for reusing.

Signed-off-by: Yong Wu <[email protected]>
Reviewed-by: Ikjoon Jang <[email protected]>
---
drivers/memory/mtk-smi.c | 75 +++++++++++++++++++++++++++-------------
1 file changed, 51 insertions(+), 24 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index b362d528944e..5c2bd5795cfd 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -61,7 +61,8 @@

enum mtk_smi_type {
MTK_SMI_GEN1,
- MTK_SMI_GEN2
+ MTK_SMI_GEN2, /* gen2 smi common */
+ MTK_SMI_GEN2_SUB_COMM, /* gen2 smi sub common */
};

#define MTK_SMI_CLK_NR_MAX 4
@@ -99,13 +100,14 @@ struct mtk_smi {
void __iomem *smi_ao_base; /* only for gen1 */
void __iomem *base; /* only for gen2 */
};
+ struct device *smi_common_dev; /* for sub common */
const struct mtk_smi_common_plat *plat;
};

struct mtk_smi_larb { /* larb: local arbiter */
struct mtk_smi smi;
void __iomem *base;
- struct device *smi_common_dev;
+ struct device *smi_common_dev; /* common or sub-common dev */
const struct mtk_smi_larb_gen *larb_gen;
int larbid;
u32 *mmu;
@@ -268,6 +270,38 @@ static const struct of_device_id mtk_smi_larb_of_ids[] = {
{}
};

+static int mtk_smi_device_link_common(struct device *dev, struct device **com_dev)
+{
+ struct platform_device *smi_com_pdev;
+ struct device_node *smi_com_node;
+ struct device *smi_com_dev;
+ struct device_link *link;
+
+ smi_com_node = of_parse_phandle(dev->of_node, "mediatek,smi", 0);
+ if (!smi_com_node)
+ return -EINVAL;
+
+ smi_com_pdev = of_find_device_by_node(smi_com_node);
+ of_node_put(smi_com_node);
+ if (smi_com_pdev) {
+ /* smi common is the supplier, Make sure it is ready before */
+ if (!platform_get_drvdata(smi_com_pdev))
+ return -EPROBE_DEFER;
+ smi_com_dev = &smi_com_pdev->dev;
+ link = device_link_add(dev, smi_com_dev,
+ DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
+ if (!link) {
+ dev_err(dev, "Unable to link smi-common dev\n");
+ return -ENODEV;
+ }
+ *com_dev = smi_com_dev;
+ } else {
+ dev_err(dev, "Failed to get the smi_common device\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
static int mtk_smi_dts_clk_init(struct device *dev, struct mtk_smi *smi,
const char * const clks[],
unsigned int clk_nr_required,
@@ -294,9 +328,6 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
struct mtk_smi_larb *larb;
struct resource *res;
struct device *dev = &pdev->dev;
- struct device_node *smi_node;
- struct platform_device *smi_pdev;
- struct device_link *link;
int ret;

larb = devm_kzalloc(dev, sizeof(*larb), GFP_KERNEL);
@@ -315,26 +346,10 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
return ret;

larb->smi.dev = dev;
- smi_node = of_parse_phandle(dev->of_node, "mediatek,smi", 0);
- if (!smi_node)
- return -EINVAL;

- smi_pdev = of_find_device_by_node(smi_node);
- of_node_put(smi_node);
- if (smi_pdev) {
- if (!platform_get_drvdata(smi_pdev))
- return -EPROBE_DEFER;
- larb->smi_common_dev = &smi_pdev->dev;
- link = device_link_add(dev, larb->smi_common_dev,
- DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
- if (!link) {
- dev_err(dev, "Unable to link smi-common dev\n");
- return -ENODEV;
- }
- } else {
- dev_err(dev, "Failed to get the smi_common device\n");
- return -EINVAL;
- }
+ ret = mtk_smi_device_link_common(dev, &larb->smi_common_dev);
+ if (ret < 0)
+ return ret;

pm_runtime_enable(dev);
platform_set_drvdata(pdev, larb);
@@ -483,6 +498,14 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
if (IS_ERR(common->base))
return PTR_ERR(common->base);
}
+
+ /* link its smi-common if this is smi-sub-common */
+ if (common->plat->type == MTK_SMI_GEN2_SUB_COMM) {
+ ret = mtk_smi_device_link_common(dev, &common->smi_common_dev);
+ if (ret < 0)
+ return ret;
+ }
+
pm_runtime_enable(dev);
platform_set_drvdata(pdev, common);
return 0;
@@ -490,6 +513,10 @@ static int mtk_smi_common_probe(struct platform_device *pdev)

static int mtk_smi_common_remove(struct platform_device *pdev)
{
+ struct mtk_smi *common = dev_get_drvdata(&pdev->dev);
+
+ if (common->plat->type == MTK_SMI_GEN2_SUB_COMM)
+ device_link_remove(&pdev->dev, common->smi_common_dev);
pm_runtime_disable(&pdev->dev);
return 0;
}
--
2.18.0

2021-09-14 11:41:17

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 09/13] memory: mtk-smi: Use devm_platform_ioremap_resource

No functional change. Simplify probing code.

Signed-off-by: Yong Wu <[email protected]>
Reviewed-by: Ikjoon Jang <[email protected]>
---
drivers/memory/mtk-smi.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 58d9f7667490..a001e41f5074 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -328,7 +328,6 @@ static int mtk_smi_dts_clk_init(struct device *dev, struct mtk_smi *smi,
static int mtk_smi_larb_probe(struct platform_device *pdev)
{
struct mtk_smi_larb *larb;
- struct resource *res;
struct device *dev = &pdev->dev;
int ret;

@@ -337,8 +336,7 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
return -ENOMEM;

larb->larb_gen = of_device_get_match_data(dev);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- larb->base = devm_ioremap_resource(dev, res);
+ larb->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(larb->base))
return PTR_ERR(larb->base);

@@ -460,7 +458,6 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct mtk_smi *common;
- struct resource *res;
int ret, clk_required = MTK_SMI_COM_REQ_CLK_NR;

common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL);
@@ -486,8 +483,7 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
* base.
*/
if (common->plat->type == MTK_SMI_GEN1) {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- common->smi_ao_base = devm_ioremap_resource(dev, res);
+ common->smi_ao_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(common->smi_ao_base))
return PTR_ERR(common->smi_ao_base);

@@ -499,8 +495,7 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
if (ret)
return ret;
} else {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- common->base = devm_ioremap_resource(dev, res);
+ common->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(common->base))
return PTR_ERR(common->base);
}
--
2.18.0

2021-09-14 11:41:22

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 10/13] memory: mtk-smi: mt8195: Add smi support

MT8195 has two smi-common, their IP are the same. Only the larbs that
connect with the smi-common are different. thus the bus_sel are different
for the two smi-common.

Signed-off-by: Yong Wu <[email protected]>
Reviewed-by: Ikjoon Jang <[email protected]>
---
drivers/memory/mtk-smi.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index a001e41f5074..35853ba980c9 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -261,6 +261,10 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt8192 = {
.config_port = mtk_smi_larb_config_port_gen2_general,
};

+static const struct mtk_smi_larb_gen mtk_smi_larb_mt8195 = {
+ .config_port = mtk_smi_larb_config_port_gen2_general,
+};
+
static const struct of_device_id mtk_smi_larb_of_ids[] = {
{.compatible = "mediatek,mt2701-smi-larb", .data = &mtk_smi_larb_mt2701},
{.compatible = "mediatek,mt2712-smi-larb", .data = &mtk_smi_larb_mt2712},
@@ -269,6 +273,7 @@ static const struct of_device_id mtk_smi_larb_of_ids[] = {
{.compatible = "mediatek,mt8173-smi-larb", .data = &mtk_smi_larb_mt8173},
{.compatible = "mediatek,mt8183-smi-larb", .data = &mtk_smi_larb_mt8183},
{.compatible = "mediatek,mt8192-smi-larb", .data = &mtk_smi_larb_mt8192},
+ {.compatible = "mediatek,mt8195-smi-larb", .data = &mtk_smi_larb_mt8195},
{}
};

@@ -443,6 +448,24 @@ static const struct mtk_smi_common_plat mtk_smi_common_mt8192 = {
F_MMU1_LARB(6),
};

+static const struct mtk_smi_common_plat mtk_smi_common_mt8195_vdo = {
+ .type = MTK_SMI_GEN2,
+ .has_gals = true,
+ .bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(3) | F_MMU1_LARB(5) |
+ F_MMU1_LARB(7),
+};
+
+static const struct mtk_smi_common_plat mtk_smi_common_mt8195_vpp = {
+ .type = MTK_SMI_GEN2,
+ .has_gals = true,
+ .bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(7),
+};
+
+static const struct mtk_smi_common_plat mtk_smi_sub_common_mt8195 = {
+ .type = MTK_SMI_GEN2_SUB_COMM,
+ .has_gals = true,
+};
+
static const struct of_device_id mtk_smi_common_of_ids[] = {
{.compatible = "mediatek,mt2701-smi-common", .data = &mtk_smi_common_gen1},
{.compatible = "mediatek,mt2712-smi-common", .data = &mtk_smi_common_gen2},
@@ -451,6 +474,9 @@ static const struct of_device_id mtk_smi_common_of_ids[] = {
{.compatible = "mediatek,mt8173-smi-common", .data = &mtk_smi_common_gen2},
{.compatible = "mediatek,mt8183-smi-common", .data = &mtk_smi_common_mt8183},
{.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},
+ {.compatible = "mediatek,mt8195-smi-sub-common", .data = &mtk_smi_sub_common_mt8195},
{}
};

--
2.18.0

2021-09-14 11:41:22

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 11/13] memory: mtk-smi: mt8195: Add initial setting for smi-common

To improve the performance, add initial setting for smi-common.
some register use some fix setting(suggested from DE).

Signed-off-by: Yong Wu <[email protected]>
---
drivers/memory/mtk-smi.c | 42 ++++++++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 35853ba980c9..689a45b39a65 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -18,11 +18,19 @@
#include <dt-bindings/memory/mtk-memory-port.h>

/* SMI COMMON */
+#define SMI_L1LEN 0x100
+
#define SMI_BUS_SEL 0x220
#define SMI_BUS_LARB_SHIFT(larbid) ((larbid) << 1)
/* All are MMU0 defaultly. Only specialize mmu1 here. */
#define F_MMU1_LARB(larbid) (0x1 << SMI_BUS_LARB_SHIFT(larbid))

+#define SMI_M4U_TH 0x234
+#define SMI_FIFO_TH1 0x238
+#define SMI_FIFO_TH2 0x23c
+#define SMI_DCM 0x300
+#define SMI_DUMMY 0x444
+
/* SMI LARB */

/* Below are about mmu enable registers, they are different in SoCs */
@@ -59,6 +67,13 @@
(_id << 8 | _id << 10 | _id << 12 | _id << 14); \
})

+#define SMI_COMMON_INIT_REGS_NR 6
+
+struct mtk_smi_reg_pair {
+ unsigned int offset;
+ u32 value;
+};
+
enum mtk_smi_type {
MTK_SMI_GEN1,
MTK_SMI_GEN2, /* gen2 smi common */
@@ -85,6 +100,8 @@ struct mtk_smi_common_plat {
enum mtk_smi_type type;
bool has_gals;
u32 bus_sel; /* Balance some larbs to enter mmu0 or mmu1 */
+
+ const struct mtk_smi_reg_pair *init;
};

struct mtk_smi_larb_gen {
@@ -419,6 +436,15 @@ static struct platform_driver mtk_smi_larb_driver = {
}
};

+static const struct mtk_smi_reg_pair mtk_smi_common_mt8195_init[SMI_COMMON_INIT_REGS_NR] = {
+ {SMI_L1LEN, 0xb},
+ {SMI_M4U_TH, 0xe100e10},
+ {SMI_FIFO_TH1, 0x506090a},
+ {SMI_FIFO_TH2, 0x506090a},
+ {SMI_DCM, 0x4f1},
+ {SMI_DUMMY, 0x1},
+};
+
static const struct mtk_smi_common_plat mtk_smi_common_gen1 = {
.type = MTK_SMI_GEN1,
};
@@ -453,12 +479,14 @@ static const struct mtk_smi_common_plat mtk_smi_common_mt8195_vdo = {
.has_gals = true,
.bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(3) | F_MMU1_LARB(5) |
F_MMU1_LARB(7),
+ .init = mtk_smi_common_mt8195_init,
};

static const struct mtk_smi_common_plat mtk_smi_common_mt8195_vpp = {
.type = MTK_SMI_GEN2,
.has_gals = true,
.bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(7),
+ .init = mtk_smi_common_mt8195_init,
};

static const struct mtk_smi_common_plat mtk_smi_sub_common_mt8195 = {
@@ -551,15 +579,21 @@ static int mtk_smi_common_remove(struct platform_device *pdev)
static int __maybe_unused mtk_smi_common_resume(struct device *dev)
{
struct mtk_smi *common = dev_get_drvdata(dev);
- u32 bus_sel = common->plat->bus_sel;
- int ret;
+ const struct mtk_smi_reg_pair *init = common->plat->init;
+ u32 bus_sel = common->plat->bus_sel; /* default is 0 */
+ int ret, i;

ret = clk_bulk_prepare_enable(common->clk_num, common->clks);
if (ret)
return ret;

- if (common->plat->type == MTK_SMI_GEN2 && bus_sel)
- writel(bus_sel, common->base + SMI_BUS_SEL);
+ if (common->plat->type != MTK_SMI_GEN2)
+ return 0;
+
+ for (i = 0; i < SMI_COMMON_INIT_REGS_NR && init && init[i].offset; i++)
+ writel_relaxed(init[i].value, common->base + init[i].offset);
+
+ writel(bus_sel, common->base + SMI_BUS_SEL);
return 0;
}

--
2.18.0

2021-09-14 11:41:39

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 12/13] memory: mtk-smi: mt8195: Add initial setting for smi-larb

To improve the performance, We add some initial setting for smi larbs.
there are two part:
1), Each port has the special ostd(outstanding) value in each larb.
2), Two general settings for each larb.
a. THRT_UPDATE: the value in bits[7:4] of 0x24 is not so good.
The HW default is 4, and we expect it is 5, thus, add a flag to update
it. This is only a DE recommendatory value, not a actual issue.
The register name(THRT_CON) means: throttling control, and the field
RD_NU_LMT means: Read Non-ultra commands limit.
This change means update the Read non-ultra command from 4 to 5 here.

b. SW_FLAG: Set 1 to the FLAG register. this is only for helping
debug. We could confirm if the larb is reset from this value is 1 or 0.

In some SoC, this setting maybe changed dynamically for some special case
like 4K, and this initial setting is enough in mt8195.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/memory/mtk-smi.c | 79 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 78 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 689a45b39a65..b883dcc0bbfa 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -32,6 +32,15 @@
#define SMI_DUMMY 0x444

/* SMI LARB */
+#define SMI_LARB_CMD_THRT_CON 0x24
+#define SMI_LARB_THRT_RD_NU_LMT_MSK GENMASK(7, 4)
+#define SMI_LARB_THRT_RD_NU_LMT (5 << 4)
+
+#define SMI_LARB_SW_FLAG 0x40
+#define SMI_LARB_SW_FLAG_1 0x1
+
+#define SMI_LARB_OSTDL_PORT 0x200
+#define SMI_LARB_OSTDL_PORTx(id) (SMI_LARB_OSTDL_PORT + (((id) & 0x1f) << 2))

/* Below are about mmu enable registers, they are different in SoCs */
/* gen1: mt2701 */
@@ -68,6 +77,11 @@
})

#define SMI_COMMON_INIT_REGS_NR 6
+#define SMI_LARB_PORT_NR_MAX 32
+
+#define MTK_SMI_FLAG_THRT_UPDATE BIT(0)
+#define MTK_SMI_FLAG_SW_FLAG BIT(1)
+#define MTK_SMI_CAPS(flags, _x) (!!((flags) & (_x)))

struct mtk_smi_reg_pair {
unsigned int offset;
@@ -108,6 +122,8 @@ struct mtk_smi_larb_gen {
int port_in_larb[MTK_LARB_NR_MAX + 1];
void (*config_port)(struct device *dev);
unsigned int larb_direct_to_common_mask;
+ unsigned int flags_general;
+ const u8 (*ostd)[SMI_LARB_PORT_NR_MAX];
};

struct mtk_smi {
@@ -224,12 +240,26 @@ static void mtk_smi_larb_config_port_mt8173(struct device *dev)
static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
{
struct mtk_smi_larb *larb = dev_get_drvdata(dev);
- u32 reg;
+ u32 reg, flags_general = larb->larb_gen->flags_general;
+ const u8 *larbostd = larb->larb_gen->ostd[larb->larbid];
int i;

if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
return;

+ if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_THRT_UPDATE)) {
+ reg = readl_relaxed(larb->base + SMI_LARB_CMD_THRT_CON);
+ reg &= ~SMI_LARB_THRT_RD_NU_LMT_MSK;
+ reg |= SMI_LARB_THRT_RD_NU_LMT;
+ writel_relaxed(reg, larb->base + SMI_LARB_CMD_THRT_CON);
+ }
+
+ if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SW_FLAG))
+ writel_relaxed(SMI_LARB_SW_FLAG_1, larb->base + SMI_LARB_SW_FLAG);
+
+ for (i = 0; i < SMI_LARB_PORT_NR_MAX && larbostd && !!larbostd[i]; i++)
+ writel_relaxed(larbostd[i], larb->base + SMI_LARB_OSTDL_PORTx(i));
+
for_each_set_bit(i, (unsigned long *)larb->mmu, 32) {
reg = readl_relaxed(larb->base + SMI_LARB_NONSEC_CON(i));
reg |= F_MMU_EN;
@@ -238,6 +268,51 @@ static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
}
}

+static const u8 mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX] = {
+ [0] = {0x0a, 0xc, 0x22, 0x22, 0x01, 0x0a,}, /* larb0 */
+ [1] = {0x0a, 0xc, 0x22, 0x22, 0x01, 0x0a,}, /* larb1 */
+ [2] = {0x12, 0x12, 0x12, 0x12, 0x0a,}, /* ... */
+ [3] = {0x12, 0x12, 0x12, 0x12, 0x28, 0x28, 0x0a,},
+ [4] = {0x06, 0x01, 0x17, 0x06, 0x0a,},
+ [5] = {0x06, 0x01, 0x17, 0x06, 0x06, 0x01, 0x06, 0x0a,},
+ [6] = {0x06, 0x01, 0x06, 0x0a,},
+ [7] = {0x0c, 0x0c, 0x12,},
+ [8] = {0x0c, 0x0c, 0x12,},
+ [9] = {0x0a, 0x08, 0x04, 0x06, 0x01, 0x01, 0x10, 0x18, 0x11, 0x0a,
+ 0x08, 0x04, 0x11, 0x06, 0x02, 0x06, 0x01, 0x11, 0x11, 0x06,},
+ [10] = {0x18, 0x08, 0x01, 0x01, 0x20, 0x12, 0x18, 0x06, 0x05, 0x10,
+ 0x08, 0x08, 0x10, 0x08, 0x08, 0x18, 0x0c, 0x09, 0x0b, 0x0d,
+ 0x0d, 0x06, 0x10, 0x10,},
+ [11] = {0x0e, 0x0e, 0x0e, 0x0e, 0x0e, 0x0e, 0x01, 0x01, 0x01, 0x01,},
+ [12] = {0x09, 0x09, 0x05, 0x05, 0x0c, 0x18, 0x02, 0x02, 0x04, 0x02,},
+ [13] = {0x02, 0x02, 0x12, 0x12, 0x02, 0x02, 0x02, 0x02, 0x08, 0x01,},
+ [14] = {0x12, 0x12, 0x02, 0x02, 0x02, 0x02, 0x16, 0x01, 0x16, 0x01,
+ 0x01, 0x02, 0x02, 0x08, 0x02,},
+ [15] = {},
+ [16] = {0x28, 0x02, 0x02, 0x12, 0x02, 0x12, 0x10, 0x02, 0x02, 0x0a,
+ 0x12, 0x02, 0x0a, 0x16, 0x02, 0x04,},
+ [17] = {0x1a, 0x0e, 0x0a, 0x0a, 0x0c, 0x0e, 0x10,},
+ [18] = {0x12, 0x06, 0x12, 0x06,},
+ [19] = {0x01, 0x04, 0x01, 0x01, 0x01, 0x01, 0x01, 0x04, 0x04, 0x01,
+ 0x01, 0x01, 0x04, 0x0a, 0x06, 0x01, 0x01, 0x01, 0x0a, 0x06,
+ 0x01, 0x01, 0x05, 0x03, 0x03, 0x04, 0x01,},
+ [20] = {0x01, 0x04, 0x01, 0x01, 0x01, 0x01, 0x01, 0x04, 0x04, 0x01,
+ 0x01, 0x01, 0x04, 0x0a, 0x06, 0x01, 0x01, 0x01, 0x0a, 0x06,
+ 0x01, 0x01, 0x05, 0x03, 0x03, 0x04, 0x01,},
+ [21] = {0x28, 0x19, 0x0c, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x04,},
+ [22] = {0x28, 0x19, 0x0c, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x04,},
+ [23] = {0x18, 0x01,},
+ [24] = {0x01, 0x01, 0x04, 0x01, 0x01, 0x01, 0x01, 0x01, 0x04, 0x01,
+ 0x01, 0x01,},
+ [25] = {0x02, 0x02, 0x02, 0x28, 0x16, 0x02, 0x02, 0x02, 0x12, 0x16,
+ 0x02, 0x01,},
+ [26] = {0x02, 0x02, 0x02, 0x28, 0x16, 0x02, 0x02, 0x02, 0x12, 0x16,
+ 0x02, 0x01,},
+ [27] = {0x02, 0x02, 0x02, 0x28, 0x16, 0x02, 0x02, 0x02, 0x12, 0x16,
+ 0x02, 0x01,},
+ [28] = {0x1a, 0x0e, 0x0a, 0x0a, 0x0c, 0x0e, 0x10,},
+};
+
static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
.port_in_larb = {
LARB0_PORT_OFFSET, LARB1_PORT_OFFSET,
@@ -280,6 +355,8 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt8192 = {

static const struct mtk_smi_larb_gen mtk_smi_larb_mt8195 = {
.config_port = mtk_smi_larb_config_port_gen2_general,
+ .flags_general = MTK_SMI_FLAG_THRT_UPDATE | MTK_SMI_FLAG_SW_FLAG,
+ .ostd = mtk_smi_larb_mt8195_ostd,
};

static const struct of_device_id mtk_smi_larb_of_ids[] = {
--
2.18.0

2021-09-14 11:41:53

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 13/13] MAINTAINERS: Add entry for MediaTek SMI

I am the author of MediaTek SMI driver, and will to maintain
and develop it further.
Add myself to cover these items.

Signed-off-by: Yong Wu <[email protected]>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index eeb4c70b3d5b..52b956fedfbc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11865,6 +11865,14 @@ M: Sean Wang <[email protected]>
S: Maintained
F: drivers/char/hw_random/mtk-rng.c

+MEDIATEK SMI DRIVER
+M: Yong Wu <[email protected]>
+L: [email protected] (moderated for non-subscribers)
+S: Supported
+F: Documentation/devicetree/bindings/memory-controllers/mediatek,smi*
+F: drivers/memory/mtk-smi.c
+F: include/soc/mediatek/smi.h
+
MEDIATEK SWITCH DRIVER
M: Sean Wang <[email protected]>
M: Landen Chao <[email protected]>
--
2.18.0

2021-09-14 11:43:10

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v4 08/13] memory: mtk-smi: Add clocks for smi-sub-common

SMI sub common only have one output port. thus it has only one gals
clocks(gals0). then, smi-sub-common require the three clocks(apb/smi/gals0)
in has_gals case.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/memory/mtk-smi.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 5c2bd5795cfd..58d9f7667490 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -74,10 +74,12 @@ static const char * const mtk_smi_larb_clks[] = {"apb", "smi", "gals"};

/*
* common: Require these four clocks in has_gals case. Otherwise, only apb/smi are required.
+ * sub common: Require apb/smi/gals0 clocks in has_gals case. Otherwise, only apb/smi are required.
*/
static const char * const mtk_smi_common_clks[] = {"apb", "smi", "gals0", "gals1"};
#define MTK_SMI_COM_REQ_CLK_NR 2
#define MTK_SMI_COM_GALS_REQ_CLK_NR MTK_SMI_CLK_NR_MAX
+#define MTK_SMI_SUB_COM_GALS_REQ_CLK_NR 3

struct mtk_smi_common_plat {
enum mtk_smi_type type;
@@ -467,8 +469,12 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
common->dev = dev;
common->plat = of_device_get_match_data(dev);

- if (common->plat->has_gals)
- clk_required = MTK_SMI_COM_GALS_REQ_CLK_NR;
+ if (common->plat->has_gals) {
+ if (common->plat->type == MTK_SMI_GEN2)
+ clk_required = MTK_SMI_COM_GALS_REQ_CLK_NR;
+ else if (common->plat->type == MTK_SMI_GEN2_SUB_COMM)
+ clk_required = MTK_SMI_SUB_COM_GALS_REQ_CLK_NR;
+ }
ret = mtk_smi_dts_clk_init(dev, common, mtk_smi_common_clks, clk_required, 0);
if (ret)
return ret;
--
2.18.0

2021-09-21 21:14:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 02/13] dt-bindings: memory: mediatek: Add mt8195 smi sub common

On Tue, 14 Sep 2021 19:36:52 +0800, Yong Wu wrote:
> Add the binding for smi-sub-common. The SMI block diagram like this:
>
> IOMMU
> | |
> smi-common
> ------------------
> | .... |
> larb0 larb7 <-max is 8
>
> The smi-common connects with smi-larb and IOMMU. The maximum larbs number
> that connects with a smi-common is 8. If the engines number is over 8,
> sometimes we use a smi-sub-common which is nearly same with smi-common.
> It supports up to 8 input and 1 output(smi-common has 2 output)
>
> Something like:
>
> IOMMU
> | |
> smi-common
> ---------------------
> | | ...
> larb0 sub-common ... <-max is 8
> -----------
> | | ... <-max is 8 too.
> larb2 larb5
>
> We don't need extra SW setting for smi-sub-common, only the sub-common has
> special clocks need to enable when the engines access dram.
>
> If it is sub-common, it should have a "mediatek,smi" phandle to point to
> its smi-common. meanwhile the sub-common only has one gals clock.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> change note: add "else mediatek,smi: false".
> ---
> .../mediatek,smi-common.yaml | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>

2021-09-22 06:46:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 00/13] MT8195 SMI support

On Tue, 14 Sep 2021 19:36:50 +0800, Yong Wu wrote:
> This patchset mainly adds SMI support for mt8195.
>
> Comparing with the previous version, add two new functions:
> a) add smi sub common
> b) add initial setting for smi-common and smi-larb.
>
> Change note:
> v4:1) base on v5.15-rc1
> 2) In the dt-binding:
> a. add "else mediatek,smi: false." in the yaml.
> b. Remove mediatek,smi_sub_common. since we have only 2 level currently,
> It should be smi-sub-common if that node has "mediatek,smi". otherwise,
> it is smi-common.
>
> [...]

Applied, thanks!

[01/13] dt-bindings: memory: mediatek: Add mt8195 smi binding
commit: b01065eee432b3ae91a2c0aaab66c2cae2e9812d
[02/13] dt-bindings: memory: mediatek: Add mt8195 smi sub common
commit: 599e681a31a2dfa7359b8e420a1157ed015f840b
[03/13] memory: mtk-smi: Use clk_bulk clock ops
commit: 0e14917c57f9d8b9b7d4f41813849a29659447b3
[04/13] memory: mtk-smi: Rename smi_gen to smi_type
commit: a5c18986f404206fdbc27f208620dc13bffb5657
[05/13] memory: mtk-smi: Adjust some code position
commit: 534e0ad2ed4f9296a8c7ccb1a393bc4d5000dbad
[06/13] memory: mtk-smi: Add error handle for smi_probe
commit: 30b869e77a1c626190bbbe6b4e5f5382b0102ac3
[07/13] memory: mtk-smi: Add device link for smi-sub-common
commit: 47404757702ec8aa5c8310cdf58a267081f0ce6c
[08/13] memory: mtk-smi: Add clocks for smi-sub-common
commit: 3e4f74e0ea5a6a6d6d825fd7afd8a10afbd1152c
[09/13] memory: mtk-smi: Use devm_platform_ioremap_resource
commit: 912fea8bf8d854aef967c82a279ffd20be0326d7
[10/13] memory: mtk-smi: mt8195: Add smi support
commit: cc4f9dcd9c1543394d6ee0d635551a2bd96bcacb
[11/13] memory: mtk-smi: mt8195: Add initial setting for smi-common
commit: 431e9cab7097b5d5eb3cf2b04d4b12d272df85e0
[12/13] memory: mtk-smi: mt8195: Add initial setting for smi-larb
commit: fe6dd2a4017d3dfbf4a530d95270a1d498a16a4c
[13/13] MAINTAINERS: Add entry for MediaTek SMI
commit: 93403ede5aa4edeec2c63541b185d9c4fc9ae1e4

Best regards,
--
Krzysztof Kozlowski <[email protected]>

Subject: Re: [PATCH v4 03/13] memory: mtk-smi: Use clk_bulk clock ops

> Use clk_bulk interface instead of the orginal one to simplify the code.
>
> For SMI larbs: Require apb/smi clocks while gals is optional.
> For SMI common: Require apb/smi/gals0/gal1 in has_gals case. Otherwise,
> also only require apb/smi, No optional clk here.
>
> About the "has_gals" flag, for smi larbs, the gals clock also may be
> optional even this platform support it. thus it always use
> *_bulk_get_optional, then the flag has_gals is unnecessary. Remove it.
> The smi_common's has_gals still keep it.
>
> Also remove clk fail logs since bulk interface already output fail log.
>
> Signed-off-by: Yong Wu <[email protected]>

Hello Yong,
thanks for the patch! However, I have an improvement to point out:

> ---
> drivers/memory/mtk-smi.c | 143 +++++++++++++++------------------------
> 1 file changed, 55 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index c5fb51f73b34..f91eaf5c3ab0 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -60,6 +60,20 @@ enum mtk_smi_gen {
> MTK_SMI_GEN2
> };
>
> +#define MTK_SMI_CLK_NR_MAX 4

This refers to mtk_smi_common_clks[] and should be probably moved after that.
In any case, I don't think that there's any need to manually define this as 4,
as you can simply use the macro ARRAY_SIZE(mtk_smi_common_clks).
Using that will make you able to not update this definition everytime an update
occurs to the mtk_smi_common_clks array.

> +
> +/* larbs: Require apb/smi clocks while gals is optional. */
> +static const char * const mtk_smi_larb_clks[] = {"apb", "smi", "gals"};
> +#define MTK_SMI_LARB_REQ_CLK_NR 2
> +#define MTK_SMI_LARB_OPT_CLK_NR 1
> +
> +/*
> + * common: Require these four clocks in has_gals case. Otherwise, only apb/smi are required.
> + */
> +static const char * const mtk_smi_common_clks[] = {"apb", "smi", "gals0", "gals1"};
> +#define MTK_SMI_COM_REQ_CLK_NR 2
> +#define MTK_SMI_COM_GALS_REQ_CLK_NR MTK_SMI_CLK_NR_MAX
> +

Apart from that,
Acked-By: AngeloGioacchino Del Regno <[email protected]>

Regards,
- Angelo

2021-10-16 09:29:55

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 03/13] memory: mtk-smi: Use clk_bulk clock ops

On 15/10/2021 15:38, AngeloGioacchino Del Regno wrote:
>> Use clk_bulk interface instead of the orginal one to simplify the code.
>>
>> For SMI larbs: Require apb/smi clocks while gals is optional.
>> For SMI common: Require apb/smi/gals0/gal1 in has_gals case. Otherwise,
>> also only require apb/smi, No optional clk here.
>>
>> About the "has_gals" flag, for smi larbs, the gals clock also may be
>> optional even this platform support it. thus it always use
>> *_bulk_get_optional, then the flag has_gals is unnecessary. Remove it.
>> The smi_common's has_gals still keep it.
>>
>> Also remove clk fail logs since bulk interface already output fail log.
>>
>> Signed-off-by: Yong Wu <[email protected]>
>
> Hello Yong,
> thanks for the patch! However, I have an improvement to point out:
>
>> ---
>> drivers/memory/mtk-smi.c | 143 +++++++++++++++------------------------
>> 1 file changed, 55 insertions(+), 88 deletions(-)
>>
>> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
>> index c5fb51f73b34..f91eaf5c3ab0 100644
>> --- a/drivers/memory/mtk-smi.c
>> +++ b/drivers/memory/mtk-smi.c
>> @@ -60,6 +60,20 @@ enum mtk_smi_gen {
>> MTK_SMI_GEN2
>> };
>>
>> +#define MTK_SMI_CLK_NR_MAX 4
>
> This refers to mtk_smi_common_clks[] and should be probably moved after that.
> In any case, I don't think that there's any need to manually define this as 4,
> as you can simply use the macro ARRAY_SIZE(mtk_smi_common_clks).
> Using that will make you able to not update this definition everytime an update
> occurs to the mtk_smi_common_clks array.
>
>> +
>> +/* larbs: Require apb/smi clocks while gals is optional. */
>> +static const char * const mtk_smi_larb_clks[] = {"apb", "smi", "gals"};
>> +#define MTK_SMI_LARB_REQ_CLK_NR 2
>> +#define MTK_SMI_LARB_OPT_CLK_NR 1
>> +
>> +/*
>> + * common: Require these four clocks in has_gals case. Otherwise, only apb/smi are required.
>> + */
>> +static const char * const mtk_smi_common_clks[] = {"apb", "smi", "gals0", "gals1"};
>> +#define MTK_SMI_COM_REQ_CLK_NR 2
>> +#define MTK_SMI_COM_GALS_REQ_CLK_NR MTK_SMI_CLK_NR_MAX
>> +
>
> Apart from that,
> Acked-By: AngeloGioacchino Del Regno <[email protected]>

The patchset was merged around a month ago:
https://lore.kernel.org/lkml/[email protected]/


Best regards,
Krzysztof

Subject: Re: [PATCH v4 03/13] memory: mtk-smi: Use clk_bulk clock ops

Il 15/10/21 15:43, Krzysztof Kozlowski ha scritto:
> On 15/10/2021 15:38, AngeloGioacchino Del Regno wrote:
>>> Use clk_bulk interface instead of the orginal one to simplify the code.
>>>
>>> For SMI larbs: Require apb/smi clocks while gals is optional.
>>> For SMI common: Require apb/smi/gals0/gal1 in has_gals case. Otherwise,
>>> also only require apb/smi, No optional clk here.
>>>
>>> About the "has_gals" flag, for smi larbs, the gals clock also may be
>>> optional even this platform support it. thus it always use
>>> *_bulk_get_optional, then the flag has_gals is unnecessary. Remove it.
>>> The smi_common's has_gals still keep it.
>>>
>>> Also remove clk fail logs since bulk interface already output fail log.
>>>
>>> Signed-off-by: Yong Wu <[email protected]>
>>
>> Hello Yong,
>> thanks for the patch! However, I have an improvement to point out:
>>
>>> ---
>>> drivers/memory/mtk-smi.c | 143 +++++++++++++++------------------------
>>> 1 file changed, 55 insertions(+), 88 deletions(-)
>>>
>>> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
>>> index c5fb51f73b34..f91eaf5c3ab0 100644
>>> --- a/drivers/memory/mtk-smi.c
>>> +++ b/drivers/memory/mtk-smi.c
>>> @@ -60,6 +60,20 @@ enum mtk_smi_gen {
>>> MTK_SMI_GEN2
>>> };
>>>
>>> +#define MTK_SMI_CLK_NR_MAX 4
>>
>> This refers to mtk_smi_common_clks[] and should be probably moved after that.
>> In any case, I don't think that there's any need to manually define this as 4,
>> as you can simply use the macro ARRAY_SIZE(mtk_smi_common_clks).
>> Using that will make you able to not update this definition everytime an update
>> occurs to the mtk_smi_common_clks array.
>>
>>> +
>>> +/* larbs: Require apb/smi clocks while gals is optional. */
>>> +static const char * const mtk_smi_larb_clks[] = {"apb", "smi", "gals"};
>>> +#define MTK_SMI_LARB_REQ_CLK_NR 2
>>> +#define MTK_SMI_LARB_OPT_CLK_NR 1
>>> +
>>> +/*
>>> + * common: Require these four clocks in has_gals case. Otherwise, only apb/smi are required.
>>> + */
>>> +static const char * const mtk_smi_common_clks[] = {"apb", "smi", "gals0", "gals1"};
>>> +#define MTK_SMI_COM_REQ_CLK_NR 2
>>> +#define MTK_SMI_COM_GALS_REQ_CLK_NR MTK_SMI_CLK_NR_MAX
>>> +
>>
>> Apart from that,
>> Acked-By: AngeloGioacchino Del Regno <[email protected]>
>
> The patchset was merged around a month ago:
> https://lore.kernel.org/lkml/[email protected]/
>
>
> Best regards,
> Krzysztof
>

Whoops. Sorry for that.
I'll send a patch to address what I pointed out, unless the original author of
this series wants to.

Thanks,
- Angelo