[v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[Crystal Guo]

A MediaTek DRAM controller interface to provide the current DDR data rate.

Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
---
 .../mediatek,common-dramc.yaml                | 129 ------------------
 .../memory-controllers/mediatek,dramc.yaml    |  44 ++++++
 2 files changed, 44 insertions(+), 129 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml
deleted file mode 100644
index c9e608c7f183..000000000000
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml
+++ /dev/null
@@ -1,129 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
-# Copyright (c) 2024 MediaTek Inc.
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/memory-controllers/mediatek,common-dramc.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: MediaTek Common DRAMC (DRAM Controller)
-
-maintainers:
-  - Crystal Guo <crystal.guo@mediatek.com>
-
-description: |
-  The DRAM controller of MediaTek SoC provides an interface to
-  get the current data rate of DRAM.
-
-properties:
-  compatible:
-    const: mediatek,common-dramc
-
-  reg:
-    minItems: 9
-    items:
-      - description: DRAMC_AO_CHA_BASE
-      - description: DRAMC_AO_CHB_BASE
-      - description: DRAMC_AO_CHC_BASE
-      - description: DRAMC_AO_CHD_BASE
-      - description: DRAMC_NAO_CHA_BASE
-      - description: DRAMC_NAO_CHB_BASE
-      - description: DRAMC_NAO_CHC_BASE
-      - description: DRAMC_NAO_CHD_BASE
-      - description: DDRPHY_AO_CHA_BASE
-      - description: DDRPHY_AO_CHB_BASE
-      - description: DDRPHY_AO_CHC_BASE
-      - description: DDRPHY_AO_CHD_BASE
-      - description: DDRPHY_NAO_CHA_BASE
-      - description: DDRPHY_NAO_CHB_BASE
-      - description: DDRPHY_NAO_CHC_BASE
-      - description: DDRPHY_NAO_CHD_BASE
-      - description: SLEEP_BASE
-
-  support-ch-cnt:
-    maxItems: 1
-
-  fmeter-version:
-    maxItems: 1
-    description:
-      Fmeter version for calculating dram data rate
-
-  crystal-freq:
-    maxItems: 1
-    description:
-      Reference clock rate in MHz
-
-  shu-of:
-    maxItems: 1
-
-  pll-id: true
-  shu-lv: true
-  sdmpcw: true
-  posdiv: true
-  fbksel: true
-  dqsopen: true
-  async-ca: true
-  dq-ser-mode: true
-
-required:
-  - compatible
-  - reg
-  - support-ch-cnt
-  - fmeter-version
-  - crystal-freq
-  - pll-id
-  - shu-lv
-  - shu-of
-  - sdmpcw
-  - posdiv
-  - fbksel
-  - dqsopen
-  - async-ca
-  - dq-ser-mode
-
-additionalProperties: false
-
-examples:
-  - |
-    soc {
-        #address-cells = <2>;
-        #size-cells = <2>;
-
-        dramc: dramc@10230000 {
-            compatible = "mediatek,common-dramc";
-            reg = <0 0x10230000 0 0x2000>, /* DRAMC_AO_CHA_BASE */
-                <0 0x10240000 0 0x2000>, /* DRAMC_AO_CHB_BASE */
-                <0 0x10250000 0 0x2000>, /* DRAMC_AO_CHC_BASE */
-                <0 0x10260000 0 0x2000>, /* DRAMC_AO_CHD_BASE */
-                <0 0x10234000 0 0x1000>, /* DRAMC_NAO_CHA_BASE */
-                <0 0x10244000 0 0x1000>, /* DRAMC_NAO_CHB_BASE */
-                <0 0x10254000 0 0x1000>, /* DRAMC_NAO_CHC_BASE */
-                <0 0x10264000 0 0x1000>, /* DRAMC_NAO_CHD_BASE */
-                <0 0x10238000 0 0x2000>, /* DDRPHY_AO_CHA_BASE */
-                <0 0x10248000 0 0x2000>, /* DDRPHY_AO_CHB_BASE */
-                <0 0x10258000 0 0x2000>, /* DDRPHY_AO_CHC_BASE */
-                <0 0x10268000 0 0x2000>, /* DDRPHY_AO_CHD_BASE */
-                <0 0x10236000 0 0x2000>, /* DDRPHY_NAO_CHA_BASE */
-                <0 0x10246000 0 0x2000>, /* DDRPHY_NAO_CHB_BASE */
-                <0 0x10256000 0 0x2000>, /* DDRPHY_NAO_CHC_BASE */
-                <0 0x10266000 0 0x2000>, /* DDRPHY_NAO_CHD_BASE */
-                <0 0x10006000 0 0x1000>; /* SLEEP_BASE */
-            support-ch-cnt = <4>;
-            fmeter-version = <1>;
-            crystal-freq = <26>;
-            pll-id = <0x0e98 0x02000000 25>;
-            shu-lv = <0x0e98 0x0000c000 14>;
-            shu-of = <0x700>;
-            sdmpcw = <0x0908 0x0007fff8 3>,
-                <0x0928 0x0007fff8 3>;
-            posdiv = <0x090c 0x00003800 11>,
-                <0x092c 0x00003800 11>;
-            fbksel = <0x0910 0x00000040 6>,
-                <0x0910 0x00000040 6>;
-            dqsopen = <0x0d94 0x04000000 26>,
-                <0x0d94 0x04000000 26>;
-            async-ca = <0x0d08 0x00000001 0>,
-                <0x0d08 0x00000001 0>;
-            dq-ser-mode = <0x0dc4 0x00000018 3>,
-                <0x0dc4 0x00000018 3>;
-        };
-    };
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml
new file mode 100644
index 000000000000..8bdacfc36cb5
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2025 MediaTek Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/mediatek,dramc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek DRAM Controller (DRAMC)
+
+maintainers:
+  - Crystal Guo <crystal.guo@mediatek.com>
+
+description:
+  A MediaTek DRAM controller interface to provide the current data rate of DRAM.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mediatek,mt8196-dramc
+
+  reg:
+    items:
+      - description: anaphy registers
+      - description: ddrphy registers
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        memory-controller@10236000 {
+            compatible = "mediatek,mt8196-dramc";
+            reg = <0 0x10236000 0 0x2000>,
+                  <0 0x10238000 0 0x2000>;
+        };
+    };
-- 
2.18.0

[v2,0/2] Add an interface to get current DDR data rate

[Crystal Guo]

*** BLURB HERE ***
This series is based on linux-next, tag: next-20250206.

Vcore DVFS feature need know the current DDR data rate.
Add MediaTek DRAMC driver to provide an interface that can
obtain current DDR data rate.

---
Changes in v2:
- Remove pr_info and pr_err, use dev_err or dev_err_probe to print
  error message;
- Replace module_init by module_platform_driver;
- Remove unnecessary global variables;
- Change fmeter-verison to platform data;
- Remove mtk-dramc.h;
- Refine compatible to "mediatek,mt8196-dramc";
- Refine CONFIG name to MEDIATEK_MC;
- Fix yaml build errors, remove unnecessary properties on yaml file.

Link to v1:
https://patchwork.kernel.org/patch/13904862

Crystal Guo (2):
  memory/mediatek: Add an interface to get current DDR data rate
  dt-bindings: memory-controllers: Add MediaTek DRAM controller
    interface

 .../memory-controllers/mediatek,dramc.yaml    |  44 ++++
 drivers/memory/Kconfig                        |   1 +
 drivers/memory/Makefile                       |   1 +
 drivers/memory/mediatek/Kconfig               |  21 ++
 drivers/memory/mediatek/Makefile              |   2 +
 drivers/memory/mediatek/mtk-dramc.c           | 196 ++++++++++++++++++
 6 files changed, 265 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml
 create mode 100644 drivers/memory/mediatek/Kconfig
 create mode 100644 drivers/memory/mediatek/Makefile
 create mode 100644 drivers/memory/mediatek/mtk-dramc.c

-- 
2.18.0

[v2,1/2] memory/mediatek: Add an interface to get current DDR data rate

[Crystal Guo]

Add MediaTek DRAMC driver to provide an interface that can
obtain current DDR data rate.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202412210955.FvO0Pee3-lkp@intel.
com/
Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
---
 drivers/memory/Kconfig              |   1 +
 drivers/memory/Makefile             |   1 +
 drivers/memory/mediatek/Kconfig     |  21 +++
 drivers/memory/mediatek/Makefile    |   2 +
 drivers/memory/mediatek/mtk-dramc.c | 196 ++++++++++++++++++++++++++++
 5 files changed, 221 insertions(+)
 create mode 100644 drivers/memory/mediatek/Kconfig
 create mode 100644 drivers/memory/mediatek/Makefile
 create mode 100644 drivers/memory/mediatek/mtk-dramc.c

diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index c82d8d8a16ea..b1698549ff81 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -227,5 +227,6 @@ config STM32_FMC2_EBI
 
 source "drivers/memory/samsung/Kconfig"
 source "drivers/memory/tegra/Kconfig"
+source "drivers/memory/mediatek/Kconfig"
 
 endif
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index d2e6ca9abbe0..c0facf529803 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_STM32_FMC2_EBI)	+= stm32-fmc2-ebi.o
 
 obj-$(CONFIG_SAMSUNG_MC)	+= samsung/
 obj-$(CONFIG_TEGRA_MC)		+= tegra/
+obj-$(CONFIG_MEDIATEK_MC)	+= mediatek/
 obj-$(CONFIG_TI_EMIF_SRAM)	+= ti-emif-sram.o
 obj-$(CONFIG_FPGA_DFL_EMIF)	+= dfl-emif.o
 
diff --git a/drivers/memory/mediatek/Kconfig b/drivers/memory/mediatek/Kconfig
new file mode 100644
index 000000000000..3f238e0d9647
--- /dev/null
+++ b/drivers/memory/mediatek/Kconfig
@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config MEDIATEK_MC
+	bool "MediaTek Memory Controller support"
+	help
+	  This option allows to enable MediaTek memory controller drivers,
+	  which may include controllers for DRAM or others.
+	  Select Y here if you need support for MediaTek memory controller.
+	  If you don't need, select N.
+
+if MEDIATEK_MC
+
+config MTK_DRAMC
+	tristate "MediaTek DRAMC driver"
+	default y
+	help
+	  This option selects the MediaTek DRAMC driver, which provides
+	  an interface for reporting the current data rate of DRAM.
+	  Select Y here if you need support for the MediaTek DRAMC driver.
+	  If you don't need, select N.
+
+endif
diff --git a/drivers/memory/mediatek/Makefile b/drivers/memory/mediatek/Makefile
new file mode 100644
index 000000000000..a1395fc55b41
--- /dev/null
+++ b/drivers/memory/mediatek/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_MTK_DRAMC)		+= mtk-dramc.o
diff --git a/drivers/memory/mediatek/mtk-dramc.c b/drivers/memory/mediatek/mtk-dramc.c
new file mode 100644
index 000000000000..d452483a98ce
--- /dev/null
+++ b/drivers/memory/mediatek/mtk-dramc.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 MediaTek Inc.
+ */
+#include <linux/bitops.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+
+#define POSDIV_PURIFY	BIT(2)
+#define PREDIV		7
+#define REF_FREQUENCY	26
+#define SHUFFLE_OFFSET	0x700
+
+/*--------------------------------------------------------------------------*/
+/* Register Offset                                                          */
+/*--------------------------------------------------------------------------*/
+#define DPHY_DVFS_STA		0x0e98
+#define APHY_PHYPLL2		0x0908
+#define APHY_CLRPLL2		0x0928
+#define APHY_PHYPLL3		0x090c
+#define APHY_CLRPLL3		0x092c
+#define APHY_PHYPLL4		0x0910
+#define APHY_ARPI0		0x0d94
+#define APHY_CA_ARDLL1		0x0d08
+#define APHY_B0_TX0		0x0dc4
+
+/*--------------------------------------------------------------------------*/
+/* Register Mask                                                            */
+/*--------------------------------------------------------------------------*/
+#define DPHY_DVFS_SHU_LV	GENMASK(15, 14)
+#define DPHY_DVFS_PLL_SEL	GENMASK(25, 25)
+#define APHY_PLL2_SDMPCW	GENMASK(18, 3)
+#define APHY_PLL3_POSDIV	GENMASK(13, 11)
+#define APHY_PLL4_FBKSEL	GENMASK(6, 6)
+#define APHY_ARPI0_SOPEN	GENMASK(26, 26)
+#define APHY_ARDLL1_CK_EN	GENMASK(0, 0)
+#define APHY_B0_TX0_SER_MODE	GENMASK(4, 3)
+
+static unsigned int read_reg_field(void __iomem *base, unsigned int offset, unsigned int mask)
+{
+	unsigned int val = readl(base + offset);
+	unsigned int shift = __ffs(mask);
+
+	return (val & mask) >> shift;
+}
+
+struct mtk_dramc_pdata {
+	unsigned int fmeter_version;
+};
+
+struct mtk_dramc_dev_t {
+	void __iomem *anaphy_base;
+	void __iomem *ddrphy_base;
+	const struct mtk_dramc_pdata *pdata;
+};
+
+static int mtk_dramc_probe(struct platform_device *pdev)
+{
+	struct mtk_dramc_dev_t *dramc;
+	const struct mtk_dramc_pdata *pdata;
+	int ret;
+
+	dramc = devm_kzalloc(&pdev->dev, sizeof(struct mtk_dramc_dev_t), GFP_KERNEL);
+	if (!dramc)
+		return dev_err_probe(&pdev->dev, -ENOMEM, "Failed to allocate memory\n");
+
+	pdata = of_device_get_match_data(&pdev->dev);
+	if (!pdata)
+		return dev_err_probe(&pdev->dev, -EINVAL, "No platform data available\n");
+
+	dramc->pdata = pdata;
+
+	dramc->anaphy_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(dramc->anaphy_base)) {
+		ret = PTR_ERR(dramc->anaphy_base);
+		return dev_err_probe(&pdev->dev, ret, "Unable to map DDRPHY NAO base\n");
+	}
+
+	dramc->ddrphy_base = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(dramc->ddrphy_base)) {
+		ret = PTR_ERR(dramc->ddrphy_base);
+		return dev_err_probe(&pdev->dev, ret, "Unable to map DDRPHY AO base\n");
+	}
+
+	platform_set_drvdata(pdev, dramc);
+	return 0;
+}
+
+static unsigned int mtk_fmeter_v1(struct mtk_dramc_dev_t *dramc)
+{
+	unsigned int shu_level, pll_sel, offset;
+	unsigned int sdmpcw, posdiv, ckdiv4, fbksel, sopen, async_ca, ser_mode;
+	unsigned int perdiv_freq, posdiv_freq, vco_freq;
+	unsigned int final_rate;
+
+	shu_level = read_reg_field(dramc->ddrphy_base, DPHY_DVFS_STA, DPHY_DVFS_SHU_LV);
+	pll_sel = read_reg_field(dramc->ddrphy_base, DPHY_DVFS_STA, DPHY_DVFS_PLL_SEL);
+	offset = SHUFFLE_OFFSET * shu_level;
+
+	sdmpcw = read_reg_field(dramc->anaphy_base,
+				((pll_sel == 0) ? APHY_PHYPLL2 : APHY_CLRPLL2) + offset,
+				APHY_PLL2_SDMPCW);
+	posdiv = read_reg_field(dramc->anaphy_base,
+				((pll_sel == 0) ? APHY_PHYPLL3 : APHY_CLRPLL3) + offset,
+				APHY_PLL3_POSDIV);
+	fbksel = read_reg_field(dramc->anaphy_base, APHY_PHYPLL4 + offset, APHY_PLL4_FBKSEL);
+	sopen = read_reg_field(dramc->anaphy_base, APHY_ARPI0 + offset, APHY_ARPI0_SOPEN);
+	async_ca = read_reg_field(dramc->anaphy_base, APHY_CA_ARDLL1 + offset, APHY_ARDLL1_CK_EN);
+	ser_mode = read_reg_field(dramc->anaphy_base, APHY_B0_TX0 + offset, APHY_B0_TX0_SER_MODE);
+
+	ckdiv4 = (ser_mode == 1) ? 1 : 0;
+	posdiv &= ~(POSDIV_PURIFY);
+
+	perdiv_freq = REF_FREQUENCY * (sdmpcw >> PREDIV);
+	posdiv_freq = (perdiv_freq >> posdiv) >> 1;
+	vco_freq = posdiv_freq << fbksel;
+	final_rate = vco_freq >> ckdiv4;
+
+	if (sopen == 1 && async_ca == 1)
+		final_rate >>= 1;
+
+	return final_rate;
+}
+
+/*
+ * mtk_dramc_get_data_rate - calculate DRAM data rate
+ *
+ * Returns DRAM data rate (MB/s)
+ */
+static unsigned int mtk_dramc_get_data_rate(struct device *dev)
+{
+	struct mtk_dramc_dev_t *dramc_dev = dev_get_drvdata(dev);
+
+	if (!dramc_dev) {
+		dev_err(dev, "DRAMC device data not found\n");
+		return -EINVAL;
+	}
+
+	if (dramc_dev->pdata) {
+		if (dramc_dev->pdata->fmeter_version == 1)
+			return mtk_fmeter_v1(dramc_dev);
+
+		dev_err(dev, "Unsupported fmeter version\n");
+		return -EINVAL;
+	}
+	dev_err(dev, "DRAMC platform data not found\n");
+	return -EINVAL;
+}
+
+static ssize_t dram_data_rate_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "DRAM data rate = %u\n",
+			mtk_dramc_get_data_rate(dev));
+}
+
+static DEVICE_ATTR_RO(dram_data_rate);
+
+static struct attribute *mtk_dramc_attrs[] = {
+	&dev_attr_dram_data_rate.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(mtk_dramc);
+
+static const struct mtk_dramc_pdata dramc_pdata_mt8196 = {
+	.fmeter_version = 1
+};
+
+static const struct of_device_id mtk_dramc_of_ids[] = {
+	{ .compatible = "mediatek,mt8196-dramc", .data = &dramc_pdata_mt8196 },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mtk_dramc_of_ids);
+
+static struct platform_driver mtk_dramc_driver = {
+	.probe = mtk_dramc_probe,
+	.driver = {
+		.name = "mtk_dramc_drv",
+		.of_match_table = mtk_dramc_of_ids,
+		.dev_groups = mtk_dramc_groups,
+	},
+};
+
+module_platform_driver(mtk_dramc_driver);
+
+MODULE_AUTHOR("Crystal Guo <crystal.guo@mediatek.com>");
+MODULE_DESCRIPTION("MediaTek DRAM Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.18.0

[v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[Crystal Guo]

A MediaTek DRAM controller interface to provide the current DDR data rate.

Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
---
 .../memory-controllers/mediatek,dramc.yaml    | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml
new file mode 100644
index 000000000000..8bdacfc36cb5
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,dramc.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2025 MediaTek Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/mediatek,dramc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek DRAM Controller (DRAMC)
+
+maintainers:
+  - Crystal Guo <crystal.guo@mediatek.com>
+
+description:
+  A MediaTek DRAM controller interface to provide the current data rate of DRAM.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mediatek,mt8196-dramc
+
+  reg:
+    items:
+      - description: anaphy registers
+      - description: ddrphy registers
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        memory-controller@10236000 {
+            compatible = "mediatek,mt8196-dramc";
+            reg = <0 0x10236000 0 0x2000>,
+                  <0 0x10238000 0 0x2000>;
+        };
+    };
-- 
2.18.0

Re: [v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[Krzysztof Kozlowski]

On 06/02/2025 13:16, Crystal Guo wrote:
> A MediaTek DRAM controller interface to provide the current DDR data rate.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597


> 
> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> ---
>  .../mediatek,common-dramc.yaml                | 129 ------------------

Why is there removal here if you add? Commit msg explains nothing here.

Why this patch was sent twice?

Please use standard email subjects, so with the PATCH keyword in the
title. `git format-patch -vX` helps here to create proper versioned
patches. Another useful tool is b4. Skipping the PATCH keyword makes
filtering of emails more difficult thus making the review process less
convenient.

Best regards,
Krzysztof

Re: [v2,1/2] memory/mediatek: Add an interface to get current DDR data rate

[Krzysztof Kozlowski]

On 07/02/2025 02:42, Crystal Guo wrote:
> Add MediaTek DRAMC driver to provide an interface that can
> obtain current DDR data rate.
> 
> Reported-by: kernel test robot <lkp@intel.com>

Kbuild did not report here anything.

> Closes: https://lore.kernel.org/oe-kbuild-all/202412210955.FvO0Pee3-lkp@intel.
> com/
> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> ---
>  drivers/memory/Kconfig              |   1 +
>  drivers/memory/Makefile             |   1 +
>  drivers/memory/mediatek/Kconfig     |  21 +++
>  drivers/memory/mediatek/Makefile    |   2 +
>  drivers/memory/mediatek/mtk-dramc.c | 196 ++++++++++++++++++++++++++++
>  5 files changed, 221 insertions(+)
>  create mode 100644 drivers/memory/mediatek/Kconfig
>  create mode 100644 drivers/memory/mediatek/Makefile
>  create mode 100644 drivers/memory/mediatek/mtk-dramc.c


You sent same patchsets multiple times.

Before next posting, please really carefully go through the guides and
process.

Best regards,
Krzysztof

Re: [v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[Krzysztof Kozlowski]

On 07/02/2025 02:42, Crystal Guo wrote:
> A MediaTek DRAM controller interface to provide the current DDR data rate.
> 
Duplicated patch.

You also ignored several comments so that's a NAK.

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

Best regards,
Krzysztof

Re: [v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[Crystal Guo (郭晶)]

On Sun, 2025-02-09 at 11: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/02/2025 13:16, Crystal Guo wrote:
> > A MediaTek DRAM controller interface to provide the current DDR
> > data rate.
> 
> Please wrap commit message according to Linux coding style /
> submission
> process (neither too early nor over the limit):
> 
https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst*L597__;Iw!!CTRNKA9wMg0ARbw!lrJR1xYY1zrw1dRnvzwOsd0FyqQawTURFAKVFpWEaMLN2eiV03gQy8WDacH-ZGIypB4-WuPupcOERlw9$
> 
> 
> > 
> > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> > ---
> >  .../mediatek,common-dramc.yaml                | 129 --------------
> > ----
> 
> Why is there removal here if you add? Commit msg explains nothing
> here.
> 
> Why this patch was sent twice?
> 
> Please use standard email subjects, so with the PATCH keyword in the
> title. `git format-patch -vX` helps here to create proper versioned
> patches. Another useful tool is b4. Skipping the PATCH keyword makes
> filtering of emails more difficult thus making the review process
> less
> convenient.
> 
> Best regards,
> Krzysztof

This patch was based on the previous version.
But the approach was actually incorrect (shall NOT be patches on the
top of the previous version), so I have resent the new v2 patches based
on a clean kernel.

Thanks
Crystal

Re: [v2,1/2] memory/mediatek: Add an interface to get current DDR data rate

[Crystal Guo (郭晶)]

On Sun, 2025-02-09 at 11:50 +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/02/2025 02:42, Crystal Guo wrote:
> > Add MediaTek DRAMC driver to provide an interface that can
> > obtain current DDR data rate.
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> Kbuild did not report here anything.

Okay, I will remove this tag in the next version.

> 
> > Closes: 
> > https://lore.kernel.org/oe-kbuild-all/202412210955.FvO0Pee3-lkp@intel
> > .
> > com/
> > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> > ---
> >  drivers/memory/Kconfig              |   1 +
> >  drivers/memory/Makefile             |   1 +
> >  drivers/memory/mediatek/Kconfig     |  21 +++
> >  drivers/memory/mediatek/Makefile    |   2 +
> >  drivers/memory/mediatek/mtk-dramc.c | 196
> > ++++++++++++++++++++++++++++
> >  5 files changed, 221 insertions(+)
> >  create mode 100644 drivers/memory/mediatek/Kconfig
> >  create mode 100644 drivers/memory/mediatek/Makefile
> >  create mode 100644 drivers/memory/mediatek/mtk-dramc.c
> 
> 
> You sent same patchsets multiple times.
> 
> Before next posting, please really carefully go through the guides
> and
> process.
> 
> Best regards,
> Krzysztof

Okay, sorry for the inconvenience caused.

Best regards,
Crystal

Re: [v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[Crystal Guo (郭晶)]

On Sun, 2025-02-09 at 11:51 +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/02/2025 02:42, Crystal Guo wrote:
> > A MediaTek DRAM controller interface to provide the current DDR
> > data rate.

Hi Krzysztof,

Regarding this commit message, you previously commented as follows: 
"Please wrap commit message according to Linux coding style /     
submission process (neither too early nor over the limit): 
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L5
"

May I ask if this means that the description "A MediaTek DRAM
controller interface to provide the current DDR data rate." needs to be
split into two lines? The total number of characters in this
description is 74.

Thanks
Crystal

> > 
> 
> Duplicated patch.
> 
> You also ignored several comments so that's a NAK.

Apologies for any confusion.

This latest patch and 
https://patchwork.kernel.org/project/linux-mediatek/list/?series=931186
 do indeed overlap. 
Due to the fact that 
https://patchwork.kernel.org/project/linux-mediatek/list/?series=931186
 was pushed based on an incorrect base, please ignore the patch series
931186. 

This latest patch is intended to address the comments provided for the
first version in 
https://patchwork.kernel.org/project/linux-mediatek/list/?series=917099
. It is based on a clean kernel, so could you help review this version?
Thanks.

Best regards
Crystal


> 
> <form letter>
> This is a friendly reminder during the review process.
> 
> It seems my or other reviewer's previous comments were not fully
> addressed. Maybe the feedback got lost between the quotes, maybe you
> just forgot to apply it. Please go back to the previous discussion
> and
> either implement all requested changes or keep discussing them.
> 
> Thank you.
> </form letter>
> 
> Best regards,
> Krzysztof

Re: [v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[Krzysztof Kozlowski]

On 12/02/2025 04:50, Crystal Guo (郭晶) wrote:
> On Sun, 2025-02-09 at 11:51 +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/02/2025 02:42, Crystal Guo wrote:
>>> A MediaTek DRAM controller interface to provide the current DDR
>>> data rate.
> 
> Hi Krzysztof,
> 
> Regarding this commit message, you previously commented as follows: 
> "Please wrap commit message according to Linux coding style /     
> submission process (neither too early nor over the limit): 
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L5
> "
> 
> May I ask if this means that the description "A MediaTek DRAM
> controller interface to provide the current DDR data rate." needs to be
> split into two lines? The total number of characters in this
> description is 74.

And what is the wrapping limit?


Best regards,
Krzysztof

Re: [v2,2/2] dt-bindings: memory-controllers: Add MediaTek DRAM controller interface

[Crystal Guo (郭晶)]

On Wed, 2025-02-12 at 06:34 +0100, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 12/02/2025 04:50, Crystal Guo (郭晶) wrote:
> > On Sun, 2025-02-09 at 11:51 +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/02/2025 02:42, Crystal Guo wrote:
> > > > A MediaTek DRAM controller interface to provide the current DDR
> > > > data rate.
> > 
> > Hi Krzysztof,
> > 
> > Regarding this commit message, you previously commented as follows:
> > "Please wrap commit message according to Linux coding style /
> > submission process (neither too early nor over the limit):
> > 
https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst*L5__;Iw!!CTRNKA9wMg0ARbw!jY0i3mRY0mA3yLhvxm42NZiFnheY6L5Q9dKVJkv911eQ9Z03SBi3-Mgslg_fOzkwAfkJJODcjD6Ka5_I$
> > "
> > 
> > May I ask if this means that the description "A MediaTek DRAM
> > controller interface to provide the current DDR data rate." needs
> > to be
> > split into two lines? The total number of characters in this
> > description is 74.
> 
> And what is the wrapping limit?
> 
> 
> Best regards,
> Krzysztof

The wrapping limit for the commit message body is 75 columns.
I will update the commit message body in next version:
A MediaTek DRAM controller interface to provide the current DDR
data rate.
Is this Okay?

Best regards,
Crystal

Re: [v2,1/2] memory/mediatek: Add an interface to get current DDR data rate

[AngeloGioacchino Del Regno]

Il 07/02/25 02:42, Crystal Guo ha scritto:
> Add MediaTek DRAMC driver to provide an interface that can
> obtain current DDR data rate.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202412210955.FvO0Pee3-lkp@intel.
> com/
> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> ---
>   drivers/memory/Kconfig              |   1 +
>   drivers/memory/Makefile             |   1 +
>   drivers/memory/mediatek/Kconfig     |  21 +++
>   drivers/memory/mediatek/Makefile    |   2 +
>   drivers/memory/mediatek/mtk-dramc.c | 196 ++++++++++++++++++++++++++++
>   5 files changed, 221 insertions(+)
>   create mode 100644 drivers/memory/mediatek/Kconfig
>   create mode 100644 drivers/memory/mediatek/Makefile
>   create mode 100644 drivers/memory/mediatek/mtk-dramc.c
> 
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index c82d8d8a16ea..b1698549ff81 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -227,5 +227,6 @@ config STM32_FMC2_EBI
>   
>   source "drivers/memory/samsung/Kconfig"
>   source "drivers/memory/tegra/Kconfig"
> +source "drivers/memory/mediatek/Kconfig"
>   
>   endif
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index d2e6ca9abbe0..c0facf529803 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_STM32_FMC2_EBI)	+= stm32-fmc2-ebi.o
>   
>   obj-$(CONFIG_SAMSUNG_MC)	+= samsung/
>   obj-$(CONFIG_TEGRA_MC)		+= tegra/
> +obj-$(CONFIG_MEDIATEK_MC)	+= mediatek/
>   obj-$(CONFIG_TI_EMIF_SRAM)	+= ti-emif-sram.o
>   obj-$(CONFIG_FPGA_DFL_EMIF)	+= dfl-emif.o
>   
> diff --git a/drivers/memory/mediatek/Kconfig b/drivers/memory/mediatek/Kconfig
> new file mode 100644
> index 000000000000..3f238e0d9647
> --- /dev/null
> +++ b/drivers/memory/mediatek/Kconfig
> @@ -0,0 +1,21 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config MEDIATEK_MC
> +	bool "MediaTek Memory Controller support"
> +	help
> +	  This option allows to enable MediaTek memory controller drivers,
> +	  which may include controllers for DRAM or others.
> +	  Select Y here if you need support for MediaTek memory controller.
> +	  If you don't need, select N.
> +
> +if MEDIATEK_MC
> +
> +config MTK_DRAMC
> +	tristate "MediaTek DRAMC driver"
> +	default y
> +	help
> +	  This option selects the MediaTek DRAMC driver, which provides
> +	  an interface for reporting the current data rate of DRAM.
> +	  Select Y here if you need support for the MediaTek DRAMC driver.
> +	  If you don't need, select N.
> +
> +endif
> diff --git a/drivers/memory/mediatek/Makefile b/drivers/memory/mediatek/Makefile
> new file mode 100644
> index 000000000000..a1395fc55b41
> --- /dev/null
> +++ b/drivers/memory/mediatek/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_MTK_DRAMC)		+= mtk-dramc.o
> diff --git a/drivers/memory/mediatek/mtk-dramc.c b/drivers/memory/mediatek/mtk-dramc.c
> new file mode 100644
> index 000000000000..d452483a98ce
> --- /dev/null
> +++ b/drivers/memory/mediatek/mtk-dramc.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 MediaTek Inc.
> + */
> +#include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +
> +#define POSDIV_PURIFY	BIT(2)
> +#define PREDIV		7
> +#define REF_FREQUENCY	26

REF_FREQUENCY should be platform data, not a definition, as this is
different in other SoCs.

> +#define SHUFFLE_OFFSET	0x700
> +
> +/*--------------------------------------------------------------------------*/
> +/* Register Offset                                                          */
> +/*--------------------------------------------------------------------------*/
> +#define DPHY_DVFS_STA		0x0e98
> +#define APHY_PHYPLL2		0x0908
> +#define APHY_CLRPLL2		0x0928
> +#define APHY_PHYPLL3		0x090c
> +#define APHY_CLRPLL3		0x092c
> +#define APHY_PHYPLL4		0x0910
> +#define APHY_ARPI0		0x0d94
> +#define APHY_CA_ARDLL1		0x0d08
> +#define APHY_B0_TX0		0x0dc4

Aren't those also SoC-dependant?

enum mtk_dramc_reg_index {
	DRAMC_DPHY_DVFS_STA,
	DRAMC_APHY_SHU_PHYPLL2,
	....etc
}

static const u16 mtk_dramc_regs_mt8195[] = {
	[DRAMC_DPHY_DVFS_STA] = 0x50c, /* ddrphy_config_NAO */
	[DRAMC_APHY_SHU_PHYPLL2] = 0x704,
	[DRAMC_APHY_SHU_CLRPLL2] = 0x724,
	[DRAMC_APHY_SHU_PHYPLL3] = 0x708,
	[DRAMC_APHY_SHU_CLRPLL3] = 0x728,
	[DRAMC_APHY_SHU_PHYPLL4] = 0x70c,
	...etc
}

static const u16 mtk_dramc_regs_mt8196[] = {
	[DRAMC_DPHY_DVFS_STA] = 0xe98, /* ddrphy_config_misc */
	[DRAMC_APHY_SHU_PHYPLL2] = 0x908,
	[DRAMC_APHY_SHU_CLRPLL2] = 0x928,
	.... etc
}

> +
> +/*--------------------------------------------------------------------------*/
> +/* Register Mask                                                            */
> +/*--------------------------------------------------------------------------*/
> +#define DPHY_DVFS_SHU_LV	GENMASK(15, 14)
> +#define DPHY_DVFS_PLL_SEL	GENMASK(25, 25)
> +#define APHY_PLL2_SDMPCW	GENMASK(18, 3)
> +#define APHY_PLL3_POSDIV	GENMASK(13, 11)
> +#define APHY_PLL4_FBKSEL	GENMASK(6, 6)
> +#define APHY_ARPI0_SOPEN	GENMASK(26, 26)
> +#define APHY_ARDLL1_CK_EN	GENMASK(0, 0)
> +#define APHY_B0_TX0_SER_MODE	GENMASK(4, 3)

And those masks should also be platform data, as those are also different
in other SoCs.

> +
> +static unsigned int read_reg_field(void __iomem *base, unsigned int offset, unsigned int mask)
> +{
> +	unsigned int val = readl(base + offset);
> +	unsigned int shift = __ffs(mask);
> +
> +	return (val & mask) >> shift;
> +}
> +
> +struct mtk_dramc_pdata {
> +	unsigned int fmeter_version;

u8 fmeter_version;
u8 ref_freq_mhz;
const u16 *regs;

...etc etc


> +};
> +
> +struct mtk_dramc_dev_t {
> +	void __iomem *anaphy_base;
> +	void __iomem *ddrphy_base;
> +	const struct mtk_dramc_pdata *pdata;
> +};
> +
> +static int mtk_dramc_probe(struct platform_device *pdev)
> +{
> +	struct mtk_dramc_dev_t *dramc;
> +	const struct mtk_dramc_pdata *pdata;
> +	int ret;
> +
> +	dramc = devm_kzalloc(&pdev->dev, sizeof(struct mtk_dramc_dev_t), GFP_KERNEL);
> +	if (!dramc)
> +		return dev_err_probe(&pdev->dev, -ENOMEM, "Failed to allocate memory\n");
> +
> +	pdata = of_device_get_match_data(&pdev->dev);
> +	if (!pdata)
> +		return dev_err_probe(&pdev->dev, -EINVAL, "No platform data available\n");
> +
> +	dramc->pdata = pdata;
> +
> +	dramc->anaphy_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(dramc->anaphy_base)) {
> +		ret = PTR_ERR(dramc->anaphy_base);
> +		return dev_err_probe(&pdev->dev, ret, "Unable to map DDRPHY NAO base\n");
> +	}
> +
> +	dramc->ddrphy_base = devm_platform_ioremap_resource(pdev, 1);
> +	if (IS_ERR(dramc->ddrphy_base)) {
> +		ret = PTR_ERR(dramc->ddrphy_base);
> +		return dev_err_probe(&pdev->dev, ret, "Unable to map DDRPHY AO base\n");
> +	}
> +
> +	platform_set_drvdata(pdev, dramc);
> +	return 0;
> +}
> +
> +static unsigned int mtk_fmeter_v1(struct mtk_dramc_dev_t *dramc)
> +{
> +	unsigned int shu_level, pll_sel, offset;
> +	unsigned int sdmpcw, posdiv, ckdiv4, fbksel, sopen, async_ca, ser_mode;
> +	unsigned int perdiv_freq, posdiv_freq, vco_freq;
> +	unsigned int final_rate;
> +
> +	shu_level = read_reg_field(dramc->ddrphy_base, DPHY_DVFS_STA, DPHY_DVFS_SHU_LV);
> +	pll_sel = read_reg_field(dramc->ddrphy_base, DPHY_DVFS_STA, DPHY_DVFS_PLL_SEL);
> +	offset = SHUFFLE_OFFSET * shu_level;
> +
> +	sdmpcw = read_reg_field(dramc->anaphy_base,
> +				((pll_sel == 0) ? APHY_PHYPLL2 : APHY_CLRPLL2) + offset,
> +				APHY_PLL2_SDMPCW);
> +	posdiv = read_reg_field(dramc->anaphy_base,
> +				((pll_sel == 0) ? APHY_PHYPLL3 : APHY_CLRPLL3) + offset,
> +				APHY_PLL3_POSDIV);
> +	fbksel = read_reg_field(dramc->anaphy_base, APHY_PHYPLL4 + offset, APHY_PLL4_FBKSEL);
> +	sopen = read_reg_field(dramc->anaphy_base, APHY_ARPI0 + offset, APHY_ARPI0_SOPEN);
> +	async_ca = read_reg_field(dramc->anaphy_base, APHY_CA_ARDLL1 + offset, APHY_ARDLL1_CK_EN);
> +	ser_mode = read_reg_field(dramc->anaphy_base, APHY_B0_TX0 + offset, APHY_B0_TX0_SER_MODE);
> +
> +	ckdiv4 = (ser_mode == 1) ? 1 : 0;
> +	posdiv &= ~(POSDIV_PURIFY);
> +
> +	perdiv_freq = REF_FREQUENCY * (sdmpcw >> PREDIV);

s/perdiv/prediv/g

> +	posdiv_freq = (perdiv_freq >> posdiv) >> 1;
> +	vco_freq = posdiv_freq << fbksel;
> +	final_rate = vco_freq >> ckdiv4;
> +

there's also one case in which `final_rate >>= 2`... please check if it is
applicable to this driver (it should).

Cheers,
Angelo

> +	if (sopen == 1 && async_ca == 1)
> +		final_rate >>= 1;
> +
> +	return final_rate;
> +}
> +
> +/*
> + * mtk_dramc_get_data_rate - calculate DRAM data rate
> + *
> + * Returns DRAM data rate (MB/s)
> + */
> +static unsigned int mtk_dramc_get_data_rate(struct device *dev)
> +{
> +	struct mtk_dramc_dev_t *dramc_dev = dev_get_drvdata(dev);
> +
> +	if (!dramc_dev) {
> +		dev_err(dev, "DRAMC device data not found\n");
> +		return -EINVAL;
> +	}
> +
> +	if (dramc_dev->pdata) {
> +		if (dramc_dev->pdata->fmeter_version == 1)
> +			return mtk_fmeter_v1(dramc_dev);
> +
> +		dev_err(dev, "Unsupported fmeter version\n");
> +		return -EINVAL;
> +	}
> +	dev_err(dev, "DRAMC platform data not found\n");
> +	return -EINVAL;
> +}
> +
> +static ssize_t dram_data_rate_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "DRAM data rate = %u\n",
> +			mtk_dramc_get_data_rate(dev));
> +}
> +
> +static DEVICE_ATTR_RO(dram_data_rate);
> +
> +static struct attribute *mtk_dramc_attrs[] = {
> +	&dev_attr_dram_data_rate.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(mtk_dramc);
> +
> +static const struct mtk_dramc_pdata dramc_pdata_mt8196 = {
> +	.fmeter_version = 1
> +};
> +
> +static const struct of_device_id mtk_dramc_of_ids[] = {
> +	{ .compatible = "mediatek,mt8196-dramc", .data = &dramc_pdata_mt8196 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mtk_dramc_of_ids);
> +
> +static struct platform_driver mtk_dramc_driver = {
> +	.probe = mtk_dramc_probe,
> +	.driver = {
> +		.name = "mtk_dramc_drv",
> +		.of_match_table = mtk_dramc_of_ids,
> +		.dev_groups = mtk_dramc_groups,
> +	},
> +};
> +
> +module_platform_driver(mtk_dramc_driver);
> +
> +MODULE_AUTHOR("Crystal Guo <crystal.guo@mediatek.com>");
> +MODULE_DESCRIPTION("MediaTek DRAM Controller Driver");
> +MODULE_LICENSE("GPL");

Re: [v2,1/2] memory/mediatek: Add an interface to get current DDR data rate

[Crystal Guo (郭晶)]

On Tue, 2025-03-04 at 15:59 +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 07/02/25 02:42, Crystal Guo ha scritto:
> > Add MediaTek DRAMC driver to provide an interface that can
> > obtain current DDR data rate.
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: 
> > https://lore.kernel.org/oe-kbuild-all/202412210955.FvO0Pee3-lkp@intel
> > .
> > com/
> > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> > ---
> >   drivers/memory/Kconfig              |   1 +
> >   drivers/memory/Makefile             |   1 +
> >   drivers/memory/mediatek/Kconfig     |  21 +++
> >   drivers/memory/mediatek/Makefile    |   2 +
> >   drivers/memory/mediatek/mtk-dramc.c | 196
> > ++++++++++++++++++++++++++++
> >   5 files changed, 221 insertions(+)
> >   create mode 100644 drivers/memory/mediatek/Kconfig
> >   create mode 100644 drivers/memory/mediatek/Makefile
> >   create mode 100644 drivers/memory/mediatek/mtk-dramc.c
> > 
> > diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> > index c82d8d8a16ea..b1698549ff81 100644
> > --- a/drivers/memory/Kconfig
> > +++ b/drivers/memory/Kconfig
> > @@ -227,5 +227,6 @@ config STM32_FMC2_EBI
> > 
> >   source "drivers/memory/samsung/Kconfig"
> >   source "drivers/memory/tegra/Kconfig"
> > +source "drivers/memory/mediatek/Kconfig"
> > 
> >   endif
> > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> > index d2e6ca9abbe0..c0facf529803 100644
> > --- a/drivers/memory/Makefile
> > +++ b/drivers/memory/Makefile
> > @@ -27,6 +27,7 @@ obj-$(CONFIG_STM32_FMC2_EBI)        += stm32-
> > fmc2-ebi.o
> > 
> >   obj-$(CONFIG_SAMSUNG_MC)    += samsung/
> >   obj-$(CONFIG_TEGRA_MC)              += tegra/
> > +obj-$(CONFIG_MEDIATEK_MC)    += mediatek/
> >   obj-$(CONFIG_TI_EMIF_SRAM)  += ti-emif-sram.o
> >   obj-$(CONFIG_FPGA_DFL_EMIF) += dfl-emif.o
> > 
> > diff --git a/drivers/memory/mediatek/Kconfig
> > b/drivers/memory/mediatek/Kconfig
> > new file mode 100644
> > index 000000000000..3f238e0d9647
> > --- /dev/null
> > +++ b/drivers/memory/mediatek/Kconfig
> > @@ -0,0 +1,21 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +config MEDIATEK_MC
> > +     bool "MediaTek Memory Controller support"
> > +     help
> > +       This option allows to enable MediaTek memory controller
> > drivers,
> > +       which may include controllers for DRAM or others.
> > +       Select Y here if you need support for MediaTek memory
> > controller.
> > +       If you don't need, select N.
> > +
> > +if MEDIATEK_MC
> > +
> > +config MTK_DRAMC
> > +     tristate "MediaTek DRAMC driver"
> > +     default y
> > +     help
> > +       This option selects the MediaTek DRAMC driver, which
> > provides
> > +       an interface for reporting the current data rate of DRAM.
> > +       Select Y here if you need support for the MediaTek DRAMC
> > driver.
> > +       If you don't need, select N.
> > +
> > +endif
> > diff --git a/drivers/memory/mediatek/Makefile
> > b/drivers/memory/mediatek/Makefile
> > new file mode 100644
> > index 000000000000..a1395fc55b41
> > --- /dev/null
> > +++ b/drivers/memory/mediatek/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +obj-$(CONFIG_MTK_DRAMC)              += mtk-dramc.o
> > diff --git a/drivers/memory/mediatek/mtk-dramc.c
> > b/drivers/memory/mediatek/mtk-dramc.c
> > new file mode 100644
> > index 000000000000..d452483a98ce
> > --- /dev/null
> > +++ b/drivers/memory/mediatek/mtk-dramc.c
> > @@ -0,0 +1,196 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2025 MediaTek Inc.
> > + */
> > +#include <linux/bitops.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/device.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/printk.h>
> > +
> > +#define POSDIV_PURIFY        BIT(2)
> > +#define PREDIV               7
> > +#define REF_FREQUENCY        26
> 
> REF_FREQUENCY should be platform data, not a definition, as this is
> different in other SoCs.
> 

Understood, it will be updated in the next version.

> > +#define SHUFFLE_OFFSET       0x700
> > +
> > +/*--------------------------------------------------------------
> > ------------*/
> > +/* Register
> > Offset                                                          */
> > +/*--------------------------------------------------------------
> > ------------*/
> > +#define DPHY_DVFS_STA                0x0e98
> > +#define APHY_PHYPLL2         0x0908
> > +#define APHY_CLRPLL2         0x0928
> > +#define APHY_PHYPLL3         0x090c
> > +#define APHY_CLRPLL3         0x092c
> > +#define APHY_PHYPLL4         0x0910
> > +#define APHY_ARPI0           0x0d94
> > +#define APHY_CA_ARDLL1               0x0d08
> > +#define APHY_B0_TX0          0x0dc4
> 
> Aren't those also SoC-dependant?

Yes, I will move those to platform data in the next version.

> 
> enum mtk_dramc_reg_index {
>         DRAMC_DPHY_DVFS_STA,
>         DRAMC_APHY_SHU_PHYPLL2,
>         ....etc
> }
> 
> static const u16 mtk_dramc_regs_mt8195[] = {
>         [DRAMC_DPHY_DVFS_STA] = 0x50c, /* ddrphy_config_NAO */
>         [DRAMC_APHY_SHU_PHYPLL2] = 0x704,
>         [DRAMC_APHY_SHU_CLRPLL2] = 0x724,
>         [DRAMC_APHY_SHU_PHYPLL3] = 0x708,
>         [DRAMC_APHY_SHU_CLRPLL3] = 0x728,
>         [DRAMC_APHY_SHU_PHYPLL4] = 0x70c,
>         ...etc
> }
> 
> static const u16 mtk_dramc_regs_mt8196[] = {
>         [DRAMC_DPHY_DVFS_STA] = 0xe98, /* ddrphy_config_misc */
>         [DRAMC_APHY_SHU_PHYPLL2] = 0x908,
>         [DRAMC_APHY_SHU_CLRPLL2] = 0x928,
>         .... etc
> }
> 
> > +
> > +/*--------------------------------------------------------------
> > ------------*/
> > +/* Register
> > Mask                                                            */
> > +/*--------------------------------------------------------------
> > ------------*/
> > +#define DPHY_DVFS_SHU_LV     GENMASK(15, 14)
> > +#define DPHY_DVFS_PLL_SEL    GENMASK(25, 25)
> > +#define APHY_PLL2_SDMPCW     GENMASK(18, 3)
> > +#define APHY_PLL3_POSDIV     GENMASK(13, 11)
> > +#define APHY_PLL4_FBKSEL     GENMASK(6, 6)
> > +#define APHY_ARPI0_SOPEN     GENMASK(26, 26)
> > +#define APHY_ARDLL1_CK_EN    GENMASK(0, 0)
> > +#define APHY_B0_TX0_SER_MODE GENMASK(4, 3)
> 
> And those masks should also be platform data, as those are also
> different
> in other SoCs.
> 

Yes, I will move those masks to platform data in the next version.

> > +
> > +static unsigned int read_reg_field(void __iomem *base, unsigned
> > int offset, unsigned int mask)
> > +{
> > +     unsigned int val = readl(base + offset);
> > +     unsigned int shift = __ffs(mask);
> > +
> > +     return (val & mask) >> shift;
> > +}
> > +
> > +struct mtk_dramc_pdata {
> > +     unsigned int fmeter_version;
> 
> u8 fmeter_version;
> u8 ref_freq_mhz;
> const u16 *regs;
> 
> ...etc etc
> 

Ok, I will update the platform data in the next version.

> 
> > +};
> > +
> > +struct mtk_dramc_dev_t {
> > +     void __iomem *anaphy_base;
> > +     void __iomem *ddrphy_base;
> > +     const struct mtk_dramc_pdata *pdata;
> > +};
> > +
> > +static int mtk_dramc_probe(struct platform_device *pdev)
> > +{
> > +     struct mtk_dramc_dev_t *dramc;
> > +     const struct mtk_dramc_pdata *pdata;
> > +     int ret;
> > +
> > +     dramc = devm_kzalloc(&pdev->dev, sizeof(struct
> > mtk_dramc_dev_t), GFP_KERNEL);
> > +     if (!dramc)
> > +             return dev_err_probe(&pdev->dev, -ENOMEM, "Failed to
> > allocate memory\n");
> > +
> > +     pdata = of_device_get_match_data(&pdev->dev);
> > +     if (!pdata)
> > +             return dev_err_probe(&pdev->dev, -EINVAL, "No
> > platform data available\n");
> > +
> > +     dramc->pdata = pdata;
> > +
> > +     dramc->anaphy_base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(dramc->anaphy_base)) {
> > +             ret = PTR_ERR(dramc->anaphy_base);
> > +             return dev_err_probe(&pdev->dev, ret, "Unable to map
> > DDRPHY NAO base\n");
> > +     }
> > +
> > +     dramc->ddrphy_base = devm_platform_ioremap_resource(pdev, 1);
> > +     if (IS_ERR(dramc->ddrphy_base)) {
> > +             ret = PTR_ERR(dramc->ddrphy_base);
> > +             return dev_err_probe(&pdev->dev, ret, "Unable to map
> > DDRPHY AO base\n");
> > +     }
> > +
> > +     platform_set_drvdata(pdev, dramc);
> > +     return 0;
> > +}
> > +
> > +static unsigned int mtk_fmeter_v1(struct mtk_dramc_dev_t *dramc)
> > +{
> > +     unsigned int shu_level, pll_sel, offset;
> > +     unsigned int sdmpcw, posdiv, ckdiv4, fbksel, sopen, async_ca,
> > ser_mode;
> > +     unsigned int perdiv_freq, posdiv_freq, vco_freq;
> > +     unsigned int final_rate;
> > +
> > +     shu_level = read_reg_field(dramc->ddrphy_base, DPHY_DVFS_STA,
> > DPHY_DVFS_SHU_LV);
> > +     pll_sel = read_reg_field(dramc->ddrphy_base, DPHY_DVFS_STA,
> > DPHY_DVFS_PLL_SEL);
> > +     offset = SHUFFLE_OFFSET * shu_level;
> > +
> > +     sdmpcw = read_reg_field(dramc->anaphy_base,
> > +                             ((pll_sel == 0) ? APHY_PHYPLL2 :
> > APHY_CLRPLL2) + offset,
> > +                             APHY_PLL2_SDMPCW);
> > +     posdiv = read_reg_field(dramc->anaphy_base,
> > +                             ((pll_sel == 0) ? APHY_PHYPLL3 :
> > APHY_CLRPLL3) + offset,
> > +                             APHY_PLL3_POSDIV);
> > +     fbksel = read_reg_field(dramc->anaphy_base, APHY_PHYPLL4 +
> > offset, APHY_PLL4_FBKSEL);
> > +     sopen = read_reg_field(dramc->anaphy_base, APHY_ARPI0 +
> > offset, APHY_ARPI0_SOPEN);
> > +     async_ca = read_reg_field(dramc->anaphy_base, APHY_CA_ARDLL1
> > + offset, APHY_ARDLL1_CK_EN);
> > +     ser_mode = read_reg_field(dramc->anaphy_base, APHY_B0_TX0 +
> > offset, APHY_B0_TX0_SER_MODE);
> > +
> > +     ckdiv4 = (ser_mode == 1) ? 1 : 0;
> > +     posdiv &= ~(POSDIV_PURIFY);
> > +
> > +     perdiv_freq = REF_FREQUENCY * (sdmpcw >> PREDIV);
> 
> s/perdiv/prediv/g

Thank you for the reminder, will make the correction in the next
version.

> 
> > +     posdiv_freq = (perdiv_freq >> posdiv) >> 1;
> > +     vco_freq = posdiv_freq << fbksel;
> > +     final_rate = vco_freq >> ckdiv4;
> > +
> 
> there's also one case in which `final_rate >>= 2`... please check if
> it is
> applicable to this driver (it should).
> 
> Cheers,
> Angelo

 
The variable name 'clkdiv4' might be misleading.
As its value can only be 1 or 0, the actual DDR data rate can be
obtained by 'final_rate = vco_freq >> clkdiv' (except when 'sopen == 1
&& async_ca == 1'), thus 'final_rate >>= 2' is not needed.
Renaming 'clkdiv4' to 'clkdiv' might be more appropriate.

Thanks,
Crystal

> 
> > +     if (sopen == 1 && async_ca == 1)
> > +             final_rate >>= 1;
> > +
> > +     return final_rate;
> > +}
> > +
> > +/*
> > + * mtk_dramc_get_data_rate - calculate DRAM data rate
> > + *
> > + * Returns DRAM data rate (MB/s)
> > + */
> > +static unsigned int mtk_dramc_get_data_rate(struct device *dev)
> > +{
> > +     struct mtk_dramc_dev_t *dramc_dev = dev_get_drvdata(dev);
> > +
> > +     if (!dramc_dev) {
> > +             dev_err(dev, "DRAMC device data not found\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (dramc_dev->pdata) {
> > +             if (dramc_dev->pdata->fmeter_version == 1)
> > +                     return mtk_fmeter_v1(dramc_dev);
> > +
> > +             dev_err(dev, "Unsupported fmeter version\n");
> > +             return -EINVAL;
> > +     }
> > +     dev_err(dev, "DRAMC platform data not found\n");
> > +     return -EINVAL;
> > +}
> > +
> > +static ssize_t dram_data_rate_show(struct device *dev,
> > +                                struct device_attribute *attr,
> > char *buf)
> > +{
> > +     return snprintf(buf, PAGE_SIZE, "DRAM data rate = %u\n",
> > +                     mtk_dramc_get_data_rate(dev));
> > +}
> > +
> > +static DEVICE_ATTR_RO(dram_data_rate);
> > +
> > +static struct attribute *mtk_dramc_attrs[] = {
> > +     &dev_attr_dram_data_rate.attr,
> > +     NULL
> > +};
> > +ATTRIBUTE_GROUPS(mtk_dramc);
> > +
> > +static const struct mtk_dramc_pdata dramc_pdata_mt8196 = {
> > +     .fmeter_version = 1
> > +};
> > +
> > +static const struct of_device_id mtk_dramc_of_ids[] = {
> > +     { .compatible = "mediatek,mt8196-dramc", .data =
> > &dramc_pdata_mt8196 },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_dramc_of_ids);
> > +
> > +static struct platform_driver mtk_dramc_driver = {
> > +     .probe = mtk_dramc_probe,
> > +     .driver = {
> > +             .name = "mtk_dramc_drv",
> > +             .of_match_table = mtk_dramc_of_ids,
> > +             .dev_groups = mtk_dramc_groups,
> > +     },
> > +};
> > +
> > +module_platform_driver(mtk_dramc_driver);
> > +
> > +MODULE_AUTHOR("Crystal Guo <crystal.guo@mediatek.com>");
> > +MODULE_DESCRIPTION("MediaTek DRAM Controller Driver");
> > +MODULE_LICENSE("GPL");
> 
>