Use SRC to reset IP modules on i.MX5 and i.MX6

The system reset controller (SRC) on i.MX51, i.MX53, and i.MX6q controls reset lines to the GPU, VPU, IPU, and OpenVG IP modules.

The following patches allow to link modules and SRC reset lines via phandle properties in the device tree. Drivers can then request their IP modules to be reset by the SRC.
The IPU driver is made to use this mechanism during initialization, to improve robustness in the case of bootloaders leaving the IPU enabled.

Fabio, does this help with your spurious interrupt issue?

regards
Philipp

---
 .../devicetree/bindings/reset/fsl,imx-src.txt      |   45 +++++++++++++++++
 .../bindings/staging/imx-drm/fsl-imx-drm.txt       |    3 ++
 arch/arm/boot/dts/imx51.dtsi                       |    6 +++
 arch/arm/boot/dts/imx53.dtsi                       |    9 ++++
 arch/arm/boot/dts/imx6q.dtsi                       |    3 ++
 arch/arm/mach-imx/Kconfig                          |    1 +
 arch/arm/mach-imx/common.h                         |    3 +-
 arch/arm/mach-imx/mach-imx6q.c                     |    2 +-
 arch/arm/mach-imx/mm-imx5.c                        |    2 +
 arch/arm/mach-imx/src.c                            |   52 +++++++++++++++++++-
 drivers/staging/imx-drm/ipu-v3/ipu-common.c        |   20 ++++++--
 include/linux/imx-src.h                            |    6 +++
 12 files changed, 146 insertions(+), 6 deletions(-) 

---
 .../devicetree/bindings/reset/fsl,imx-src.txt      |   45 ++++++++++++++++++++
 arch/arm/mach-imx/src.c                            |   41 ++++++++++++++++++
 include/linux/imx-src.h                            |    6 +++
 3 files changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx-src.txt
 create mode 100644 include/linux/imx-src.h

new file mode 100644
index 0000000..8f1e66a
@@ -0,0 +1,45 @@
+Freescale i.MX System Reset Controller
+======================================
+
+Required properties:
+- compatible: Should be "fsl,<chip>-src"
+- reg: should be register base and length as documented in the
+  datasheet
+- interrupts: Should contain SRC interrupt and CPU WDOG interrupt,
+  in this order.
+- #reset-cells: 1, see below
+
+example:
+
+src: src at 020d8000 {
+        compatible = "fsl,imx6q-src";
+        reg = <0x020d8000 0x4000>;
+        interrupts = <0 91 0x04 0 96 0x04>;
+        #reset-cells = <1>;
+};
+
+Specifying reset lines connected to IP modules
+==============================================
+
+The system reset controller can be used to reset the GPU, VPU,
+IPU, and OpenVG IP modules on i.MX5 and i.MX6 ICs. Those device
+nodes should specify the reset line on the SRC in their reset
+property, containing a phandle to the SRC device node and a
+RESET_INDEX specifying which module to reset.
+
+example:
+
+        ipu0: ipu0 {
+                reset = <&src 2>;
+        };
+        ipu1: ipu1 {
+                reset = <&src 4>;
+        };
+
+The following RESET_INDEX values are valid for i.MX5:
+GPU_RESET     0
+VPU_RESET     1
+IPU1_RESET    2
+OPEN_VG_RESET 3
+The following additional RESET_INDEX value is valid for i.MX6:
+IPU2_RESET    4

new file mode 100644
index 0000000..b93ed96
@@ -0,0 +1,6 @@
+#ifndef __IMX_SRC_H__
+#define __IMX_SRC_H__
+
+extern int imx_src_reset(int sw_reset_idx);
+
+#endif /* __IMX_SRC_H__ */

1.7.10.4

I proposed something very similar a while back; it may be useful to look at the previous discussion there:

http://www.spinics.net/lists/arm-kernel/msg202451.html

Unfortunately, I haven't had time to follow up on the proposal.

Hi Stephen,

Am Mittwoch, den 09.01.2013, 11:15 -0700 schrieb Stephen Warren:

Thanks, I had started playing around with some "reset subsystem" patches, but so far shied away from what feels like a whole lot of code for little effect. I came to the same conclusion that replacing gpio resets would probably be overkill, but I was only peripherally aware of the tegra reset controller.
So a simple common struct and some oftree parsing code in drivers/reset that can replace both the proposed imx patch and what tegra_periph_reset_* do would be considered useful?

regards
Philipp

Yes, I didn't really imagine the "reset subsystem" would do more than parsing the DT property that represented the reset and look up the registered reset controller and reset ID. Plus, provide some central function to pass the parsed reset ID to the reset controller driver. Of course, if it expanded to cover non-DT cases it might get more complex, but that can always be added later.

Hi Philipp,

I'm with Stephen that we should try to create a common API for this. Instead of ask device drivers to call IMX specific imx_src_reset(), we should have them call some type of common API with struct device as the argument. And the work of calling of_parse_phandle_with_args() should be done inside the API. It's pointless to ask every single client driver to do this same work.

Shawn

Am Donnerstag, den 10.01.2013, 14:56 +0800 schrieb Shawn Guo:

Agreed. At least for devices with a single dedicated reset line this would be a much better API.

regards
Philipp

---
 arch/arm/boot/dts/imx6q.dtsi |    3 +++
 1 file changed, 3 insertions(+)

index d6265ca..aac017b 100644
@@ -555,6 +555,7 @@
 				compatible = "fsl,imx6q-src";
 				reg = <0x020d8000 0x4000>;
 				interrupts = <0 91 0x04 0 96 0x04>;
+				#reset-cells = <1>;
 			};
 
 			gpc: gpc at 020dc000 {

@@ -1046,6 +1047,7 @@
 			interrupts = <0 6 0x4 0 5 0x4>;
 			clocks = <&clks 130>, <&clks 131>, <&clks 132>;
 			clock-names = "bus", "di0", "di1";
+			reset = <&src 2>;
 		};
 
 		ipu2: ipu at 02800000 {

@@ -1055,6 +1057,7 @@
 			interrupts = <0 8 0x4 0 7 0x4>;
 			clocks = <&clks 133>, <&clks 134>, <&clks 137>;
 			clock-names = "bus", "di0", "di1";
+			reset = <&src 4>;
 		};
 	};
 };

1.7.10.4

Request the System Reset Controller to reset the IPU if specified via device tree phandle.

---
 .../bindings/staging/imx-drm/fsl-imx-drm.txt       |    3 +++
 drivers/staging/imx-drm/ipu-v3/ipu-common.c        |   20 +++++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

index 07654f0..e677f34 100644
@@ -8,6 +8,8 @@ Required properties:
 - interrupts: Should contain sync interrupt and error interrupt,
   in this order.
 - #crtc-cells: 1, See below
+- reset: phandle pointing to the system reset controller and
+         reset line index, see reset/fsl,src.txt for details
 
 example:
 

@@ -16,6 +18,7 @@ ipu: ipu at 18000000 {
 	compatible = "fsl,imx53-ipu";
 	reg = <0x18000000 0x080000000>;
 	interrupts = <11 10>;
+	reset = <&src 2>;
 };
 
 Parallel display support

index f7059cd..be972bf 100644
@@ -28,6 +28,8 @@
 #include <linux/of_device.h>
 #include <asm/mach/irq.h>
 
+#include <linux/imx-src.h>
+
 #include "imx-ipu-v3.h"
 #include "ipu-prv.h"
 

@@ -660,7 +662,7 @@ int ipu_idmac_disable_channel(struct ipuv3_channel *channel)
 }
 EXPORT_SYMBOL_GPL(ipu_idmac_disable_channel);
 
-static int ipu_reset(struct ipu_soc *ipu)
+static int ipu_memory_reset(struct ipu_soc *ipu)
 {
 	unsigned long timeout;
 

@@ -1022,6 +1024,7 @@ static int ipu_probe(struct platform_device *pdev)
 	unsigned long ipu_base;
 	int i, ret, irq_sync, irq_err;
 	const struct ipu_devtype *devtype;
+	struct of_phandle_args args;
 
 	devtype = of_id->data;
 

@@ -1104,7 +1107,18 @@ static int ipu_probe(struct platform_device *pdev)
 	if (ret)
 		goto out_failed_irq;
 
-	ret = ipu_reset(ipu);
+	ret = of_parse_phandle_with_args(pdev->dev.of_node,
+					 "reset", "#reset-cells", 0, &args);
+	if (ret || args.args_count <= 0) {
+		dev_warn(&pdev->dev, "failed to get reset controller\n");
+	} else {
+		ret = imx_src_reset(args.args[0]);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to reset: %d\n", ret);
+			goto out_failed_reset;
+		}
+	}
+	ret = ipu_memory_reset(ipu);
 	if (ret)
 		goto out_failed_reset;
 

@@ -1130,8 +1144,8 @@ static int ipu_probe(struct platform_device *pdev)
 failed_add_clients:
 	ipu_submodules_exit(ipu);
 failed_submodules_init:
-	ipu_irq_exit(ipu);
 out_failed_reset:
+	ipu_irq_exit(ipu);
 out_failed_irq:
 	clk_disable_unprepare(ipu->clk);
 failed_clk_get:

1.7.10.4

Dear Philipp Zabel,

Does this apply to latest next please?

Best regards,
Marek Vasut

The SRC in i.MX51 and i.MX53 is similar to the one in i.MX6q minus the IPU2 reset line and multi core CPU reset/enable bits.

---
 arch/arm/mach-imx/Kconfig      |    1 +
 arch/arm/mach-imx/common.h     |    3 ++-
 arch/arm/mach-imx/mach-imx6q.c |    2 +-
 arch/arm/mach-imx/mm-imx5.c    |    2 ++
 arch/arm/mach-imx/src.c        |   11 ++++++++++-
 5 files changed, 16 insertions(+), 3 deletions(-)

index 3e628fd..d7924e5 100644
@@ -829,6 +829,7 @@ config	SOC_IMX53
 	select ARCH_MX53
 	select HAVE_CAN_FLEXCAN if CAN
 	select IMX_HAVE_PLATFORM_IMX2_WDT
+	select HAVE_IMX_SRC
 	select PINCTRL
 	select PINCTRL_IMX53
 	select SOC_IMX5

index 7191ab4..f36be3c 100644
@@ -133,7 +133,8 @@ static inline void imx_smp_prepare(void) {}
 #endif
 extern void imx_enable_cpu(int cpu, bool enable);
 extern void imx_set_cpu_jump(int cpu, void *jump_addr);
-extern void imx_src_init(void);
+extern void imx5_src_init(void);
+extern void imx6q_src_init(void);
 extern void imx_src_prepare_restart(void);
 extern void imx_gpc_init(void);
 extern void imx_gpc_pre_suspend(void);

index cd277a0..b1e076c 100644
@@ -229,7 +229,7 @@ static const struct of_device_id imx6q_irq_match[] __initconst = {
 static void __init imx6q_init_irq(void)
 {
 	l2x0_of_init(0, ~0UL);
-	imx_src_init();
+	imx6q_src_init();
 	imx_gpc_init();
 	of_irq_init(imx6q_irq_match);
 }

index 79d71cf..53f87be 100644
@@ -106,6 +106,7 @@ void __init imx51_init_early(void)
 	mxc_set_cpu_type(MXC_CPU_MX51);
 	mxc_iomux_v3_init(MX51_IO_ADDRESS(MX51_IOMUXC_BASE_ADDR));
 	mxc_arch_reset_init(MX51_IO_ADDRESS(MX51_WDOG1_BASE_ADDR));
+	imx5_src_init();
 }
 
 void __init imx53_init_early(void)

@@ -113,6 +114,7 @@ void __init imx53_init_early(void)
 	mxc_set_cpu_type(MXC_CPU_MX53);
 	mxc_iomux_v3_init(MX53_IO_ADDRESS(MX53_IOMUXC_BASE_ADDR));
 	mxc_arch_reset_init(MX53_IO_ADDRESS(MX53_WDOG1_BASE_ADDR));
+	imx5_src_init();
 }
 
 void __init mx50_init_irq(void)

index 10658b4..7dbc08cb 100644
@@ -97,7 +97,16 @@ void imx_src_prepare_restart(void)
 	writel_relaxed(0, src_base + SRC_GPR1);
 }
 
-void __init imx_src_init(void)
+void __init imx5_src_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx5-src");
+	src_base = of_iomap(np, 0);
+	WARN_ON(!src_base);
+}
+
+void __init imx6q_src_init(void)
 {
 	struct device_node *np;
 	u32 val;

1.7.10.4

Also, link SRC to IPU via phandle.

---
 arch/arm/boot/dts/imx51.dtsi |    6 ++++++
 arch/arm/boot/dts/imx53.dtsi |    9 +++++++++
 2 files changed, 15 insertions(+)

index 1f5d45e..b3e9742 100644
@@ -448,6 +448,12 @@
 				status = "disabled";
 			};
 
+			src: src at 73fd0000 {
+				compatible = "fsl,imx5-src";
+				reg = <0x73fd0000 0x4000>;
+				#reset-cells = <1>;
+			};
+
 			clks: ccm at 73fd4000{
 				compatible = "fsl,imx51-ccm";
 				reg = <0x73fd4000 0x4000>;

index edc3f1e..7266942 100644
@@ -72,6 +72,9 @@
 			compatible = "fsl,imx53-ipu";
 			reg = <0x18000000 0x080000000>;
 			interrupts = <11 10>;
+			clocks = <&clks 59>, <&clks 110>, <&clks 61>;
+			clock-names = "bus", "di0", "di1";
+			reset = <&src 2>;
 		};
 
 		aips at 50000000 { /* AIPS1 */

@@ -497,6 +500,12 @@
 				status = "disabled";
 			};
 
+			src: src at 53fd0000 {
+				compatible = "fsl,imx5-src";
+				reg = <0x53fd0000 0x4000>;
+				#reset-cells = <1>;
+			};
+
 			clks: ccm at 53fd4000{
 				compatible = "fsl,imx53-ccm";
 				reg = <0x53fd4000 0x4000>;

1.7.10.4

I would like to give it a trym but I got some errors when trying to apply these patches.

Which git tree should I use, or maybe do you have a git tree with them applied so that I can quickly try?

Thanks,

Fabio Estevam

Hi Fabio,

Am Mittwoch, den 09.01.2013, 15:33 -0200 schrieb Fabio Estevam:

I rebased them onto next-20130109 (with this base compile-tested only).

regards
Philipp

Just tested it, but unfortunately I still get the crash.

Ok, since I am on mx51 I just added a:

index 1f5d45e..8380b14 100644
@@ -67,6 +67,7 @@
                        compatible = "fsl,imx51-ipu";
                        reg = <0x40000000 0x20000000>;
                        interrupts = <11 10>;
+                       reset = <&src 2>;
                };
 

and now it works fine.

Thanks,

Fabio Estevam

Am Mittwoch, den 09.01.2013, 15:57 -0200 schrieb Fabio Estevam:

Thanks, sorry for missing the essential part for i.MX51.
So it seems hooking up the SRC to a common peripheral device reset call is the way to go.

regards
Philipp