arm/arm64: mediatek: Fix mmsys device probing

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mfd/mtk-mmsys.c

drivers/mfd/Kconfig +9 -0

index b860eb5aa194..d23a3b9a2c58 100644
@@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
 	help
 	  Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MEDIATEK_MMSYS
+	tristate "Mediatek MMSYS interface"
+	select MFD_CORE
+	select REGMAP_MMIO
+	help
+	  Select this if you have a MMSYS subsystem in your SoC. The
+	  MMSYS subsystem has at least a clock driver part and some
+	  DRM components.
+
 config MFD_MXS_LRADC
 	tristate "Freescale i.MX23/i.MX28 LRADC"
 	depends on ARCH_MXS || COMPILE_TEST

drivers/mfd/Makefile +2 -0

index d9d2cf0d32ef..b96118bd68d9 100644
@@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)		+= ezx-pcap.o

drivers/mfd/mtk-mmsys.c new file +49 -0

new file mode 100644
index 000000000000..c802343fb1c6
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
+ *
+ * Copyright (c) 2018 Matthias Brugger <[email protected]>
+ *
+ * Author: Matthias Brugger <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum {
+	MMSYS_MT2701 = 1,
+};
+
+static const struct mfd_cell mmsys_mt2701_devs[] = {
+	{ .name = "clk-mt2701-mm", },
+	{ .name = "drm-mt2701-mm", },
+};
+
+static int mmsys_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *mmsys_cells;
+	int nr_cells;
+	long id;
+	int ret;
+
+	id = (long) of_device_get_match_data(&pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+		return -EINVAL;
+	}
+
+	switch (id) {
+	case MMSYS_MT2701:
+		mmsys_cells = mmsys_mt2701_devs;
+		nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+		break;
+	default:
+		return -ENODEV;
+	}

drivers/mfd/mtk-mmsys.c new file +19 -0

+static const struct of_device_id of_match_mmsys[] = {
+	{ .compatible = "mediatek,mt2701-mmsys",
+	  .data = (void *) MMSYS_MT2701,
+	},
+	{ /* sentinel */ },
+};
+
+static struct platform_driver mmsys_drv = {
+	.probe = mmsys_probe,
+	.driver = {
+		.name = "mediatek-mmysys",
+		.of_match_table = of_match_ptr(of_match_mmsys),
+	},
+};
+
+builtin_platform_driver(mmsys_drv);
+
+MODULE_DESCRIPTION("Mediatek MMSYS multi-function driver");
+MODULE_LICENSE("GPL");

From: Matthias Brugger <[email protected]>

Add binding description for the mmsys mfd for some Mediatek devices. mmsys has some registers to control clock gates (which is used in the clk driver) and some registers to set the routing and enable the differnet blocks of the display subsystem.

Signed-off-by Matthias Brugger <[email protected]>

---
 .../bindings/arm/mediatek/mediatek,mmsys.txt       |  2 --
 .../bindings/display/mediatek/mediatek,disp.txt    |  2 +-
 .../devicetree/bindings/mfd/mediatek,mmsys.txt     | 27 ++++++++++++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt

Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt +0 -2

index 4eb8bbe15c01..4468345f8b1a 100644
@@ -6,10 +6,8 @@ The Mediatek mmsys controller provides various clocks to the system.
 Required Properties:
 
 - compatible: Should be one of:
-	- "mediatek,mt2701-mmsys", "syscon"
 	- "mediatek,mt2712-mmsys", "syscon"
 	- "mediatek,mt6797-mmsys", "syscon"
-	- "mediatek,mt8173-mmsys", "syscon"
 - #clock-cells: Must be 1
 
 The mmsys controller uses the common clk binding from

Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt +1 -1

index 383183a89164..85a3b4ec06cd 100644
@@ -9,7 +9,7 @@ function block.
 
 All DISP device tree nodes must be siblings to the central MMSYS_CONFIG node.
 For a description of the MMSYS_CONFIG binding, see
-Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt.
+Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt
 
 DISP function blocks
 ====================

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt new file +0 -0

new file mode 100644
index 000000000000..2331ae16917e

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt new file +25 -0

+MMSYS is a multifunction device with the following sub modules:
+- clocks for the multi-media subsystem
+- central node for the DRM subsystem.
+
+This document describes the binding for MFD device. The MFD takes care to initailize
+the clock driver and the DRM driver. More info see
+Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
+
+Required properties:
+- compatible: Should be one of:
+	- "mediatek,mt2701-mmsys", "syscon"
+	- "mediatek,mt8173-mmsys", "syscon"
+- #clock-cells: Must be 1
+
+Optional properties:
+- power-domains: list of powerdomains needed for the subsystem to work
+
+Example:
+
+mmsys: clock-controller at 14000000 {
+	compatible = "mediatek,mt8173-mmsys", "syscon";
+	reg = <0 0x14000000 0 0x1000>;
+	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+	#clock-cells = <1>;
+};

From: Matthias Brugger <[email protected]>

With the mtk-mmsys MFD device in place, we switch the probing for mt2701 from device-tree to mfd.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

drivers/gpu/drm/mediatek/mtk_drm_drv.c +23 -7

index a48e28adad09..88ee35907744 100644
@@ -386,7 +386,7 @@ static int mtk_drm_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct mtk_drm_private *private;
-	struct device_node *node;
+	struct device_node *node, *parent_node, *mmsys_node;
 	struct component_match *match = NULL;
 	int ret;
 	int i;

@@ -399,12 +399,23 @@ static int mtk_drm_probe(struct platform_device *pdev)
 	INIT_WORK(&private->commit.work, mtk_atomic_work);
 	private->data = of_device_get_match_data(dev);
 
-	private->config_regs = syscon_node_to_regmap(dev->of_node);
-	if (IS_ERR(private->config_regs))
-		return PTR_ERR(private->config_regs);
+	/* Check if called from mfd */
+	if (!dev->of_node) {
+		mmsys_node = pdev->dev.parent->of_node;
+		private->data = (struct mtk_mmsys_driver_data *)
+				platform_get_device_id(pdev)->driver_data;
+		private->config_regs =
+			syscon_node_to_regmap(mmsys_node);
+		parent_node = mmsys_node->parent;
+	} else {
+		private->config_regs = syscon_node_to_regmap(dev->of_node);
+		if (IS_ERR(private->config_regs))
+			return PTR_ERR(private->config_regs);
+		parent_node = dev->of_node->parent;
+	}
 
 	/* Iterate over sibling DISP function blocks */
-	for_each_child_of_node(dev->of_node->parent, node) {
+	for_each_child_of_node(parent_node, node) {
 		const struct of_device_id *of_id;
 		enum mtk_ddp_comp_type comp_type;
 		int comp_id;

@@ -545,13 +556,17 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
 			 mtk_drm_sys_resume);
 
 static const struct of_device_id mtk_drm_of_ids[] = {
-	{ .compatible = "mediatek,mt2701-mmsys",
-	  .data = &mt2701_mmsys_driver_data},
 	{ .compatible = "mediatek,mt8173-mmsys",
 	  .data = &mt8173_mmsys_driver_data},
 	{ }
 };
 
+static const struct platform_device_id mtk_drm_ids[] = {
+	{ "drm-mt2701-mm", (kernel_ulong_t)&mt2701_mmsys_driver_data },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, mtk_drm_ids);
+
 static struct platform_driver mtk_drm_platform_driver = {
 	.probe	= mtk_drm_probe,
 	.remove	= mtk_drm_remove,

@@ -560,6 +575,7 @@ static struct platform_driver mtk_drm_platform_driver = {
 		.of_match_table = mtk_drm_of_ids,
 		.pm     = &mtk_drm_pm_ops,
 	},
+	.id_table = mtk_drm_ids,
 };
 
 static struct platform_driver * const mtk_drm_drivers[] = {

From: Matthias Brugger <[email protected]>

When probe through the MFD, it can happen that the
clock drivers wasn't probed before the ddp driver gets invoked. The driver used to omit a warning that the driver failed to get the clocks. Omit this error on the defered probe path.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/gpu/drm/mediatek/mtk_drm_ddp.c +2 -1

index bafc5c77c4fb..6b399348a2dc 100644
@@ -374,7 +374,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
 
 	ddp->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(ddp->clk)) {
-		dev_err(dev, "Failed to get clock\n");
+		if (PTR_ERR(ddp->clk) != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get clock\n");
 		return PTR_ERR(ddp->clk);
 	}
 

Quoted 11 lines · On Fri, 27 Apr 2018, [email protected] wrote:

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++

Quoted 1 line

+};

This driver is pretty pointless. It doesn't actually do anything.

I think you just want to use "simple-mfd" instead.

Quoted 19 lines

drivers/mfd/mtk-mmsys.c new file +19 -0

+static const struct of_device_id of_match_mmsys[] = {
+	{ .compatible = "mediatek,mt2701-mmsys",
+	  .data = (void *) MMSYS_MT2701,
+	},
+	{ /* sentinel */ },
+};
+
+static struct platform_driver mmsys_drv = {
+	.probe = mmsys_probe,
+	.driver = {
+		.name = "mediatek-mmysys",
+		.of_match_table = of_match_ptr(of_match_mmsys),
+	},
+};
+
+builtin_platform_driver(mmsys_drv);
+
+MODULE_DESCRIPTION("Mediatek MMSYS multi-function driver");
+MODULE_LICENSE("GPL");

Quoted 102 lines · On Fri, 27 Apr 2018, [email protected] wrote:

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mfd/mtk-mmsys.c

drivers/mfd/Kconfig +9 -0

index b860eb5aa194..d23a3b9a2c58 100644
@@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
 	help
 	  Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MEDIATEK_MMSYS
+	tristate "Mediatek MMSYS interface"
+	select MFD_CORE
+	select REGMAP_MMIO
+	help
+	  Select this if you have a MMSYS subsystem in your SoC. The
+	  MMSYS subsystem has at least a clock driver part and some
+	  DRM components.
+
 config MFD_MXS_LRADC
 	tristate "Freescale i.MX23/i.MX28 LRADC"
 	depends on ARCH_MXS || COMPILE_TEST

drivers/mfd/Makefile +2 -0

index d9d2cf0d32ef..b96118bd68d9 100644
@@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)		+= ezx-pcap.o

drivers/mfd/mtk-mmsys.c new file +49 -0

new file mode 100644
index 000000000000..c802343fb1c6
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
+ *
+ * Copyright (c) 2018 Matthias Brugger <[email protected]>
+ *
+ * Author: Matthias Brugger <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum {
+	MMSYS_MT2701 = 1,
+};
+
+static const struct mfd_cell mmsys_mt2701_devs[] = {
+	{ .name = "clk-mt2701-mm", },
+	{ .name = "drm-mt2701-mm", },
+};
+
+static int mmsys_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *mmsys_cells;
+	int nr_cells;
+	long id;
+	int ret;
+
+	id = (long) of_device_get_match_data(&pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+		return -EINVAL;
+	}
+
+	switch (id) {
+	case MMSYS_MT2701:
+		mmsys_cells = mmsys_mt2701_devs;
+		nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+		break;
+	default:
+		return -ENODEV;
+	}

drivers/mfd/mtk-mmsys.c line 59

Quoted 10 lines

drivers/mfd/mtk-mmsys.c new file +10 -0

+
+	ret = devm_mfd_add_devices(&pdev->dev, 0, mmsys_cells, nr_cells,
+					NULL, 0, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add MFD devices %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};

This driver is pretty pointless. It doesn't actually do anything.

I think you just want to use "simple-mfd" instead.

Quoted 49 lines · On Fri, 27 Apr 2018, [email protected] wrote:

From: Matthias Brugger <[email protected]>

Add binding description for the mmsys mfd for some Mediatek devices. mmsys has some registers to control clock gates (which is used in the clk driver) and some registers to set the routing and enable the differnet blocks of the display subsystem.

Signed-off-by Matthias Brugger <[email protected]>

---
 .../bindings/arm/mediatek/mediatek,mmsys.txt       |  2 --
 .../bindings/display/mediatek/mediatek,disp.txt    |  2 +-
 .../devicetree/bindings/mfd/mediatek,mmsys.txt     | 27 ++++++++++++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt

Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt +0 -2

index 4eb8bbe15c01..4468345f8b1a 100644
@@ -6,10 +6,8 @@ The Mediatek mmsys controller provides various clocks to the system.
 Required Properties:
 
 - compatible: Should be one of:
-	- "mediatek,mt2701-mmsys", "syscon"
 	- "mediatek,mt2712-mmsys", "syscon"
 	- "mediatek,mt6797-mmsys", "syscon"
-	- "mediatek,mt8173-mmsys", "syscon"
 - #clock-cells: Must be 1
 
 The mmsys controller uses the common clk binding from

Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt +1 -1

index 383183a89164..85a3b4ec06cd 100644
@@ -9,7 +9,7 @@ function block.
 
 All DISP device tree nodes must be siblings to the central MMSYS_CONFIG node.
 For a description of the MMSYS_CONFIG binding, see
-Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt.
+Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt
 
 DISP function blocks
 ====================

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt new file +0 -0

new file mode 100644
index 000000000000..2331ae16917e

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt line 1

Quoted 1 line

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt new file +1 -0

@@ -0,0 +1,27 @@
+MediaTek MMSYS Multifunction Device Driver

What is "MMSYS"?

"Multi-Function Driver"s are specific to Linux.

What actually is the device?

Quoted 25 lines

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt new file +25 -0

+MMSYS is a multifunction device with the following sub modules:
+- clocks for the multi-media subsystem
+- central node for the DRM subsystem.
+
+This document describes the binding for MFD device. The MFD takes care to initailize
+the clock driver and the DRM driver. More info see
+Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
+
+Required properties:
+- compatible: Should be one of:
+	- "mediatek,mt2701-mmsys", "syscon"
+	- "mediatek,mt8173-mmsys", "syscon"
+- #clock-cells: Must be 1
+
+Optional properties:
+- power-domains: list of powerdomains needed for the subsystem to work
+
+Example:
+
+mmsys: clock-controller@14000000 {
+	compatible = "mediatek,mt8173-mmsys", "syscon";
+	reg = <0 0x14000000 0 0x1000>;
+	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+	#clock-cells = <1>;
+};

Quoted 9 lines · Lee Jones

Quoted 7 lines · matthias.bgg at kernel.org

From: Matthias Brugger <[email protected]>

Add binding description for the mmsys mfd for some Mediatek devices. mmsys has some registers to control clock gates (which is used in the clk driver) and some registers to set the routing and enable the differnet blocks of the display subsystem.

Quoted 7 lines

Quoted 7 lines

Signed-off-by Matthias Brugger <[email protected]>

---
 .../bindings/arm/mediatek/mediatek,mmsys.txt       |  2 --
 .../bindings/display/mediatek/mediatek,disp.txt    |  2 +-
 .../devicetree/bindings/mfd/mediatek,mmsys.txt     | 27 ++++++++++++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt

Quoted 55 lines · Lee Jones

Quoted 49 lines · matthias.bgg at kernel.org

From: Matthias Brugger <[email protected]>

Add binding description for the mmsys mfd for some Mediatek devices. mmsys has some registers to control clock gates (which is used in the clk driver) and some registers to set the routing and enable the differnet blocks of the display subsystem.

Signed-off-by Matthias Brugger <[email protected]>

---
 .../bindings/arm/mediatek/mediatek,mmsys.txt       |  2 --
 .../bindings/display/mediatek/mediatek,disp.txt    |  2 +-
 .../devicetree/bindings/mfd/mediatek,mmsys.txt     | 27 ++++++++++++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt

Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt +0 -2

index 4eb8bbe15c01..4468345f8b1a 100644
@@ -6,10 +6,8 @@ The Mediatek mmsys controller provides various clocks to the system.
 Required Properties:
 
 - compatible: Should be one of:
-	- "mediatek,mt2701-mmsys", "syscon"
 	- "mediatek,mt2712-mmsys", "syscon"
 	- "mediatek,mt6797-mmsys", "syscon"
-	- "mediatek,mt8173-mmsys", "syscon"
 - #clock-cells: Must be 1
 
 The mmsys controller uses the common clk binding from

Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt +1 -1

index 383183a89164..85a3b4ec06cd 100644
@@ -9,7 +9,7 @@ function block.
 
 All DISP device tree nodes must be siblings to the central MMSYS_CONFIG node.
 For a description of the MMSYS_CONFIG binding, see
-Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt.
+Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt
 
 DISP function blocks
 ====================

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt new file +0 -0

new file mode 100644
index 000000000000..2331ae16917e

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt line 1

Quoted 1 line

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt new file +1 -0

@@ -0,0 +1,27 @@
+MediaTek MMSYS Multifunction Device Driver

What is "MMSYS"?

Quoted 3 lines

"Multi-Function Driver"s are specific to Linux.

What actually is the device?

It's includes all the DRM parts of the SoC together with the clocks for the device. MMSYS IMHO stands for Multi-Media-System.

Quoted 26 lines

Quoted 25 lines

Documentation/devicetree/bindings/mfd/mediatek,mmsys.txt new file +25 -0

+MMSYS is a multifunction device with the following sub modules:
+- clocks for the multi-media subsystem
+- central node for the DRM subsystem.
+
+This document describes the binding for MFD device. The MFD takes care to initailize
+the clock driver and the DRM driver. More info see
+Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
+
+Required properties:
+- compatible: Should be one of:
+	- "mediatek,mt2701-mmsys", "syscon"
+	- "mediatek,mt8173-mmsys", "syscon"
+- #clock-cells: Must be 1
+
+Optional properties:
+- power-domains: list of powerdomains needed for the subsystem to work
+
+Example:
+
+mmsys: clock-controller at 14000000 {
+	compatible = "mediatek,mt8173-mmsys", "syscon";
+	reg = <0 0x14000000 0 0x1000>;
+	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+	#clock-cells = <1>;
+};

Quoted 117 lines · Lee Jones

Quoted 102 lines · matthias.bgg at kernel.org

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mfd/mtk-mmsys.c

drivers/mfd/Kconfig +9 -0

index b860eb5aa194..d23a3b9a2c58 100644
@@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
 	help
 	  Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MEDIATEK_MMSYS
+	tristate "Mediatek MMSYS interface"
+	select MFD_CORE
+	select REGMAP_MMIO
+	help
+	  Select this if you have a MMSYS subsystem in your SoC. The
+	  MMSYS subsystem has at least a clock driver part and some
+	  DRM components.
+
 config MFD_MXS_LRADC
 	tristate "Freescale i.MX23/i.MX28 LRADC"
 	depends on ARCH_MXS || COMPILE_TEST

drivers/mfd/Makefile +2 -0

index d9d2cf0d32ef..b96118bd68d9 100644
@@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)		+= ezx-pcap.o

drivers/mfd/mtk-mmsys.c new file +49 -0

new file mode 100644
index 000000000000..c802343fb1c6
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
+ *
+ * Copyright (c) 2018 Matthias Brugger <[email protected]>
+ *
+ * Author: Matthias Brugger <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum {
+	MMSYS_MT2701 = 1,
+};
+
+static const struct mfd_cell mmsys_mt2701_devs[] = {
+	{ .name = "clk-mt2701-mm", },
+	{ .name = "drm-mt2701-mm", },
+};
+
+static int mmsys_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *mmsys_cells;
+	int nr_cells;
+	long id;
+	int ret;
+
+	id = (long) of_device_get_match_data(&pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+		return -EINVAL;
+	}
+
+	switch (id) {
+	case MMSYS_MT2701:
+		mmsys_cells = mmsys_mt2701_devs;
+		nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+		break;
+	default:
+		return -ENODEV;
+	}

drivers/mfd/mtk-mmsys.c line 59

Quoted 10 lines

drivers/mfd/mtk-mmsys.c new file +10 -0

+
+	ret = devm_mfd_add_devices(&pdev->dev, 0, mmsys_cells, nr_cells,
+					NULL, 0, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add MFD devices %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};

This driver is pretty pointless. It doesn't actually do anything.

Quoted 2 lines

I think you just want to use "simple-mfd" instead.

I think the problem is, that right now we have two drivers which use the same devicetree binding, which are clk and drm driver. With a simple-mfd we would need two compatibles, and this would break backwards compatibility.

Quoted 121 lines · Matthias Brugger

Quoted 119 lines · Lee Jones

Quoted 102 lines · On Fri, 27 Apr 2018, [email protected] wrote:

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mfd/mtk-mmsys.c

drivers/mfd/Kconfig +9 -0

index b860eb5aa194..d23a3b9a2c58 100644
@@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
 	help
 	  Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MEDIATEK_MMSYS
+	tristate "Mediatek MMSYS interface"
+	select MFD_CORE
+	select REGMAP_MMIO
+	help
+	  Select this if you have a MMSYS subsystem in your SoC. The
+	  MMSYS subsystem has at least a clock driver part and some
+	  DRM components.
+
 config MFD_MXS_LRADC
 	tristate "Freescale i.MX23/i.MX28 LRADC"
 	depends on ARCH_MXS || COMPILE_TEST

drivers/mfd/Makefile +2 -0

index d9d2cf0d32ef..b96118bd68d9 100644
@@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)		+= ezx-pcap.o

drivers/mfd/mtk-mmsys.c new file +49 -0

new file mode 100644
index 000000000000..c802343fb1c6
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
+ *
+ * Copyright (c) 2018 Matthias Brugger <[email protected]>
+ *
+ * Author: Matthias Brugger <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum {
+	MMSYS_MT2701 = 1,
+};
+
+static const struct mfd_cell mmsys_mt2701_devs[] = {
+	{ .name = "clk-mt2701-mm", },
+	{ .name = "drm-mt2701-mm", },
+};
+
+static int mmsys_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *mmsys_cells;
+	int nr_cells;
+	long id;
+	int ret;
+
+	id = (long) of_device_get_match_data(&pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+		return -EINVAL;
+	}
+
+	switch (id) {
+	case MMSYS_MT2701:
+		mmsys_cells = mmsys_mt2701_devs;
+		nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+		break;
+	default:
+		return -ENODEV;
+	}

drivers/mfd/mtk-mmsys.c line 59

Quoted 10 lines

drivers/mfd/mtk-mmsys.c new file +10 -0

+
+	ret = devm_mfd_add_devices(&pdev->dev, 0, mmsys_cells, nr_cells,
+					NULL, 0, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add MFD devices %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};

This driver is pretty pointless. It doesn't actually do anything.

I think you just want to use "simple-mfd" instead.

Quoted 3 lines

I think the problem is, that right now we have two drivers which use the same devicetree binding, which are clk and drm driver. With a simple-mfd we would need two compatibles, and this would break backwards compatibility.

So what functionality does this driver provide you with that you do not have currently?

Quoted 122 lines · Lee Jones

Quoted 121 lines · Matthias Brugger

Quoted 119 lines · Lee Jones

Quoted 102 lines · matthias.bgg at kernel.org

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mfd/mtk-mmsys.c

drivers/mfd/Kconfig +9 -0

index b860eb5aa194..d23a3b9a2c58 100644
@@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
 	help
 	  Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MEDIATEK_MMSYS
+	tristate "Mediatek MMSYS interface"
+	select MFD_CORE
+	select REGMAP_MMIO
+	help
+	  Select this if you have a MMSYS subsystem in your SoC. The
+	  MMSYS subsystem has at least a clock driver part and some
+	  DRM components.
+
 config MFD_MXS_LRADC
 	tristate "Freescale i.MX23/i.MX28 LRADC"
 	depends on ARCH_MXS || COMPILE_TEST

drivers/mfd/Makefile +2 -0

index d9d2cf0d32ef..b96118bd68d9 100644
@@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)		+= ezx-pcap.o

drivers/mfd/mtk-mmsys.c new file +49 -0

new file mode 100644
index 000000000000..c802343fb1c6
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
+ *
+ * Copyright (c) 2018 Matthias Brugger <[email protected]>
+ *
+ * Author: Matthias Brugger <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum {
+	MMSYS_MT2701 = 1,
+};
+
+static const struct mfd_cell mmsys_mt2701_devs[] = {
+	{ .name = "clk-mt2701-mm", },
+	{ .name = "drm-mt2701-mm", },
+};
+
+static int mmsys_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *mmsys_cells;
+	int nr_cells;
+	long id;
+	int ret;
+
+	id = (long) of_device_get_match_data(&pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+		return -EINVAL;
+	}
+
+	switch (id) {
+	case MMSYS_MT2701:
+		mmsys_cells = mmsys_mt2701_devs;
+		nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+		break;
+	default:
+		return -ENODEV;
+	}

drivers/mfd/mtk-mmsys.c line 59

Quoted 10 lines

drivers/mfd/mtk-mmsys.c new file +10 -0

+
+	ret = devm_mfd_add_devices(&pdev->dev, 0, mmsys_cells, nr_cells,
+					NULL, 0, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add MFD devices %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};

This driver is pretty pointless. It doesn't actually do anything.

I think you just want to use "simple-mfd" instead.

Quoted 7 lines

Quoted 3 lines

I think the problem is, that right now we have two drivers which use the same devicetree binding, which are clk and drm driver. With a simple-mfd we would need two compatibles, and this would break backwards compatibility.

So what functionality does this driver provide you with that you do not have currently?

I'm not sure if I get your question. Point is, that the MMSYS implementation for mt8173 is broken, as it assumes that we can probe two drivers with the mediatek,mt8173-mmsys compatible. Somehow it used to work, but from what I understand it was a bug. So older devicetrees use just on mt8173-mmsys compatible in ther DTB.

I would like to keep backwards compatibility for the device tree, that's why I was searching for a solution where we can probe two drivers and came up with this mfd solution.

So no new functionality, the clk driver provides the clock the drm components need.

Quoted 122 lines · Lee Jones

Quoted 121 lines · Matthias Brugger

Quoted 119 lines · Lee Jones

Quoted 102 lines · matthias.bgg at kernel.org

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mfd/mtk-mmsys.c

drivers/mfd/Kconfig +9 -0

index b860eb5aa194..d23a3b9a2c58 100644
@@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
   help
     Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MEDIATEK_MMSYS
+  tristate "Mediatek MMSYS interface"
+  select MFD_CORE
+  select REGMAP_MMIO
+  help
+    Select this if you have a MMSYS subsystem in your SoC. The
+    MMSYS subsystem has at least a clock driver part and some
+    DRM components.
+
 config MFD_MXS_LRADC
   tristate "Freescale i.MX23/i.MX28 LRADC"
   depends on ARCH_MXS || COMPILE_TEST

drivers/mfd/Makefile +2 -0

index d9d2cf0d32ef..b96118bd68d9 100644
@@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)        += mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)     += mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)     += mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
+
 obj-$(CONFIG_MFD_CORE)            += mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)            += ezx-pcap.o

drivers/mfd/mtk-mmsys.c new file +49 -0

new file mode 100644
index 000000000000..c802343fb1c6
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
+ *
+ * Copyright (c) 2018 Matthias Brugger <[email protected]>
+ *
+ * Author: Matthias Brugger <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum {
+  MMSYS_MT2701 = 1,
+};
+
+static const struct mfd_cell mmsys_mt2701_devs[] = {
+  { .name = "clk-mt2701-mm", },
+  { .name = "drm-mt2701-mm", },
+};
+
+static int mmsys_probe(struct platform_device *pdev)
+{
+  const struct mfd_cell *mmsys_cells;
+  int nr_cells;
+  long id;
+  int ret;
+
+  id = (long) of_device_get_match_data(&pdev->dev);
+  if (!id) {
+          dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+          return -EINVAL;
+  }
+
+  switch (id) {
+  case MMSYS_MT2701:
+          mmsys_cells = mmsys_mt2701_devs;
+          nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+          break;
+  default:
+          return -ENODEV;
+  }

drivers/mfd/mtk-mmsys.c line 59

Quoted 10 lines

drivers/mfd/mtk-mmsys.c new file +10 -0

+
+  ret = devm_mfd_add_devices(&pdev->dev, 0, mmsys_cells, nr_cells,
+                                  NULL, 0, NULL);
+  if (ret) {
+          dev_err(&pdev->dev, "failed to add MFD devices %d\n", ret);
+          return ret;
+  }
+
+  return 0;
+};

This driver is pretty pointless. It doesn't actually do anything.

I think you just want to use "simple-mfd" instead.

Quoted 7 lines

Quoted 3 lines

I think the problem is, that right now we have two drivers which use the same devicetree binding, which are clk and drm driver. With a simple-mfd we would need two compatibles, and this would break backwards compatibility.

So what functionality does this driver provide you with that you do not have currently?

I'm not sure if I get your question. Point is, that the MMSYS implementation for mt8173 is broken, as it assumes that we can probe two drivers with the mediatek,mt8173-mmsys compatible. Somehow it used to work, but from what I understand it was a bug. So older devicetrees use just on mt8173-mmsys compatible in ther DTB.

I would like to keep backwards compatibility for the device tree, that's why I was searching for a solution where we can probe two drivers and came up with this mfd solution.

So no new functionality, the clk driver provides the clock the drm components need.

Regards,
Matthias

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 133 lines · Matthias Brugger

Quoted 122 lines · Lee Jones

Quoted 121 lines · Matthias Brugger

Quoted 119 lines · Lee Jones

Quoted 102 lines · On Fri, 27 Apr 2018, [email protected] wrote:

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mfd/mtk-mmsys.c

drivers/mfd/Kconfig +9 -0

index b860eb5aa194..d23a3b9a2c58 100644
@@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
 	help
 	  Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MEDIATEK_MMSYS
+	tristate "Mediatek MMSYS interface"
+	select MFD_CORE
+	select REGMAP_MMIO
+	help
+	  Select this if you have a MMSYS subsystem in your SoC. The
+	  MMSYS subsystem has at least a clock driver part and some
+	  DRM components.
+
 config MFD_MXS_LRADC
 	tristate "Freescale i.MX23/i.MX28 LRADC"
 	depends on ARCH_MXS || COMPILE_TEST

drivers/mfd/Makefile +2 -0

index d9d2cf0d32ef..b96118bd68d9 100644
@@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)		+= ezx-pcap.o

drivers/mfd/mtk-mmsys.c new file +49 -0

new file mode 100644
index 000000000000..c802343fb1c6
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
+ *
+ * Copyright (c) 2018 Matthias Brugger <[email protected]>
+ *
+ * Author: Matthias Brugger <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum {
+	MMSYS_MT2701 = 1,
+};
+
+static const struct mfd_cell mmsys_mt2701_devs[] = {
+	{ .name = "clk-mt2701-mm", },
+	{ .name = "drm-mt2701-mm", },
+};
+
+static int mmsys_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *mmsys_cells;
+	int nr_cells;
+	long id;
+	int ret;
+
+	id = (long) of_device_get_match_data(&pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+		return -EINVAL;
+	}
+
+	switch (id) {
+	case MMSYS_MT2701:
+		mmsys_cells = mmsys_mt2701_devs;
+		nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+		break;
+	default:
+		return -ENODEV;
+	}

drivers/mfd/mtk-mmsys.c line 59

Quoted 10 lines

drivers/mfd/mtk-mmsys.c new file +10 -0

+
+	ret = devm_mfd_add_devices(&pdev->dev, 0, mmsys_cells, nr_cells,
+					NULL, 0, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add MFD devices %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};

This driver is pretty pointless. It doesn't actually do anything.

I think you just want to use "simple-mfd" instead.

Quoted 7 lines

Quoted 3 lines

I think the problem is, that right now we have two drivers which use the same devicetree binding, which are clk and drm driver. With a simple-mfd we would need two compatibles, and this would break backwards compatibility.

So what functionality does this driver provide you with that you do not have currently?

Quoted 5 lines

I'm not sure if I get your question. Point is, that the MMSYS implementation for mt8173 is broken, as it assumes that we can probe two drivers with the mediatek,mt8173-mmsys compatible. Somehow it used to work, but from what I understand it was a bug. So older devicetrees use just on mt8173-mmsys compatible in ther DTB.

Okay, that is what I was getting at. Thanks for the explanation.

Do you have a datasheet I can look at?

Quoted 5 lines

I would like to keep backwards compatibility for the device tree, that's why I was searching for a solution where we can probe two drivers and came up with this mfd solution.

So no new functionality, the clk driver provides the clock the drm components need.

Quoted 138 lines · Lee Jones

Quoted 131 lines · Matthias Brugger

Quoted 122 lines · Lee Jones

Quoted 121 lines · Matthias Brugger

Quoted 119 lines · Lee Jones

Quoted 102 lines · matthias.bgg at kernel.org

From: Matthias Brugger <[email protected]>

The MMSYS subsystem includes clocks and drm components. This patch adds a MFD device to probe both drivers from the same device tree compatible.

Signed-off-by Matthias Brugger <[email protected]>

---
 drivers/mfd/Kconfig     |  9 ++++++
 drivers/mfd/Makefile    |  2 ++
 drivers/mfd/mtk-mmsys.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mfd/mtk-mmsys.c

drivers/mfd/Kconfig +9 -0

index b860eb5aa194..d23a3b9a2c58 100644
@@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
 	help
 	  Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MEDIATEK_MMSYS
+	tristate "Mediatek MMSYS interface"
+	select MFD_CORE
+	select REGMAP_MMIO
+	help
+	  Select this if you have a MMSYS subsystem in your SoC. The
+	  MMSYS subsystem has at least a clock driver part and some
+	  DRM components.
+
 config MFD_MXS_LRADC
 	tristate "Freescale i.MX23/i.MX28 LRADC"
 	depends on ARCH_MXS || COMPILE_TEST

drivers/mfd/Makefile +2 -0

index d9d2cf0d32ef..b96118bd68d9 100644
@@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)		+= ezx-pcap.o

drivers/mfd/mtk-mmsys.c new file +49 -0

new file mode 100644
index 000000000000..c802343fb1c6
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
+ *
+ * Copyright (c) 2018 Matthias Brugger <[email protected]>
+ *
+ * Author: Matthias Brugger <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum {
+	MMSYS_MT2701 = 1,
+};
+
+static const struct mfd_cell mmsys_mt2701_devs[] = {
+	{ .name = "clk-mt2701-mm", },
+	{ .name = "drm-mt2701-mm", },
+};
+
+static int mmsys_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *mmsys_cells;
+	int nr_cells;
+	long id;
+	int ret;
+
+	id = (long) of_device_get_match_data(&pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+		return -EINVAL;
+	}
+
+	switch (id) {
+	case MMSYS_MT2701:
+		mmsys_cells = mmsys_mt2701_devs;
+		nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+		break;
+	default:
+		return -ENODEV;
+	}

drivers/mfd/mtk-mmsys.c line 59

Quoted 10 lines

drivers/mfd/mtk-mmsys.c new file +10 -0

+
+	ret = devm_mfd_add_devices(&pdev->dev, 0, mmsys_cells, nr_cells,
+					NULL, 0, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add MFD devices %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};

This driver is pretty pointless. It doesn't actually do anything.

I think you just want to use "simple-mfd" instead.

Quoted 7 lines

Quoted 3 lines

I think the problem is, that right now we have two drivers which use the same devicetree binding, which are clk and drm driver. With a simple-mfd we would need two compatibles, and this would break backwards compatibility.

So what functionality does this driver provide you with that you do not have currently?

Quoted 5 lines

I'm not sure if I get your question. Point is, that the MMSYS implementation for mt8173 is broken, as it assumes that we can probe two drivers with the mediatek,mt8173-mmsys compatible. Somehow it used to work, but from what I understand it was a bug. So older devicetrees use just on mt8173-mmsys compatible in ther DTB.

Quoted 4 lines

Okay, that is what I was getting at. Thanks for the explanation.

Do you have a datasheet I can look at?

Unfortunately there is no datasheet you can get without a NDA. The only public available information I'm aware of is for the (not upstream supported) 96board [1]. And it's only the register description. You can find some more explanation about the MMSYS in older threads which tried to solve the very same problem [2]

[1]
https://www.96boards.org/documentation/consumer/mediatekx20/additional-docs/docs/MT6797_Register_Table_Part_2.pdf
[2] http://lists.infradead.org/pipermail/linux-mediatek/2017-October/010979.html

Quoted 6 lines

Quoted 5 lines

I would like to keep backwards compatibility for the device tree, that's why I was searching for a solution where we can probe two drivers and came up with this mfd solution.

So no new functionality, the clk driver provides the clock the drm components need.

Quoted 6 lines · Matthias Brugger

Quoted 5 lines · Lee Jones

Quoted 4 lines · Matthias Brugger

Quoted 3 lines · Lee Jones

Quoted 2 lines · Matthias Brugger

Quoted 1 line · Lee Jones

On Fri, 27 Apr 2018, [email protected] wrote:

Quoted 56 lines

Quoted 51 lines

Quoted 45 lines

Quoted 37 lines

Quoted 37 lines

Quoted 36 lines

Quoted 21 lines

(quoted diff) +21 -0

+static int mmsys_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *mmsys_cells;
+	int nr_cells;
+	long id;
+	int ret;
+
+	id = (long) of_device_get_match_data(&pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "of_device_get match_data() failed\n");
+		return -EINVAL;
+	}
+
+	switch (id) {
+	case MMSYS_MT2701:
+		mmsys_cells = mmsys_mt2701_devs;
+		nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
+		break;
+	default:
+		return -ENODEV;
+	}

Review

Quoted 10 lines

(quoted diff) +10 -0

+
+	ret = devm_mfd_add_devices(&pdev->dev, 0, mmsys_cells, nr_cells,
+					NULL, 0, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add MFD devices %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};

This driver is pretty pointless. It doesn't actually do anything.

I think you just want to use "simple-mfd" instead.

Quoted 7 lines

Quoted 3 lines

I think the problem is, that right now we have two drivers which use the same devicetree binding, which are clk and drm driver. With a simple-mfd we would need two compatibles, and this would break backwards compatibility.

So what functionality does this driver provide you with that you do not have currently?

Quoted 5 lines

I'm not sure if I get your question. Point is, that the MMSYS implementation for mt8173 is broken, as it assumes that we can probe two drivers with the mediatek,mt8173-mmsys compatible. Somehow it used to work, but from what I understand it was a bug. So older devicetrees use just on mt8173-mmsys compatible in ther DTB.

Quoted 4 lines

Okay, that is what I was getting at. Thanks for the explanation.

Do you have a datasheet I can look at?

Quoted 8 lines

Unfortunately there is no datasheet you can get without a NDA. The only public available information I'm aware of is for the (not upstream supported) 96board [1]. And it's only the register description. You can find some more explanation about the MMSYS in older threads which tried to solve the very same problem [2]

[1]
https://www.96boards.org/documentation/consumer/mediatekx20/additional-docs/docs/MT6797_Register_Table_Part_2.pdf
[2] http://lists.infradead.org/pipermail/linux-mediatek/2017-October/010979.html

And now we have some history. Thanks for providing those.

So my initial reaction is the one I have just confirmed for myself. MFD is being used as a work-around in this odd use-case and really isn't the right place for it.

If you're adamant that you do not wish to change the Device Tree, my suggestion is to either move the clk functionality into the DRM driver, or register the clk driver from DRM using standard platform_device_register() API.