All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] provide imx rproc driver
@ 2017-07-10 14:42 ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-10 14:42 UTC (permalink / raw)
  To: devicetree, kernel, linux-arm-kernel, linux-kernel, Rob Herring,
	Mark Rutland, Russell King, Shawn Guo, Fabio Estevam,
	Ohad Ben-Cohen, Bjorn Andersson, linux-remoteproc, linux
  Cc: Oleksij Rempel

this patch set to provide remoteproc functionality on
i.MX7D SoC.

For testing I used this simple counter written in ASM:
======================================
        .syntax unified
        .text
        .thumb
        .int 0x10020000                 @ Initial SP value
        .int reset + 1

reset:

        mov     r0, #0x55
        ldr     r1, =(0x40)
1:
        str     r0, [r1]
        add     r0, 1
        b       1b

        /* Dummy data, required by remoteproc loader */
        /* Please FIXME, this part seem to be incorrect */
        .data
        .section .resource_table, "aw"
        .word   1, 0, 0, 0      /* struct resource_table base */
        .word   0               /* uint32_t offset[1] */
============================================================
compiled with:
${CROSS}as -o imx7m4.o imx7m4.S
${CROSS}ld -Ttext=0x0 -o imx7m4.elf imx7m4.o
cp imx7m4.elf /srv/nfs/sid-armhf/lib/firmware/rproc-imx_rproc-fw

Functionality was confirmed with current OpenOCD master.
OpenOCD cfg file can be found here:
https://github.com/olerem/openocd/blob/imx7-2017.06.14/tcl/target/imx7.cfg

Oleksij Rempel (2):
  remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor
    Controller driver
  remoteproc: imx_rproc: add a NXP/Freescale imx_rproc driver

 .../devicetree/bindings/remoteproc/imx-rproc.txt   |  44 +++
 drivers/remoteproc/Kconfig                         |  10 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/imx_rproc.c                     | 417 +++++++++++++++++++++
 4 files changed, 472 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
 create mode 100644 drivers/remoteproc/imx_rproc.c

-- 
2.11.0

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 0/2] provide imx rproc driver
@ 2017-07-10 14:42 ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-10 14:42 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Russell King, Shawn Guo, Fabio Estevam, Ohad Ben-Cohen,
	Bjorn Andersson, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linux-YEK0n+YFykbzxQdaRaTXBw
  Cc: Oleksij Rempel

this patch set to provide remoteproc functionality on
i.MX7D SoC.

For testing I used this simple counter written in ASM:
======================================
        .syntax unified
        .text
        .thumb
        .int 0x10020000                 @ Initial SP value
        .int reset + 1

reset:

        mov     r0, #0x55
        ldr     r1, =(0x40)
1:
        str     r0, [r1]
        add     r0, 1
        b       1b

        /* Dummy data, required by remoteproc loader */
        /* Please FIXME, this part seem to be incorrect */
        .data
        .section .resource_table, "aw"
        .word   1, 0, 0, 0      /* struct resource_table base */
        .word   0               /* uint32_t offset[1] */
============================================================
compiled with:
${CROSS}as -o imx7m4.o imx7m4.S
${CROSS}ld -Ttext=0x0 -o imx7m4.elf imx7m4.o
cp imx7m4.elf /srv/nfs/sid-armhf/lib/firmware/rproc-imx_rproc-fw

Functionality was confirmed with current OpenOCD master.
OpenOCD cfg file can be found here:
https://github.com/olerem/openocd/blob/imx7-2017.06.14/tcl/target/imx7.cfg

Oleksij Rempel (2):
  remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor
    Controller driver
  remoteproc: imx_rproc: add a NXP/Freescale imx_rproc driver

 .../devicetree/bindings/remoteproc/imx-rproc.txt   |  44 +++
 drivers/remoteproc/Kconfig                         |  10 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/imx_rproc.c                     | 417 +++++++++++++++++++++
 4 files changed, 472 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
 create mode 100644 drivers/remoteproc/imx_rproc.c

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 0/2] provide imx rproc driver
@ 2017-07-10 14:42 ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-10 14:42 UTC (permalink / raw)
  To: linux-arm-kernel

this patch set to provide remoteproc functionality on
i.MX7D SoC.

For testing I used this simple counter written in ASM:
======================================
        .syntax unified
        .text
        .thumb
        .int 0x10020000                 @ Initial SP value
        .int reset + 1

reset:

        mov     r0, #0x55
        ldr     r1, =(0x40)
1:
        str     r0, [r1]
        add     r0, 1
        b       1b

        /* Dummy data, required by remoteproc loader */
        /* Please FIXME, this part seem to be incorrect */
        .data
        .section .resource_table, "aw"
        .word   1, 0, 0, 0      /* struct resource_table base */
        .word   0               /* uint32_t offset[1] */
============================================================
compiled with:
${CROSS}as -o imx7m4.o imx7m4.S
${CROSS}ld -Ttext=0x0 -o imx7m4.elf imx7m4.o
cp imx7m4.elf /srv/nfs/sid-armhf/lib/firmware/rproc-imx_rproc-fw

Functionality was confirmed with current OpenOCD master.
OpenOCD cfg file can be found here:
https://github.com/olerem/openocd/blob/imx7-2017.06.14/tcl/target/imx7.cfg

Oleksij Rempel (2):
  remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor
    Controller driver
  remoteproc: imx_rproc: add a NXP/Freescale imx_rproc driver

 .../devicetree/bindings/remoteproc/imx-rproc.txt   |  44 +++
 drivers/remoteproc/Kconfig                         |  10 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/imx_rproc.c                     | 417 +++++++++++++++++++++
 4 files changed, 472 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
 create mode 100644 drivers/remoteproc/imx_rproc.c

-- 
2.11.0

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
  2017-07-10 14:42 ` Oleksij Rempel
@ 2017-07-10 14:42   ` Oleksij Rempel
  -1 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-10 14:42 UTC (permalink / raw)
  To: devicetree, kernel, linux-arm-kernel, linux-kernel, Rob Herring,
	Mark Rutland, Russell King, Shawn Guo, Fabio Estevam,
	Ohad Ben-Cohen, Bjorn Andersson, linux-remoteproc, linux
  Cc: Oleksij Rempel

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt

diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
new file mode 100644
index 000000000000..e7c61993e1b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
@@ -0,0 +1,44 @@
+NXP iMX6SX/iMX7D Co-Processor Bindings
+----------------------------------------
+
+This binding provides support for ARM Cortex M4 Co-processor found on some
+NXP iMX SoCs.
+
+Required properties:
+- compatible		Should be one of:
+				"fsl,imx7d-rproc"
+				"fsl,imx6sx-rproc"
+- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
+- syscfg		Phandle to syscon block which provide access to
+			System Reset Controller
+
+Optional properties:
+- reg:			Should contain the address ranges for some of internal
+			memory regions.
+- reg-names:		Contains the corresponding names for the memory
+			regions. These should be named "ddr", "ocram", "ocram-s",
+			"ocram-epdc" or "ocram-pxp".
+Fallowing memory ranges are expected:
+- For "fsl,imx7d-rproc"
+	<0x00900000 0x00020000> - "ocram"
+	<0x00920000 0x00020000> - "ocram-epdc"
+	<0x00940000 0x00008000> - "ocram-pxp"
+	<0x80000000 0x0FFF0000> - "ddr" (code area)
+	<0x80000000 0x60000000> - "ddr" (data area)
+- For "fsl,imx6sx-rproc"
+	<0x008F8000 0x00004000> - "ocram-s"
+	<0x80000000 0x0FFF8000> - "ddr" (code area)
+	<0x80000000 0x60000000> - "ddr" (data area)
+
+Note: the "ddr" code and data ranges are overlapping. Code area is smaller
+than data area.  So this range should be carefully chosen according to your
+system and application requirements.
+
+Example:
+	imx_rproc: imx7d-rp0@0 {
+		compatible	= "fsl,imx7d-rproc";
+		reg		= <0x80000000 0x80000>, <0x00900000 0x2000>;
+		reg-names	= "ddr", "ocram";
+		syscon		= <&src>;
+		clocks		= <&clks IMX7D_ARM_M4_ROOT_CLK>;
+	};
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-10 14:42   ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-10 14:42 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt

diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
new file mode 100644
index 000000000000..e7c61993e1b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
@@ -0,0 +1,44 @@
+NXP iMX6SX/iMX7D Co-Processor Bindings
+----------------------------------------
+
+This binding provides support for ARM Cortex M4 Co-processor found on some
+NXP iMX SoCs.
+
+Required properties:
+- compatible		Should be one of:
+				"fsl,imx7d-rproc"
+				"fsl,imx6sx-rproc"
+- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
+- syscfg		Phandle to syscon block which provide access to
+			System Reset Controller
+
+Optional properties:
+- reg:			Should contain the address ranges for some of internal
+			memory regions.
+- reg-names:		Contains the corresponding names for the memory
+			regions. These should be named "ddr", "ocram", "ocram-s",
+			"ocram-epdc" or "ocram-pxp".
+Fallowing memory ranges are expected:
+- For "fsl,imx7d-rproc"
+	<0x00900000 0x00020000> - "ocram"
+	<0x00920000 0x00020000> - "ocram-epdc"
+	<0x00940000 0x00008000> - "ocram-pxp"
+	<0x80000000 0x0FFF0000> - "ddr" (code area)
+	<0x80000000 0x60000000> - "ddr" (data area)
+- For "fsl,imx6sx-rproc"
+	<0x008F8000 0x00004000> - "ocram-s"
+	<0x80000000 0x0FFF8000> - "ddr" (code area)
+	<0x80000000 0x60000000> - "ddr" (data area)
+
+Note: the "ddr" code and data ranges are overlapping. Code area is smaller
+than data area.  So this range should be carefully chosen according to your
+system and application requirements.
+
+Example:
+	imx_rproc: imx7d-rp0 at 0 {
+		compatible	= "fsl,imx7d-rproc";
+		reg		= <0x80000000 0x80000>, <0x00900000 0x2000>;
+		reg-names	= "ddr", "ocram";
+		syscon		= <&src>;
+		clocks		= <&clks IMX7D_ARM_M4_ROOT_CLK>;
+	};
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v1 2/2] remoteproc: imx_rproc: add a NXP/Freescale imx_rproc driver
@ 2017-07-10 14:42   ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-10 14:42 UTC (permalink / raw)
  To: devicetree, kernel, linux-arm-kernel, linux-kernel, Rob Herring,
	Mark Rutland, Russell King, Shawn Guo, Fabio Estevam,
	Ohad Ben-Cohen, Bjorn Andersson, linux-remoteproc, linux
  Cc: Oleksij Rempel

Provide a basic driver to control Cortex M4 co-processor found
on NXP i.MX7D and i.MX6SX.
Currently it is able to resolve addresses between M4 and main CPU,
start and stop the co-processor. Other functionality is not provided
or test.

This driver was tested on NXP i.MX7D and expected to work on
i.MX6SX as well.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/remoteproc/Kconfig     |  10 +
 drivers/remoteproc/Makefile    |   1 +
 drivers/remoteproc/imx_rproc.c | 417 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 428 insertions(+)
 create mode 100644 drivers/remoteproc/imx_rproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index faad69a1a597..1f6644908629 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -13,6 +13,16 @@ config REMOTEPROC
 
 if REMOTEPROC
 
+config IMX_REMOTEPROC
+	tristate "IMX6/7 remoteproc support"
+	depends on SOC_IMX6SX || SOC_IMX7D
+	depends on REMOTEPROC
+	help
+	  Say y here to support iMX's remote processors (Cortex M4
+	  on iMX7D) via the remote processor framework.
+
+	  It's safe to say N here.
+
 config OMAP_REMOTEPROC
 	tristate "OMAP remoteproc support"
 	depends on HAS_DMA
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ffc5e430df27..d351c25cdb4e 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -19,3 +19,4 @@ qcom_wcnss_pil-y			+= qcom_wcnss.o
 qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
+obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
new file mode 100644
index 000000000000..2a5300c3323c
--- /dev/null
+++ b/drivers/remoteproc/imx_rproc.c
@@ -0,0 +1,417 @@
+/*
+ * Copyright (c) 2017 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/remoteproc.h>
+
+#define IMX7D_SRC_SCR			0x0C
+#define IMX7D_ENABLE_M4			BIT(3)
+#define IMX7D_SW_M4P_RST		BIT(2)
+#define IMX7D_SW_M4C_RST		BIT(1)
+#define IMX7D_SW_M4C_NON_SCLR_RST	BIT(0)
+
+#define IMX7D_M4_RST_MASK		(IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
+					 | IMX7D_SW_M4C_RST \
+					 | IMX7D_SW_M4C_NON_SCLR_RST)
+
+#define IMX7D_M4_START			(IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
+					 | IMX7D_SW_M4C_RST)
+#define IMX7D_M4_STOP			IMX7D_SW_M4C_NON_SCLR_RST
+
+/* Address: 0x020D8000 */
+#define IMX6SX_SRC_SCR			0x00
+#define IMX6SX_ENABLE_M4		BIT(22)
+#define IMX6SX_SW_M4P_RST		BIT(12)
+#define IMX6SX_SW_M4C_NON_SCLR_RST	BIT(4)
+#define IMX6SX_SW_M4C_RST		BIT(3)
+
+#define IMX6SX_M4_START			(IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
+					 | IMX6SX_SW_M4C_RST)
+#define IMX6SX_M4_STOP			IMX6SX_SW_M4C_NON_SCLR_RST
+#define IMX6SX_M4_RST_MASK		(IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
+					 | IMX6SX_SW_M4C_NON_SCLR_RST \
+					 | IMX6SX_SW_M4C_RST)
+
+#define IMX7D_RPROC_MEM_MAX		8
+
+/**
+ * struct imx_rproc_mem - slim internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @sys_addr: Bus address used to access the memory region
+ * @size: Size of the memory region
+ */
+struct imx_rproc_mem {
+	void __iomem *cpu_addr;
+	phys_addr_t sys_addr;
+	size_t size;
+};
+
+/* att flags */
+/* M4 own area. Can be mapped at probe */
+#define ATT_OWN		BIT(1)
+
+/* address translation table */
+struct imx_rproc_att {
+	u32 da;	/* device address (From Cortex M4 view)*/
+	u32 sa;	/* system bus address */
+	u32 size; /* size of reg range */
+	int flags;
+};
+
+struct imx_rproc_dcfg {
+	u32				src_reg;
+	u32				src_mask;
+	u32				src_start;
+	u32				src_stop;
+	const struct imx_rproc_att	*att;
+	size_t				att_size;
+};
+
+struct imx_rproc {
+	struct device			*dev;
+	struct regmap			*regmap;
+	struct rproc			*rproc;
+	const struct imx_rproc_dcfg	*dcfg;
+	struct imx_rproc_mem		mem[IMX7D_RPROC_MEM_MAX];
+	struct clk			*clk;
+};
+
+static const struct imx_rproc_att imx_rproc_att_imx7d[] = {
+	/* dev addr , sys addr  , size	    , flags */
+	/* OCRAM_S (M4 Boot code) - alias */
+	{ 0x00000000, 0x00180000, 0x00008000, 0 },
+	/* OCRAM_S (Code) */
+	{ 0x00180000, 0x00180000, 0x00008000, ATT_OWN },
+	/* OCRAM (Code) - alias */
+	{ 0x00900000, 0x00900000, 0x00020000, 0 },
+	/* OCRAM_EPDC (Code) - alias */
+	{ 0x00920000, 0x00920000, 0x00020000, 0 },
+	/* OCRAM_PXP (Code) - alias */
+	{ 0x00940000, 0x00940000, 0x00008000, 0 },
+	/* TCML (Code) */
+	{ 0x1FFF8000, 0x007F8000, 0x00008000, ATT_OWN },
+	/* DDR (Code) - alias, first part of DDR (Data) */
+	{ 0x10000000, 0x80000000, 0x0FFF0000, 0 },
+
+	/* TCMU (Data) */
+	{ 0x20000000, 0x00800000, 0x00008000, ATT_OWN },
+	/* OCRAM (Data) */
+	{ 0x20200000, 0x00900000, 0x00020000, 0 },
+	/* OCRAM_EPDC (Data) */
+	{ 0x20220000, 0x00920000, 0x00020000, 0 },
+	/* OCRAM_PXP (Data) */
+	{ 0x20240000, 0x00940000, 0x00008000, 0 },
+	/* DDR (Data) */
+	{ 0x80000000, 0x80000000, 0x60000000, 0 },
+};
+
+static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
+	/* dev addr , sys addr  , size	    , flags */
+	/* TCML (M4 Boot Code) - alias */
+	{ 0x00000000, 0x007F8000, 0x00008000, 0 },
+	/* OCRAM_S (Code) */
+	{ 0x00180000, 0x008F8000, 0x00004000, 0 },
+	/* OCRAM_S (Code) - alias */
+	{ 0x00180000, 0x008FC000, 0x00004000, 0 },
+	/* TCML (Code) */
+	{ 0x1FFF8000, 0x007F8000, 0x00008000, ATT_OWN },
+	/* DDR (Code) - alias, first part of DDR (Data) */
+	{ 0x10000000, 0x80000000, 0x0FFF8000, 0 },
+
+	/* TCMU (Data) */
+	{ 0x20000000, 0x00800000, 0x00008000, ATT_OWN },
+	/* OCRAM_S (Data) - alias? */
+	{ 0x208F8000, 0x008F8000, 0x00004000, 0 },
+	/* DDR (Data) */
+	{ 0x80000000, 0x80000000, 0x60000000, 0 },
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
+	.src_reg	= IMX7D_SRC_SCR,
+	.src_mask	= IMX7D_M4_RST_MASK,
+	.src_start	= IMX7D_M4_START,
+	.src_stop	= IMX7D_M4_STOP,
+	.att		= imx_rproc_att_imx7d,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
+	.src_reg	= IMX6SX_SRC_SCR,
+	.src_mask	= IMX6SX_M4_RST_MASK,
+	.src_start	= IMX6SX_M4_START,
+	.src_stop	= IMX6SX_M4_STOP,
+	.att		= imx_rproc_att_imx6sx,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
+};
+
+static int imx_rproc_start(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct device *dev = priv->dev;
+	int ret;
+
+	ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
+				 dcfg->src_mask, dcfg->src_start);
+	if (ret)
+		dev_err(dev, "Filed to enable M4!\n");
+
+	return ret;
+}
+
+static int imx_rproc_stop(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct device *dev = priv->dev;
+	int ret;
+
+	ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
+				 dcfg->src_mask, dcfg->src_stop);
+	if (ret)
+		dev_err(dev, "Filed to stop M4!\n");
+
+	return ret;
+}
+
+static int imx_rproc_da_to_sys(struct imx_rproc *priv, u64 da,
+			       int len, u64 *sys)
+{
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	int i;
+
+	/* parse address translation table */
+	for (i = 0; i < dcfg->att_size; i++) {
+		const struct imx_rproc_att *att = &dcfg->att[i];
+
+		if (da >= att->da && da + len < att->da + att->size) {
+			unsigned int offset = da - att->da;
+
+			*sys = att->sa + offset;
+			return 0;
+		}
+	}
+
+	dev_warn(priv->dev, "Translation filed: da = 0x%llx len = 0x%x\n",
+		 da, len);
+	return -ENOENT;
+}
+
+static void *imx_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct imx_rproc *priv = rproc->priv;
+	void *va = NULL;
+	u64 sys;
+	int i;
+
+	if (len <= 0)
+		return NULL;
+
+	/*
+	 * On device side we have many aliases, so we need to convert device
+	 * address (M4) to system bus address first.
+	 */
+	if (imx_rproc_da_to_sys(priv, da, len, &sys))
+		return NULL;
+
+	for (i = 0; i < IMX7D_RPROC_MEM_MAX; i++) {
+		if (sys >= priv->mem[i].sys_addr && sys + len <
+		    priv->mem[i].sys_addr +  priv->mem[i].size) {
+			unsigned int offset = sys - priv->mem[i].sys_addr;
+			/* __force to make sparse happy with type conversion */
+			va = (__force void *)(priv->mem[i].cpu_addr + offset);
+			break;
+		}
+	}
+
+	dev_dbg(&rproc->dev, "da = 0x%llx len = 0x%x va = 0x%p\n", da, len, va);
+
+	return va;
+}
+
+static const struct rproc_ops imx_rproc_ops = {
+	.start		= imx_rproc_start,
+	.stop		= imx_rproc_stop,
+	.da_to_va       = imx_rproc_da_to_va,
+};
+
+static int imx_rproc_addr_init(struct imx_rproc *priv,
+			       struct platform_device *pdev)
+{
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	static const char * const mem_names[] = { "ddr", "ocram", "ocram-s",
+		"ocram-epdc", "ocram-pxp" };
+	struct resource *res;
+	int a, b = 0, err;
+
+	/* remap required addresses */
+	for (a = 0; a < dcfg->att_size; a++) {
+		const struct imx_rproc_att *att = &dcfg->att[a];
+
+		if (!(att->flags & ATT_OWN))
+			continue;
+
+		if (b > IMX7D_RPROC_MEM_MAX)
+			break;
+
+		priv->mem[b].cpu_addr = devm_ioremap(&pdev->dev,
+						     att->sa, att->size);
+		if (IS_ERR(priv->mem[b].cpu_addr)) {
+			dev_err(&pdev->dev, "devm_ioremap_resource failed\n");
+			err = PTR_ERR(priv->mem[b].cpu_addr);
+			return err;
+		}
+		priv->mem[b].sys_addr = att->sa;
+		priv->mem[b].size = att->size;
+		b++;
+	}
+
+	/* remap optional addresses */
+	for (a = 0; a < ARRAY_SIZE(mem_names); a++) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[a]);
+		if (!res)
+			continue;
+
+		if (b > IMX7D_RPROC_MEM_MAX)
+			break;
+
+		priv->mem[b].cpu_addr = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->mem[b].cpu_addr)) {
+			dev_err(&pdev->dev, "devm_ioremap_resource failed\n");
+			err = PTR_ERR(priv->mem[b].cpu_addr);
+			return err;
+		}
+		priv->mem[b].sys_addr = res->start;
+		priv->mem[b].size = resource_size(res);
+		b++;
+	}
+
+	return 0;
+}
+
+static int imx_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct imx_rproc *priv;
+	struct rproc *rproc;
+	struct regmap_config config = { .name = "imx-rproc" };
+	const struct imx_rproc_dcfg *dcfg;
+	struct regmap *regmap;
+	int ret;
+
+	regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "failed to find syscon\n");
+		return PTR_ERR(regmap);
+	}
+	regmap_attach_dev(dev, regmap, &config);
+
+	/* set some other name then imx */
+	rproc = rproc_alloc(dev, "imx-rproc", &imx_rproc_ops,
+			    NULL, sizeof(*priv));
+	if (!rproc) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	dcfg = of_device_get_match_data(dev);
+	if (!dcfg)
+		return -EINVAL;
+
+	priv = rproc->priv;
+	priv->rproc = rproc;
+	priv->regmap = regmap;
+	priv->dcfg = dcfg;
+	priv->dev = dev;
+
+	dev_set_drvdata(dev, rproc);
+
+	ret = imx_rproc_addr_init(priv, pdev);
+	if (ret) {
+		dev_err(dev, "filed on imx_rproc_addr_init\n");
+		goto err_put_rproc;
+	}
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "Failed to get clock\n");
+		rproc_free(rproc);
+		return PTR_ERR(priv->clk);
+	}
+
+	/*
+	 * clk for M4 block including memory. Should be
+	 * enabled before .start for FW transfer.
+	 */
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(&rproc->dev, "Failed to enable clock\n");
+		rproc_free(rproc);
+		return ret;
+	}
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "rproc_add failed\n");
+		goto err_put_clk;
+	}
+
+	return ret;
+
+err_put_clk:
+	clk_disable_unprepare(priv->clk);
+err_put_rproc:
+	rproc_free(rproc);
+err:
+	return ret;
+}
+
+static int imx_rproc_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+	struct imx_rproc *priv = rproc->priv;
+
+	clk_disable_unprepare(priv->clk);
+	rproc_del(rproc);
+	rproc_free(rproc);
+
+	return 0;
+}
+
+static const struct of_device_id imx_rproc_of_match[] = {
+	{ .compatible = "fsl,imx7d-rproc", .data = &imx_rproc_cfg_imx7d },
+	{ .compatible = "fsl,imx6sx-rproc", .data = &imx_rproc_cfg_imx6sx },
+	{},
+};
+MODULE_DEVICE_TABLE(of, imx_rproc_of_match);
+
+static struct platform_driver imx_rproc_driver = {
+	.probe = imx_rproc_probe,
+	.remove = imx_rproc_remove,
+	.driver = {
+		.name = "imx-rproc",
+		.of_match_table = imx_rproc_of_match,
+	},
+};
+
+module_platform_driver(imx_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("IMX6SX/7D remote processor control driver");
+MODULE_AUTHOR("Oleksij Rempel <o.rempel@pengutronix.de>");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v1 2/2] remoteproc: imx_rproc: add a NXP/Freescale imx_rproc driver
@ 2017-07-10 14:42   ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-10 14:42 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Russell King, Shawn Guo, Fabio Estevam, Ohad Ben-Cohen,
	Bjorn Andersson, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linux-YEK0n+YFykbzxQdaRaTXBw
  Cc: Oleksij Rempel

Provide a basic driver to control Cortex M4 co-processor found
on NXP i.MX7D and i.MX6SX.
Currently it is able to resolve addresses between M4 and main CPU,
start and stop the co-processor. Other functionality is not provided
or test.

This driver was tested on NXP i.MX7D and expected to work on
i.MX6SX as well.

Signed-off-by: Oleksij Rempel <o.rempel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/remoteproc/Kconfig     |  10 +
 drivers/remoteproc/Makefile    |   1 +
 drivers/remoteproc/imx_rproc.c | 417 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 428 insertions(+)
 create mode 100644 drivers/remoteproc/imx_rproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index faad69a1a597..1f6644908629 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -13,6 +13,16 @@ config REMOTEPROC
 
 if REMOTEPROC
 
+config IMX_REMOTEPROC
+	tristate "IMX6/7 remoteproc support"
+	depends on SOC_IMX6SX || SOC_IMX7D
+	depends on REMOTEPROC
+	help
+	  Say y here to support iMX's remote processors (Cortex M4
+	  on iMX7D) via the remote processor framework.
+
+	  It's safe to say N here.
+
 config OMAP_REMOTEPROC
 	tristate "OMAP remoteproc support"
 	depends on HAS_DMA
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ffc5e430df27..d351c25cdb4e 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -19,3 +19,4 @@ qcom_wcnss_pil-y			+= qcom_wcnss.o
 qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
+obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
new file mode 100644
index 000000000000..2a5300c3323c
--- /dev/null
+++ b/drivers/remoteproc/imx_rproc.c
@@ -0,0 +1,417 @@
+/*
+ * Copyright (c) 2017 Pengutronix, Oleksij Rempel <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/remoteproc.h>
+
+#define IMX7D_SRC_SCR			0x0C
+#define IMX7D_ENABLE_M4			BIT(3)
+#define IMX7D_SW_M4P_RST		BIT(2)
+#define IMX7D_SW_M4C_RST		BIT(1)
+#define IMX7D_SW_M4C_NON_SCLR_RST	BIT(0)
+
+#define IMX7D_M4_RST_MASK		(IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
+					 | IMX7D_SW_M4C_RST \
+					 | IMX7D_SW_M4C_NON_SCLR_RST)
+
+#define IMX7D_M4_START			(IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
+					 | IMX7D_SW_M4C_RST)
+#define IMX7D_M4_STOP			IMX7D_SW_M4C_NON_SCLR_RST
+
+/* Address: 0x020D8000 */
+#define IMX6SX_SRC_SCR			0x00
+#define IMX6SX_ENABLE_M4		BIT(22)
+#define IMX6SX_SW_M4P_RST		BIT(12)
+#define IMX6SX_SW_M4C_NON_SCLR_RST	BIT(4)
+#define IMX6SX_SW_M4C_RST		BIT(3)
+
+#define IMX6SX_M4_START			(IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
+					 | IMX6SX_SW_M4C_RST)
+#define IMX6SX_M4_STOP			IMX6SX_SW_M4C_NON_SCLR_RST
+#define IMX6SX_M4_RST_MASK		(IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
+					 | IMX6SX_SW_M4C_NON_SCLR_RST \
+					 | IMX6SX_SW_M4C_RST)
+
+#define IMX7D_RPROC_MEM_MAX		8
+
+/**
+ * struct imx_rproc_mem - slim internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @sys_addr: Bus address used to access the memory region
+ * @size: Size of the memory region
+ */
+struct imx_rproc_mem {
+	void __iomem *cpu_addr;
+	phys_addr_t sys_addr;
+	size_t size;
+};
+
+/* att flags */
+/* M4 own area. Can be mapped at probe */
+#define ATT_OWN		BIT(1)
+
+/* address translation table */
+struct imx_rproc_att {
+	u32 da;	/* device address (From Cortex M4 view)*/
+	u32 sa;	/* system bus address */
+	u32 size; /* size of reg range */
+	int flags;
+};
+
+struct imx_rproc_dcfg {
+	u32				src_reg;
+	u32				src_mask;
+	u32				src_start;
+	u32				src_stop;
+	const struct imx_rproc_att	*att;
+	size_t				att_size;
+};
+
+struct imx_rproc {
+	struct device			*dev;
+	struct regmap			*regmap;
+	struct rproc			*rproc;
+	const struct imx_rproc_dcfg	*dcfg;
+	struct imx_rproc_mem		mem[IMX7D_RPROC_MEM_MAX];
+	struct clk			*clk;
+};
+
+static const struct imx_rproc_att imx_rproc_att_imx7d[] = {
+	/* dev addr , sys addr  , size	    , flags */
+	/* OCRAM_S (M4 Boot code) - alias */
+	{ 0x00000000, 0x00180000, 0x00008000, 0 },
+	/* OCRAM_S (Code) */
+	{ 0x00180000, 0x00180000, 0x00008000, ATT_OWN },
+	/* OCRAM (Code) - alias */
+	{ 0x00900000, 0x00900000, 0x00020000, 0 },
+	/* OCRAM_EPDC (Code) - alias */
+	{ 0x00920000, 0x00920000, 0x00020000, 0 },
+	/* OCRAM_PXP (Code) - alias */
+	{ 0x00940000, 0x00940000, 0x00008000, 0 },
+	/* TCML (Code) */
+	{ 0x1FFF8000, 0x007F8000, 0x00008000, ATT_OWN },
+	/* DDR (Code) - alias, first part of DDR (Data) */
+	{ 0x10000000, 0x80000000, 0x0FFF0000, 0 },
+
+	/* TCMU (Data) */
+	{ 0x20000000, 0x00800000, 0x00008000, ATT_OWN },
+	/* OCRAM (Data) */
+	{ 0x20200000, 0x00900000, 0x00020000, 0 },
+	/* OCRAM_EPDC (Data) */
+	{ 0x20220000, 0x00920000, 0x00020000, 0 },
+	/* OCRAM_PXP (Data) */
+	{ 0x20240000, 0x00940000, 0x00008000, 0 },
+	/* DDR (Data) */
+	{ 0x80000000, 0x80000000, 0x60000000, 0 },
+};
+
+static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
+	/* dev addr , sys addr  , size	    , flags */
+	/* TCML (M4 Boot Code) - alias */
+	{ 0x00000000, 0x007F8000, 0x00008000, 0 },
+	/* OCRAM_S (Code) */
+	{ 0x00180000, 0x008F8000, 0x00004000, 0 },
+	/* OCRAM_S (Code) - alias */
+	{ 0x00180000, 0x008FC000, 0x00004000, 0 },
+	/* TCML (Code) */
+	{ 0x1FFF8000, 0x007F8000, 0x00008000, ATT_OWN },
+	/* DDR (Code) - alias, first part of DDR (Data) */
+	{ 0x10000000, 0x80000000, 0x0FFF8000, 0 },
+
+	/* TCMU (Data) */
+	{ 0x20000000, 0x00800000, 0x00008000, ATT_OWN },
+	/* OCRAM_S (Data) - alias? */
+	{ 0x208F8000, 0x008F8000, 0x00004000, 0 },
+	/* DDR (Data) */
+	{ 0x80000000, 0x80000000, 0x60000000, 0 },
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
+	.src_reg	= IMX7D_SRC_SCR,
+	.src_mask	= IMX7D_M4_RST_MASK,
+	.src_start	= IMX7D_M4_START,
+	.src_stop	= IMX7D_M4_STOP,
+	.att		= imx_rproc_att_imx7d,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
+	.src_reg	= IMX6SX_SRC_SCR,
+	.src_mask	= IMX6SX_M4_RST_MASK,
+	.src_start	= IMX6SX_M4_START,
+	.src_stop	= IMX6SX_M4_STOP,
+	.att		= imx_rproc_att_imx6sx,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
+};
+
+static int imx_rproc_start(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct device *dev = priv->dev;
+	int ret;
+
+	ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
+				 dcfg->src_mask, dcfg->src_start);
+	if (ret)
+		dev_err(dev, "Filed to enable M4!\n");
+
+	return ret;
+}
+
+static int imx_rproc_stop(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct device *dev = priv->dev;
+	int ret;
+
+	ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
+				 dcfg->src_mask, dcfg->src_stop);
+	if (ret)
+		dev_err(dev, "Filed to stop M4!\n");
+
+	return ret;
+}
+
+static int imx_rproc_da_to_sys(struct imx_rproc *priv, u64 da,
+			       int len, u64 *sys)
+{
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	int i;
+
+	/* parse address translation table */
+	for (i = 0; i < dcfg->att_size; i++) {
+		const struct imx_rproc_att *att = &dcfg->att[i];
+
+		if (da >= att->da && da + len < att->da + att->size) {
+			unsigned int offset = da - att->da;
+
+			*sys = att->sa + offset;
+			return 0;
+		}
+	}
+
+	dev_warn(priv->dev, "Translation filed: da = 0x%llx len = 0x%x\n",
+		 da, len);
+	return -ENOENT;
+}
+
+static void *imx_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct imx_rproc *priv = rproc->priv;
+	void *va = NULL;
+	u64 sys;
+	int i;
+
+	if (len <= 0)
+		return NULL;
+
+	/*
+	 * On device side we have many aliases, so we need to convert device
+	 * address (M4) to system bus address first.
+	 */
+	if (imx_rproc_da_to_sys(priv, da, len, &sys))
+		return NULL;
+
+	for (i = 0; i < IMX7D_RPROC_MEM_MAX; i++) {
+		if (sys >= priv->mem[i].sys_addr && sys + len <
+		    priv->mem[i].sys_addr +  priv->mem[i].size) {
+			unsigned int offset = sys - priv->mem[i].sys_addr;
+			/* __force to make sparse happy with type conversion */
+			va = (__force void *)(priv->mem[i].cpu_addr + offset);
+			break;
+		}
+	}
+
+	dev_dbg(&rproc->dev, "da = 0x%llx len = 0x%x va = 0x%p\n", da, len, va);
+
+	return va;
+}
+
+static const struct rproc_ops imx_rproc_ops = {
+	.start		= imx_rproc_start,
+	.stop		= imx_rproc_stop,
+	.da_to_va       = imx_rproc_da_to_va,
+};
+
+static int imx_rproc_addr_init(struct imx_rproc *priv,
+			       struct platform_device *pdev)
+{
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	static const char * const mem_names[] = { "ddr", "ocram", "ocram-s",
+		"ocram-epdc", "ocram-pxp" };
+	struct resource *res;
+	int a, b = 0, err;
+
+	/* remap required addresses */
+	for (a = 0; a < dcfg->att_size; a++) {
+		const struct imx_rproc_att *att = &dcfg->att[a];
+
+		if (!(att->flags & ATT_OWN))
+			continue;
+
+		if (b > IMX7D_RPROC_MEM_MAX)
+			break;
+
+		priv->mem[b].cpu_addr = devm_ioremap(&pdev->dev,
+						     att->sa, att->size);
+		if (IS_ERR(priv->mem[b].cpu_addr)) {
+			dev_err(&pdev->dev, "devm_ioremap_resource failed\n");
+			err = PTR_ERR(priv->mem[b].cpu_addr);
+			return err;
+		}
+		priv->mem[b].sys_addr = att->sa;
+		priv->mem[b].size = att->size;
+		b++;
+	}
+
+	/* remap optional addresses */
+	for (a = 0; a < ARRAY_SIZE(mem_names); a++) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[a]);
+		if (!res)
+			continue;
+
+		if (b > IMX7D_RPROC_MEM_MAX)
+			break;
+
+		priv->mem[b].cpu_addr = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->mem[b].cpu_addr)) {
+			dev_err(&pdev->dev, "devm_ioremap_resource failed\n");
+			err = PTR_ERR(priv->mem[b].cpu_addr);
+			return err;
+		}
+		priv->mem[b].sys_addr = res->start;
+		priv->mem[b].size = resource_size(res);
+		b++;
+	}
+
+	return 0;
+}
+
+static int imx_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct imx_rproc *priv;
+	struct rproc *rproc;
+	struct regmap_config config = { .name = "imx-rproc" };
+	const struct imx_rproc_dcfg *dcfg;
+	struct regmap *regmap;
+	int ret;
+
+	regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "failed to find syscon\n");
+		return PTR_ERR(regmap);
+	}
+	regmap_attach_dev(dev, regmap, &config);
+
+	/* set some other name then imx */
+	rproc = rproc_alloc(dev, "imx-rproc", &imx_rproc_ops,
+			    NULL, sizeof(*priv));
+	if (!rproc) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	dcfg = of_device_get_match_data(dev);
+	if (!dcfg)
+		return -EINVAL;
+
+	priv = rproc->priv;
+	priv->rproc = rproc;
+	priv->regmap = regmap;
+	priv->dcfg = dcfg;
+	priv->dev = dev;
+
+	dev_set_drvdata(dev, rproc);
+
+	ret = imx_rproc_addr_init(priv, pdev);
+	if (ret) {
+		dev_err(dev, "filed on imx_rproc_addr_init\n");
+		goto err_put_rproc;
+	}
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "Failed to get clock\n");
+		rproc_free(rproc);
+		return PTR_ERR(priv->clk);
+	}
+
+	/*
+	 * clk for M4 block including memory. Should be
+	 * enabled before .start for FW transfer.
+	 */
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(&rproc->dev, "Failed to enable clock\n");
+		rproc_free(rproc);
+		return ret;
+	}
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "rproc_add failed\n");
+		goto err_put_clk;
+	}
+
+	return ret;
+
+err_put_clk:
+	clk_disable_unprepare(priv->clk);
+err_put_rproc:
+	rproc_free(rproc);
+err:
+	return ret;
+}
+
+static int imx_rproc_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+	struct imx_rproc *priv = rproc->priv;
+
+	clk_disable_unprepare(priv->clk);
+	rproc_del(rproc);
+	rproc_free(rproc);
+
+	return 0;
+}
+
+static const struct of_device_id imx_rproc_of_match[] = {
+	{ .compatible = "fsl,imx7d-rproc", .data = &imx_rproc_cfg_imx7d },
+	{ .compatible = "fsl,imx6sx-rproc", .data = &imx_rproc_cfg_imx6sx },
+	{},
+};
+MODULE_DEVICE_TABLE(of, imx_rproc_of_match);
+
+static struct platform_driver imx_rproc_driver = {
+	.probe = imx_rproc_probe,
+	.remove = imx_rproc_remove,
+	.driver = {
+		.name = "imx-rproc",
+		.of_match_table = imx_rproc_of_match,
+	},
+};
+
+module_platform_driver(imx_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("IMX6SX/7D remote processor control driver");
+MODULE_AUTHOR("Oleksij Rempel <o.rempel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v1 2/2] remoteproc: imx_rproc: add a NXP/Freescale imx_rproc driver
@ 2017-07-10 14:42   ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-10 14:42 UTC (permalink / raw)
  To: linux-arm-kernel

Provide a basic driver to control Cortex M4 co-processor found
on NXP i.MX7D and i.MX6SX.
Currently it is able to resolve addresses between M4 and main CPU,
start and stop the co-processor. Other functionality is not provided
or test.

This driver was tested on NXP i.MX7D and expected to work on
i.MX6SX as well.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/remoteproc/Kconfig     |  10 +
 drivers/remoteproc/Makefile    |   1 +
 drivers/remoteproc/imx_rproc.c | 417 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 428 insertions(+)
 create mode 100644 drivers/remoteproc/imx_rproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index faad69a1a597..1f6644908629 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -13,6 +13,16 @@ config REMOTEPROC
 
 if REMOTEPROC
 
+config IMX_REMOTEPROC
+	tristate "IMX6/7 remoteproc support"
+	depends on SOC_IMX6SX || SOC_IMX7D
+	depends on REMOTEPROC
+	help
+	  Say y here to support iMX's remote processors (Cortex M4
+	  on iMX7D) via the remote processor framework.
+
+	  It's safe to say N here.
+
 config OMAP_REMOTEPROC
 	tristate "OMAP remoteproc support"
 	depends on HAS_DMA
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ffc5e430df27..d351c25cdb4e 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -19,3 +19,4 @@ qcom_wcnss_pil-y			+= qcom_wcnss.o
 qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
+obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
new file mode 100644
index 000000000000..2a5300c3323c
--- /dev/null
+++ b/drivers/remoteproc/imx_rproc.c
@@ -0,0 +1,417 @@
+/*
+ * Copyright (c) 2017 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/remoteproc.h>
+
+#define IMX7D_SRC_SCR			0x0C
+#define IMX7D_ENABLE_M4			BIT(3)
+#define IMX7D_SW_M4P_RST		BIT(2)
+#define IMX7D_SW_M4C_RST		BIT(1)
+#define IMX7D_SW_M4C_NON_SCLR_RST	BIT(0)
+
+#define IMX7D_M4_RST_MASK		(IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
+					 | IMX7D_SW_M4C_RST \
+					 | IMX7D_SW_M4C_NON_SCLR_RST)
+
+#define IMX7D_M4_START			(IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
+					 | IMX7D_SW_M4C_RST)
+#define IMX7D_M4_STOP			IMX7D_SW_M4C_NON_SCLR_RST
+
+/* Address: 0x020D8000 */
+#define IMX6SX_SRC_SCR			0x00
+#define IMX6SX_ENABLE_M4		BIT(22)
+#define IMX6SX_SW_M4P_RST		BIT(12)
+#define IMX6SX_SW_M4C_NON_SCLR_RST	BIT(4)
+#define IMX6SX_SW_M4C_RST		BIT(3)
+
+#define IMX6SX_M4_START			(IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
+					 | IMX6SX_SW_M4C_RST)
+#define IMX6SX_M4_STOP			IMX6SX_SW_M4C_NON_SCLR_RST
+#define IMX6SX_M4_RST_MASK		(IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
+					 | IMX6SX_SW_M4C_NON_SCLR_RST \
+					 | IMX6SX_SW_M4C_RST)
+
+#define IMX7D_RPROC_MEM_MAX		8
+
+/**
+ * struct imx_rproc_mem - slim internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @sys_addr: Bus address used to access the memory region
+ * @size: Size of the memory region
+ */
+struct imx_rproc_mem {
+	void __iomem *cpu_addr;
+	phys_addr_t sys_addr;
+	size_t size;
+};
+
+/* att flags */
+/* M4 own area. Can be mapped at probe */
+#define ATT_OWN		BIT(1)
+
+/* address translation table */
+struct imx_rproc_att {
+	u32 da;	/* device address (From Cortex M4 view)*/
+	u32 sa;	/* system bus address */
+	u32 size; /* size of reg range */
+	int flags;
+};
+
+struct imx_rproc_dcfg {
+	u32				src_reg;
+	u32				src_mask;
+	u32				src_start;
+	u32				src_stop;
+	const struct imx_rproc_att	*att;
+	size_t				att_size;
+};
+
+struct imx_rproc {
+	struct device			*dev;
+	struct regmap			*regmap;
+	struct rproc			*rproc;
+	const struct imx_rproc_dcfg	*dcfg;
+	struct imx_rproc_mem		mem[IMX7D_RPROC_MEM_MAX];
+	struct clk			*clk;
+};
+
+static const struct imx_rproc_att imx_rproc_att_imx7d[] = {
+	/* dev addr , sys addr  , size	    , flags */
+	/* OCRAM_S (M4 Boot code) - alias */
+	{ 0x00000000, 0x00180000, 0x00008000, 0 },
+	/* OCRAM_S (Code) */
+	{ 0x00180000, 0x00180000, 0x00008000, ATT_OWN },
+	/* OCRAM (Code) - alias */
+	{ 0x00900000, 0x00900000, 0x00020000, 0 },
+	/* OCRAM_EPDC (Code) - alias */
+	{ 0x00920000, 0x00920000, 0x00020000, 0 },
+	/* OCRAM_PXP (Code) - alias */
+	{ 0x00940000, 0x00940000, 0x00008000, 0 },
+	/* TCML (Code) */
+	{ 0x1FFF8000, 0x007F8000, 0x00008000, ATT_OWN },
+	/* DDR (Code) - alias, first part of DDR (Data) */
+	{ 0x10000000, 0x80000000, 0x0FFF0000, 0 },
+
+	/* TCMU (Data) */
+	{ 0x20000000, 0x00800000, 0x00008000, ATT_OWN },
+	/* OCRAM (Data) */
+	{ 0x20200000, 0x00900000, 0x00020000, 0 },
+	/* OCRAM_EPDC (Data) */
+	{ 0x20220000, 0x00920000, 0x00020000, 0 },
+	/* OCRAM_PXP (Data) */
+	{ 0x20240000, 0x00940000, 0x00008000, 0 },
+	/* DDR (Data) */
+	{ 0x80000000, 0x80000000, 0x60000000, 0 },
+};
+
+static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
+	/* dev addr , sys addr  , size	    , flags */
+	/* TCML (M4 Boot Code) - alias */
+	{ 0x00000000, 0x007F8000, 0x00008000, 0 },
+	/* OCRAM_S (Code) */
+	{ 0x00180000, 0x008F8000, 0x00004000, 0 },
+	/* OCRAM_S (Code) - alias */
+	{ 0x00180000, 0x008FC000, 0x00004000, 0 },
+	/* TCML (Code) */
+	{ 0x1FFF8000, 0x007F8000, 0x00008000, ATT_OWN },
+	/* DDR (Code) - alias, first part of DDR (Data) */
+	{ 0x10000000, 0x80000000, 0x0FFF8000, 0 },
+
+	/* TCMU (Data) */
+	{ 0x20000000, 0x00800000, 0x00008000, ATT_OWN },
+	/* OCRAM_S (Data) - alias? */
+	{ 0x208F8000, 0x008F8000, 0x00004000, 0 },
+	/* DDR (Data) */
+	{ 0x80000000, 0x80000000, 0x60000000, 0 },
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
+	.src_reg	= IMX7D_SRC_SCR,
+	.src_mask	= IMX7D_M4_RST_MASK,
+	.src_start	= IMX7D_M4_START,
+	.src_stop	= IMX7D_M4_STOP,
+	.att		= imx_rproc_att_imx7d,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
+	.src_reg	= IMX6SX_SRC_SCR,
+	.src_mask	= IMX6SX_M4_RST_MASK,
+	.src_start	= IMX6SX_M4_START,
+	.src_stop	= IMX6SX_M4_STOP,
+	.att		= imx_rproc_att_imx6sx,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
+};
+
+static int imx_rproc_start(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct device *dev = priv->dev;
+	int ret;
+
+	ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
+				 dcfg->src_mask, dcfg->src_start);
+	if (ret)
+		dev_err(dev, "Filed to enable M4!\n");
+
+	return ret;
+}
+
+static int imx_rproc_stop(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct device *dev = priv->dev;
+	int ret;
+
+	ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
+				 dcfg->src_mask, dcfg->src_stop);
+	if (ret)
+		dev_err(dev, "Filed to stop M4!\n");
+
+	return ret;
+}
+
+static int imx_rproc_da_to_sys(struct imx_rproc *priv, u64 da,
+			       int len, u64 *sys)
+{
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	int i;
+
+	/* parse address translation table */
+	for (i = 0; i < dcfg->att_size; i++) {
+		const struct imx_rproc_att *att = &dcfg->att[i];
+
+		if (da >= att->da && da + len < att->da + att->size) {
+			unsigned int offset = da - att->da;
+
+			*sys = att->sa + offset;
+			return 0;
+		}
+	}
+
+	dev_warn(priv->dev, "Translation filed: da = 0x%llx len = 0x%x\n",
+		 da, len);
+	return -ENOENT;
+}
+
+static void *imx_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct imx_rproc *priv = rproc->priv;
+	void *va = NULL;
+	u64 sys;
+	int i;
+
+	if (len <= 0)
+		return NULL;
+
+	/*
+	 * On device side we have many aliases, so we need to convert device
+	 * address (M4) to system bus address first.
+	 */
+	if (imx_rproc_da_to_sys(priv, da, len, &sys))
+		return NULL;
+
+	for (i = 0; i < IMX7D_RPROC_MEM_MAX; i++) {
+		if (sys >= priv->mem[i].sys_addr && sys + len <
+		    priv->mem[i].sys_addr +  priv->mem[i].size) {
+			unsigned int offset = sys - priv->mem[i].sys_addr;
+			/* __force to make sparse happy with type conversion */
+			va = (__force void *)(priv->mem[i].cpu_addr + offset);
+			break;
+		}
+	}
+
+	dev_dbg(&rproc->dev, "da = 0x%llx len = 0x%x va = 0x%p\n", da, len, va);
+
+	return va;
+}
+
+static const struct rproc_ops imx_rproc_ops = {
+	.start		= imx_rproc_start,
+	.stop		= imx_rproc_stop,
+	.da_to_va       = imx_rproc_da_to_va,
+};
+
+static int imx_rproc_addr_init(struct imx_rproc *priv,
+			       struct platform_device *pdev)
+{
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	static const char * const mem_names[] = { "ddr", "ocram", "ocram-s",
+		"ocram-epdc", "ocram-pxp" };
+	struct resource *res;
+	int a, b = 0, err;
+
+	/* remap required addresses */
+	for (a = 0; a < dcfg->att_size; a++) {
+		const struct imx_rproc_att *att = &dcfg->att[a];
+
+		if (!(att->flags & ATT_OWN))
+			continue;
+
+		if (b > IMX7D_RPROC_MEM_MAX)
+			break;
+
+		priv->mem[b].cpu_addr = devm_ioremap(&pdev->dev,
+						     att->sa, att->size);
+		if (IS_ERR(priv->mem[b].cpu_addr)) {
+			dev_err(&pdev->dev, "devm_ioremap_resource failed\n");
+			err = PTR_ERR(priv->mem[b].cpu_addr);
+			return err;
+		}
+		priv->mem[b].sys_addr = att->sa;
+		priv->mem[b].size = att->size;
+		b++;
+	}
+
+	/* remap optional addresses */
+	for (a = 0; a < ARRAY_SIZE(mem_names); a++) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[a]);
+		if (!res)
+			continue;
+
+		if (b > IMX7D_RPROC_MEM_MAX)
+			break;
+
+		priv->mem[b].cpu_addr = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->mem[b].cpu_addr)) {
+			dev_err(&pdev->dev, "devm_ioremap_resource failed\n");
+			err = PTR_ERR(priv->mem[b].cpu_addr);
+			return err;
+		}
+		priv->mem[b].sys_addr = res->start;
+		priv->mem[b].size = resource_size(res);
+		b++;
+	}
+
+	return 0;
+}
+
+static int imx_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct imx_rproc *priv;
+	struct rproc *rproc;
+	struct regmap_config config = { .name = "imx-rproc" };
+	const struct imx_rproc_dcfg *dcfg;
+	struct regmap *regmap;
+	int ret;
+
+	regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "failed to find syscon\n");
+		return PTR_ERR(regmap);
+	}
+	regmap_attach_dev(dev, regmap, &config);
+
+	/* set some other name then imx */
+	rproc = rproc_alloc(dev, "imx-rproc", &imx_rproc_ops,
+			    NULL, sizeof(*priv));
+	if (!rproc) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	dcfg = of_device_get_match_data(dev);
+	if (!dcfg)
+		return -EINVAL;
+
+	priv = rproc->priv;
+	priv->rproc = rproc;
+	priv->regmap = regmap;
+	priv->dcfg = dcfg;
+	priv->dev = dev;
+
+	dev_set_drvdata(dev, rproc);
+
+	ret = imx_rproc_addr_init(priv, pdev);
+	if (ret) {
+		dev_err(dev, "filed on imx_rproc_addr_init\n");
+		goto err_put_rproc;
+	}
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "Failed to get clock\n");
+		rproc_free(rproc);
+		return PTR_ERR(priv->clk);
+	}
+
+	/*
+	 * clk for M4 block including memory. Should be
+	 * enabled before .start for FW transfer.
+	 */
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(&rproc->dev, "Failed to enable clock\n");
+		rproc_free(rproc);
+		return ret;
+	}
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "rproc_add failed\n");
+		goto err_put_clk;
+	}
+
+	return ret;
+
+err_put_clk:
+	clk_disable_unprepare(priv->clk);
+err_put_rproc:
+	rproc_free(rproc);
+err:
+	return ret;
+}
+
+static int imx_rproc_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+	struct imx_rproc *priv = rproc->priv;
+
+	clk_disable_unprepare(priv->clk);
+	rproc_del(rproc);
+	rproc_free(rproc);
+
+	return 0;
+}
+
+static const struct of_device_id imx_rproc_of_match[] = {
+	{ .compatible = "fsl,imx7d-rproc", .data = &imx_rproc_cfg_imx7d },
+	{ .compatible = "fsl,imx6sx-rproc", .data = &imx_rproc_cfg_imx6sx },
+	{},
+};
+MODULE_DEVICE_TABLE(of, imx_rproc_of_match);
+
+static struct platform_driver imx_rproc_driver = {
+	.probe = imx_rproc_probe,
+	.remove = imx_rproc_remove,
+	.driver = {
+		.name = "imx-rproc",
+		.of_match_table = imx_rproc_of_match,
+	},
+};
+
+module_platform_driver(imx_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("IMX6SX/7D remote processor control driver");
+MODULE_AUTHOR("Oleksij Rempel <o.rempel@pengutronix.de>");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-10 22:14     ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-10 22:14 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: devicetree, kernel, linux-arm-kernel, linux-kernel, Rob Herring,
	Mark Rutland, Russell King, Shawn Guo, Fabio Estevam,
	Ohad Ben-Cohen, linux-remoteproc, linux

On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:

> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> new file mode 100644
> index 000000000000..e7c61993e1b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> @@ -0,0 +1,44 @@
> +NXP iMX6SX/iMX7D Co-Processor Bindings
> +----------------------------------------
> +
> +This binding provides support for ARM Cortex M4 Co-processor found on some
> +NXP iMX SoCs.
> +
> +Required properties:
> +- compatible		Should be one of:
> +				"fsl,imx7d-rproc"
> +				"fsl,imx6sx-rproc"
> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
> +- syscfg		Phandle to syscon block which provide access to

This is called "syscon" in the code and the example.

> +			System Reset Controller
> +
> +Optional properties:
> +- reg:			Should contain the address ranges for some of internal
> +			memory regions.

Something less hand-wavy, like: "Should list the memory regions for the
remoteproc"

> +- reg-names:		Contains the corresponding names for the memory
> +			regions. These should be named "ddr", "ocram", "ocram-s",
> +			"ocram-epdc" or "ocram-pxp".

Make this comment define which memory regions are required for each of
the systems.

> +Fallowing memory ranges are expected:
> +- For "fsl,imx7d-rproc"
> +	<0x00900000 0x00020000> - "ocram"
> +	<0x00920000 0x00020000> - "ocram-epdc"
> +	<0x00940000 0x00008000> - "ocram-pxp"
> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
> +	<0x80000000 0x60000000> - "ddr" (data area)

There's no reason to list the actual regions in the binding
document. Just list the requires regions for each system.

> +- For "fsl,imx6sx-rproc"
> +	<0x008F8000 0x00004000> - "ocram-s"
> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
> +	<0x80000000 0x60000000> - "ddr" (data area)
> +
> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
> +than data area.  So this range should be carefully chosen according to your
> +system and application requirements.
> +

This is a source of future issues as this indicates that I should have
reg-names list "ddr" twice.

Also, as you warn about the user needing to pick the values for "ddr",
does that mean that it's a carveout of System RAM?

> +Example:
> +	imx_rproc: imx7d-rp0@0 {

imx7d-rproc@80000000 {

And there's currently no reason to label this node.

> +		compatible	= "fsl,imx7d-rproc";
> +		reg		= <0x80000000 0x80000>, <0x00900000 0x2000>;
> +		reg-names	= "ddr", "ocram";
> +		syscon		= <&src>;
> +		clocks		= <&clks IMX7D_ARM_M4_ROOT_CLK>;
> +	};

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-10 22:14     ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-10 22:14 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Russell King, Shawn Guo, Fabio Estevam, Ohad Ben-Cohen,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linux-YEK0n+YFykbzxQdaRaTXBw

On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:

> Signed-off-by: Oleksij Rempel <o.rempel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> new file mode 100644
> index 000000000000..e7c61993e1b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> @@ -0,0 +1,44 @@
> +NXP iMX6SX/iMX7D Co-Processor Bindings
> +----------------------------------------
> +
> +This binding provides support for ARM Cortex M4 Co-processor found on some
> +NXP iMX SoCs.
> +
> +Required properties:
> +- compatible		Should be one of:
> +				"fsl,imx7d-rproc"
> +				"fsl,imx6sx-rproc"
> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
> +- syscfg		Phandle to syscon block which provide access to

This is called "syscon" in the code and the example.

> +			System Reset Controller
> +
> +Optional properties:
> +- reg:			Should contain the address ranges for some of internal
> +			memory regions.

Something less hand-wavy, like: "Should list the memory regions for the
remoteproc"

> +- reg-names:		Contains the corresponding names for the memory
> +			regions. These should be named "ddr", "ocram", "ocram-s",
> +			"ocram-epdc" or "ocram-pxp".

Make this comment define which memory regions are required for each of
the systems.

> +Fallowing memory ranges are expected:
> +- For "fsl,imx7d-rproc"
> +	<0x00900000 0x00020000> - "ocram"
> +	<0x00920000 0x00020000> - "ocram-epdc"
> +	<0x00940000 0x00008000> - "ocram-pxp"
> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
> +	<0x80000000 0x60000000> - "ddr" (data area)

There's no reason to list the actual regions in the binding
document. Just list the requires regions for each system.

> +- For "fsl,imx6sx-rproc"
> +	<0x008F8000 0x00004000> - "ocram-s"
> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
> +	<0x80000000 0x60000000> - "ddr" (data area)
> +
> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
> +than data area.  So this range should be carefully chosen according to your
> +system and application requirements.
> +

This is a source of future issues as this indicates that I should have
reg-names list "ddr" twice.

Also, as you warn about the user needing to pick the values for "ddr",
does that mean that it's a carveout of System RAM?

> +Example:
> +	imx_rproc: imx7d-rp0@0 {

imx7d-rproc@80000000 {

And there's currently no reason to label this node.

> +		compatible	= "fsl,imx7d-rproc";
> +		reg		= <0x80000000 0x80000>, <0x00900000 0x2000>;
> +		reg-names	= "ddr", "ocram";
> +		syscon		= <&src>;
> +		clocks		= <&clks IMX7D_ARM_M4_ROOT_CLK>;
> +	};

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-10 22:14     ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-10 22:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:

> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> new file mode 100644
> index 000000000000..e7c61993e1b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> @@ -0,0 +1,44 @@
> +NXP iMX6SX/iMX7D Co-Processor Bindings
> +----------------------------------------
> +
> +This binding provides support for ARM Cortex M4 Co-processor found on some
> +NXP iMX SoCs.
> +
> +Required properties:
> +- compatible		Should be one of:
> +				"fsl,imx7d-rproc"
> +				"fsl,imx6sx-rproc"
> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
> +- syscfg		Phandle to syscon block which provide access to

This is called "syscon" in the code and the example.

> +			System Reset Controller
> +
> +Optional properties:
> +- reg:			Should contain the address ranges for some of internal
> +			memory regions.

Something less hand-wavy, like: "Should list the memory regions for the
remoteproc"

> +- reg-names:		Contains the corresponding names for the memory
> +			regions. These should be named "ddr", "ocram", "ocram-s",
> +			"ocram-epdc" or "ocram-pxp".

Make this comment define which memory regions are required for each of
the systems.

> +Fallowing memory ranges are expected:
> +- For "fsl,imx7d-rproc"
> +	<0x00900000 0x00020000> - "ocram"
> +	<0x00920000 0x00020000> - "ocram-epdc"
> +	<0x00940000 0x00008000> - "ocram-pxp"
> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
> +	<0x80000000 0x60000000> - "ddr" (data area)

There's no reason to list the actual regions in the binding
document. Just list the requires regions for each system.

> +- For "fsl,imx6sx-rproc"
> +	<0x008F8000 0x00004000> - "ocram-s"
> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
> +	<0x80000000 0x60000000> - "ddr" (data area)
> +
> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
> +than data area.  So this range should be carefully chosen according to your
> +system and application requirements.
> +

This is a source of future issues as this indicates that I should have
reg-names list "ddr" twice.

Also, as you warn about the user needing to pick the values for "ddr",
does that mean that it's a carveout of System RAM?

> +Example:
> +	imx_rproc: imx7d-rp0 at 0 {

imx7d-rproc at 80000000 {

And there's currently no reason to label this node.

> +		compatible	= "fsl,imx7d-rproc";
> +		reg		= <0x80000000 0x80000>, <0x00900000 0x2000>;
> +		reg-names	= "ddr", "ocram";
> +		syscon		= <&src>;
> +		clocks		= <&clks IMX7D_ARM_M4_ROOT_CLK>;
> +	};

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
  2017-07-10 22:14     ` Bjorn Andersson
  (?)
@ 2017-07-18  8:45       ` Oleksij Rempel
  -1 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-18  8:45 UTC (permalink / raw)
  To: Bjorn Andersson, Oleksij Rempel
  Cc: devicetree, kernel, linux-arm-kernel, linux-kernel, Rob Herring,
	Mark Rutland, Russell King, Shawn Guo, Fabio Estevam,
	Ohad Ben-Cohen, linux-remoteproc, linux

Hallo Bjorn,

On 11.07.2017 00:14, Bjorn Andersson wrote:
> On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> ---
>>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>>  1 file changed, 44 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>> new file mode 100644
>> index 000000000000..e7c61993e1b8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>> @@ -0,0 +1,44 @@
>> +NXP iMX6SX/iMX7D Co-Processor Bindings
>> +----------------------------------------
>> +
>> +This binding provides support for ARM Cortex M4 Co-processor found on some
>> +NXP iMX SoCs.
>> +
>> +Required properties:
>> +- compatible		Should be one of:
>> +				"fsl,imx7d-rproc"
>> +				"fsl,imx6sx-rproc"
>> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
>> +- syscfg		Phandle to syscon block which provide access to
>
> This is called "syscon" in the code and the example.

done.

>> +			System Reset Controller
>> +
>> +Optional properties:
>> +- reg:			Should contain the address ranges for some of internal
>> +			memory regions.
>
> Something less hand-wavy, like: "Should list the memory regions for the
> remoteproc"
>
>> +- reg-names:		Contains the corresponding names for the memory
>> +			regions. These should be named "ddr", "ocram", "ocram-s",
>> +			"ocram-epdc" or "ocram-pxp".
>
> Make this comment define which memory regions are required for each of
> the systems.

done.

>> +Fallowing memory ranges are expected:
>> +- For "fsl,imx7d-rproc"
>> +	<0x00900000 0x00020000> - "ocram"
>> +	<0x00920000 0x00020000> - "ocram-epdc"
>> +	<0x00940000 0x00008000> - "ocram-pxp"
>> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
>> +	<0x80000000 0x60000000> - "ddr" (data area)
>
> There's no reason to list the actual regions in the binding
> document. Just list the requires regions for each system.
>
>> +- For "fsl,imx6sx-rproc"
>> +	<0x008F8000 0x00004000> - "ocram-s"
>> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
>> +	<0x80000000 0x60000000> - "ddr" (data area)
>> +
>> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
>> +than data area.  So this range should be carefully chosen according to your
>> +system and application requirements.
>> +
>
> This is a source of future issues as this indicates that I should have
> reg-names list "ddr" twice.

Then I will name it:
"ddri" (incstruction/code area),
"ddrd" (data area)

unless there are other suggestions.

> Also, as you warn about the user needing to pick the values for "ddr",
> does that mean that it's a carveout of System RAM?

Yes, it is a part of System RAM.

>> +Example:
>> +	imx_rproc: imx7d-rp0@0 {
>
> imx7d-rproc@80000000 {
>
> And there's currently no reason to label this node.
>
>> +		compatible	= "fsl,imx7d-rproc";
>> +		reg		= <0x80000000 0x80000>, <0x00900000 0x2000>;
>> +		reg-names	= "ddr", "ocram";
>> +		syscon		= <&src>;
>> +		clocks		= <&clks IMX7D_ARM_M4_ROOT_CLK>;
>> +	};

Regards,
Oleksij

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-18  8:45       ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-18  8:45 UTC (permalink / raw)
  To: Bjorn Andersson, Oleksij Rempel
  Cc: Mark Rutland, devicetree, Ohad Ben-Cohen, linux-remoteproc,
	linux-kernel, Russell King, Rob Herring, kernel, Fabio Estevam,
	linux, Shawn Guo, linux-arm-kernel

Hallo Bjorn,

On 11.07.2017 00:14, Bjorn Andersson wrote:
> On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> ---
>>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>>  1 file changed, 44 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>> new file mode 100644
>> index 000000000000..e7c61993e1b8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>> @@ -0,0 +1,44 @@
>> +NXP iMX6SX/iMX7D Co-Processor Bindings
>> +----------------------------------------
>> +
>> +This binding provides support for ARM Cortex M4 Co-processor found on some
>> +NXP iMX SoCs.
>> +
>> +Required properties:
>> +- compatible		Should be one of:
>> +				"fsl,imx7d-rproc"
>> +				"fsl,imx6sx-rproc"
>> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
>> +- syscfg		Phandle to syscon block which provide access to
>
> This is called "syscon" in the code and the example.

done.

>> +			System Reset Controller
>> +
>> +Optional properties:
>> +- reg:			Should contain the address ranges for some of internal
>> +			memory regions.
>
> Something less hand-wavy, like: "Should list the memory regions for the
> remoteproc"
>
>> +- reg-names:		Contains the corresponding names for the memory
>> +			regions. These should be named "ddr", "ocram", "ocram-s",
>> +			"ocram-epdc" or "ocram-pxp".
>
> Make this comment define which memory regions are required for each of
> the systems.

done.

>> +Fallowing memory ranges are expected:
>> +- For "fsl,imx7d-rproc"
>> +	<0x00900000 0x00020000> - "ocram"
>> +	<0x00920000 0x00020000> - "ocram-epdc"
>> +	<0x00940000 0x00008000> - "ocram-pxp"
>> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
>> +	<0x80000000 0x60000000> - "ddr" (data area)
>
> There's no reason to list the actual regions in the binding
> document. Just list the requires regions for each system.
>
>> +- For "fsl,imx6sx-rproc"
>> +	<0x008F8000 0x00004000> - "ocram-s"
>> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
>> +	<0x80000000 0x60000000> - "ddr" (data area)
>> +
>> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
>> +than data area.  So this range should be carefully chosen according to your
>> +system and application requirements.
>> +
>
> This is a source of future issues as this indicates that I should have
> reg-names list "ddr" twice.

Then I will name it:
"ddri" (incstruction/code area),
"ddrd" (data area)

unless there are other suggestions.

> Also, as you warn about the user needing to pick the values for "ddr",
> does that mean that it's a carveout of System RAM?

Yes, it is a part of System RAM.

>> +Example:
>> +	imx_rproc: imx7d-rp0@0 {
>
> imx7d-rproc@80000000 {
>
> And there's currently no reason to label this node.
>
>> +		compatible	= "fsl,imx7d-rproc";
>> +		reg		= <0x80000000 0x80000>, <0x00900000 0x2000>;
>> +		reg-names	= "ddr", "ocram";
>> +		syscon		= <&src>;
>> +		clocks		= <&clks IMX7D_ARM_M4_ROOT_CLK>;
>> +	};

Regards,
Oleksij

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-18  8:45       ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-18  8:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hallo Bjorn,

On 11.07.2017 00:14, Bjorn Andersson wrote:
> On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> ---
>>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>>  1 file changed, 44 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>> new file mode 100644
>> index 000000000000..e7c61993e1b8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>> @@ -0,0 +1,44 @@
>> +NXP iMX6SX/iMX7D Co-Processor Bindings
>> +----------------------------------------
>> +
>> +This binding provides support for ARM Cortex M4 Co-processor found on some
>> +NXP iMX SoCs.
>> +
>> +Required properties:
>> +- compatible		Should be one of:
>> +				"fsl,imx7d-rproc"
>> +				"fsl,imx6sx-rproc"
>> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
>> +- syscfg		Phandle to syscon block which provide access to
>
> This is called "syscon" in the code and the example.

done.

>> +			System Reset Controller
>> +
>> +Optional properties:
>> +- reg:			Should contain the address ranges for some of internal
>> +			memory regions.
>
> Something less hand-wavy, like: "Should list the memory regions for the
> remoteproc"
>
>> +- reg-names:		Contains the corresponding names for the memory
>> +			regions. These should be named "ddr", "ocram", "ocram-s",
>> +			"ocram-epdc" or "ocram-pxp".
>
> Make this comment define which memory regions are required for each of
> the systems.

done.

>> +Fallowing memory ranges are expected:
>> +- For "fsl,imx7d-rproc"
>> +	<0x00900000 0x00020000> - "ocram"
>> +	<0x00920000 0x00020000> - "ocram-epdc"
>> +	<0x00940000 0x00008000> - "ocram-pxp"
>> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
>> +	<0x80000000 0x60000000> - "ddr" (data area)
>
> There's no reason to list the actual regions in the binding
> document. Just list the requires regions for each system.
>
>> +- For "fsl,imx6sx-rproc"
>> +	<0x008F8000 0x00004000> - "ocram-s"
>> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
>> +	<0x80000000 0x60000000> - "ddr" (data area)
>> +
>> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
>> +than data area.  So this range should be carefully chosen according to your
>> +system and application requirements.
>> +
>
> This is a source of future issues as this indicates that I should have
> reg-names list "ddr" twice.

Then I will name it:
"ddri" (incstruction/code area),
"ddrd" (data area)

unless there are other suggestions.

> Also, as you warn about the user needing to pick the values for "ddr",
> does that mean that it's a carveout of System RAM?

Yes, it is a part of System RAM.

>> +Example:
>> +	imx_rproc: imx7d-rp0 at 0 {
>
> imx7d-rproc at 80000000 {
>
> And there's currently no reason to label this node.
>
>> +		compatible	= "fsl,imx7d-rproc";
>> +		reg		= <0x80000000 0x80000>, <0x00900000 0x2000>;
>> +		reg-names	= "ddr", "ocram";
>> +		syscon		= <&src>;
>> +		clocks		= <&clks IMX7D_ARM_M4_ROOT_CLK>;
>> +	};

Regards,
Oleksij

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
  2017-07-18  8:45       ` Oleksij Rempel
  (?)
  (?)
@ 2017-07-18 16:38         ` Bjorn Andersson
  -1 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-18 16:38 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Oleksij Rempel, devicetree, kernel, linux-arm-kernel,
	linux-kernel, Rob Herring, Mark Rutland, Russell King, Shawn Guo,
	Fabio Estevam, Ohad Ben-Cohen, linux-remoteproc, linux

On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:

> Hallo Bjorn,
> 
> On 11.07.2017 00:14, Bjorn Andersson wrote:
> > On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
> > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > >  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
> > >  1 file changed, 44 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > new file mode 100644
> > > index 000000000000..e7c61993e1b8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > @@ -0,0 +1,44 @@
> > > +NXP iMX6SX/iMX7D Co-Processor Bindings
> > > +----------------------------------------
> > > +
> > > +This binding provides support for ARM Cortex M4 Co-processor found on some
> > > +NXP iMX SoCs.
> > > +
> > > +Required properties:
> > > +- compatible		Should be one of:
> > > +				"fsl,imx7d-rproc"
> > > +				"fsl,imx6sx-rproc"
> > > +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
> > > +- syscfg		Phandle to syscon block which provide access to
> > 
> > This is called "syscon" in the code and the example.
> 
> done.
> 
> > > +			System Reset Controller
> > > +
> > > +Optional properties:
> > > +- reg:			Should contain the address ranges for some of internal
> > > +			memory regions.
> > 
> > Something less hand-wavy, like: "Should list the memory regions for the
> > remoteproc"
> > 
> > > +- reg-names:		Contains the corresponding names for the memory
> > > +			regions. These should be named "ddr", "ocram", "ocram-s",
> > > +			"ocram-epdc" or "ocram-pxp".
> > 
> > Make this comment define which memory regions are required for each of
> > the systems.
> 
> done.
> 
> > > +Fallowing memory ranges are expected:
> > > +- For "fsl,imx7d-rproc"
> > > +	<0x00900000 0x00020000> - "ocram"
> > > +	<0x00920000 0x00020000> - "ocram-epdc"
> > > +	<0x00940000 0x00008000> - "ocram-pxp"
> > > +	<0x80000000 0x0FFF0000> - "ddr" (code area)
> > > +	<0x80000000 0x60000000> - "ddr" (data area)
> > 
> > There's no reason to list the actual regions in the binding
> > document. Just list the requires regions for each system.
> > 
> > > +- For "fsl,imx6sx-rproc"
> > > +	<0x008F8000 0x00004000> - "ocram-s"
> > > +	<0x80000000 0x0FFF8000> - "ddr" (code area)
> > > +	<0x80000000 0x60000000> - "ddr" (data area)
> > > +
> > > +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
> > > +than data area.  So this range should be carefully chosen according to your
> > > +system and application requirements.
> > > +
> > 
> > This is a source of future issues as this indicates that I should have
> > reg-names list "ddr" twice.
> 
> Then I will name it:
> "ddri" (incstruction/code area),
> "ddrd" (data area)
> 
> unless there are other suggestions.
> 

But are you saying that it's correct that these two memory regions
should overlap?

> > Also, as you warn about the user needing to pick the values for "ddr",
> > does that mean that it's a carveout of System RAM?
> 
> Yes, it is a part of System RAM.
> 

Then there will be an associated reserved-memory region for the
region(s), you should add a label to this and use "memory-region" to get
hold of the addresses instead.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-18 16:38         ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-18 16:38 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Oleksij Rempel, devicetree, kernel, linux-arm-kernel,
	linux-kernel, Rob Herring, Mark Rutland, Russell King, Shawn Guo,
	Fabio Estevam, Ohad Ben-Cohen, linux-remoteproc, linux

On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:

> Hallo Bjorn,
> 
> On 11.07.2017 00:14, Bjorn Andersson wrote:
> > On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
> > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > >  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
> > >  1 file changed, 44 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > new file mode 100644
> > > index 000000000000..e7c61993e1b8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > @@ -0,0 +1,44 @@
> > > +NXP iMX6SX/iMX7D Co-Processor Bindings
> > > +----------------------------------------
> > > +
> > > +This binding provides support for ARM Cortex M4 Co-processor found on some
> > > +NXP iMX SoCs.
> > > +
> > > +Required properties:
> > > +- compatible		Should be one of:
> > > +				"fsl,imx7d-rproc"
> > > +				"fsl,imx6sx-rproc"
> > > +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
> > > +- syscfg		Phandle to syscon block which provide access to
> > 
> > This is called "syscon" in the code and the example.
> 
> done.
> 
> > > +			System Reset Controller
> > > +
> > > +Optional properties:
> > > +- reg:			Should contain the address ranges for some of internal
> > > +			memory regions.
> > 
> > Something less hand-wavy, like: "Should list the memory regions for the
> > remoteproc"
> > 
> > > +- reg-names:		Contains the corresponding names for the memory
> > > +			regions. These should be named "ddr", "ocram", "ocram-s",
> > > +			"ocram-epdc" or "ocram-pxp".
> > 
> > Make this comment define which memory regions are required for each of
> > the systems.
> 
> done.
> 
> > > +Fallowing memory ranges are expected:
> > > +- For "fsl,imx7d-rproc"
> > > +	<0x00900000 0x00020000> - "ocram"
> > > +	<0x00920000 0x00020000> - "ocram-epdc"
> > > +	<0x00940000 0x00008000> - "ocram-pxp"
> > > +	<0x80000000 0x0FFF0000> - "ddr" (code area)
> > > +	<0x80000000 0x60000000> - "ddr" (data area)
> > 
> > There's no reason to list the actual regions in the binding
> > document. Just list the requires regions for each system.
> > 
> > > +- For "fsl,imx6sx-rproc"
> > > +	<0x008F8000 0x00004000> - "ocram-s"
> > > +	<0x80000000 0x0FFF8000> - "ddr" (code area)
> > > +	<0x80000000 0x60000000> - "ddr" (data area)
> > > +
> > > +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
> > > +than data area.  So this range should be carefully chosen according to your
> > > +system and application requirements.
> > > +
> > 
> > This is a source of future issues as this indicates that I should have
> > reg-names list "ddr" twice.
> 
> Then I will name it:
> "ddri" (incstruction/code area),
> "ddrd" (data area)
> 
> unless there are other suggestions.
> 

But are you saying that it's correct that these two memory regions
should overlap?

> > Also, as you warn about the user needing to pick the values for "ddr",
> > does that mean that it's a carveout of System RAM?
> 
> Yes, it is a part of System RAM.
> 

Then there will be an associated reserved-memory region for the
region(s), you should add a label to this and use "memory-region" to get
hold of the addresses instead.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-18 16:38         ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-18 16:38 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Oleksij Rempel, devicetree-u79uwXL29TY76Z2rM5mHXA,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Russell King, Shawn Guo, Fabio Estevam, Ohad Ben-Cohen,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linux-YEK0n+YFykbzxQdaRaTXBw

On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:

> Hallo Bjorn,
> 
> On 11.07.2017 00:14, Bjorn Andersson wrote:
> > On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
> > 
> > > Signed-off-by: Oleksij Rempel <o.rempel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > ---
> > >  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
> > >  1 file changed, 44 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > new file mode 100644
> > > index 000000000000..e7c61993e1b8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > @@ -0,0 +1,44 @@
> > > +NXP iMX6SX/iMX7D Co-Processor Bindings
> > > +----------------------------------------
> > > +
> > > +This binding provides support for ARM Cortex M4 Co-processor found on some
> > > +NXP iMX SoCs.
> > > +
> > > +Required properties:
> > > +- compatible		Should be one of:
> > > +				"fsl,imx7d-rproc"
> > > +				"fsl,imx6sx-rproc"
> > > +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
> > > +- syscfg		Phandle to syscon block which provide access to
> > 
> > This is called "syscon" in the code and the example.
> 
> done.
> 
> > > +			System Reset Controller
> > > +
> > > +Optional properties:
> > > +- reg:			Should contain the address ranges for some of internal
> > > +			memory regions.
> > 
> > Something less hand-wavy, like: "Should list the memory regions for the
> > remoteproc"
> > 
> > > +- reg-names:		Contains the corresponding names for the memory
> > > +			regions. These should be named "ddr", "ocram", "ocram-s",
> > > +			"ocram-epdc" or "ocram-pxp".
> > 
> > Make this comment define which memory regions are required for each of
> > the systems.
> 
> done.
> 
> > > +Fallowing memory ranges are expected:
> > > +- For "fsl,imx7d-rproc"
> > > +	<0x00900000 0x00020000> - "ocram"
> > > +	<0x00920000 0x00020000> - "ocram-epdc"
> > > +	<0x00940000 0x00008000> - "ocram-pxp"
> > > +	<0x80000000 0x0FFF0000> - "ddr" (code area)
> > > +	<0x80000000 0x60000000> - "ddr" (data area)
> > 
> > There's no reason to list the actual regions in the binding
> > document. Just list the requires regions for each system.
> > 
> > > +- For "fsl,imx6sx-rproc"
> > > +	<0x008F8000 0x00004000> - "ocram-s"
> > > +	<0x80000000 0x0FFF8000> - "ddr" (code area)
> > > +	<0x80000000 0x60000000> - "ddr" (data area)
> > > +
> > > +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
> > > +than data area.  So this range should be carefully chosen according to your
> > > +system and application requirements.
> > > +
> > 
> > This is a source of future issues as this indicates that I should have
> > reg-names list "ddr" twice.
> 
> Then I will name it:
> "ddri" (incstruction/code area),
> "ddrd" (data area)
> 
> unless there are other suggestions.
> 

But are you saying that it's correct that these two memory regions
should overlap?

> > Also, as you warn about the user needing to pick the values for "ddr",
> > does that mean that it's a carveout of System RAM?
> 
> Yes, it is a part of System RAM.
> 

Then there will be an associated reserved-memory region for the
region(s), you should add a label to this and use "memory-region" to get
hold of the addresses instead.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-18 16:38         ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-18 16:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:

> Hallo Bjorn,
> 
> On 11.07.2017 00:14, Bjorn Andersson wrote:
> > On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
> > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > >  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
> > >  1 file changed, 44 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > new file mode 100644
> > > index 000000000000..e7c61993e1b8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
> > > @@ -0,0 +1,44 @@
> > > +NXP iMX6SX/iMX7D Co-Processor Bindings
> > > +----------------------------------------
> > > +
> > > +This binding provides support for ARM Cortex M4 Co-processor found on some
> > > +NXP iMX SoCs.
> > > +
> > > +Required properties:
> > > +- compatible		Should be one of:
> > > +				"fsl,imx7d-rproc"
> > > +				"fsl,imx6sx-rproc"
> > > +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
> > > +- syscfg		Phandle to syscon block which provide access to
> > 
> > This is called "syscon" in the code and the example.
> 
> done.
> 
> > > +			System Reset Controller
> > > +
> > > +Optional properties:
> > > +- reg:			Should contain the address ranges for some of internal
> > > +			memory regions.
> > 
> > Something less hand-wavy, like: "Should list the memory regions for the
> > remoteproc"
> > 
> > > +- reg-names:		Contains the corresponding names for the memory
> > > +			regions. These should be named "ddr", "ocram", "ocram-s",
> > > +			"ocram-epdc" or "ocram-pxp".
> > 
> > Make this comment define which memory regions are required for each of
> > the systems.
> 
> done.
> 
> > > +Fallowing memory ranges are expected:
> > > +- For "fsl,imx7d-rproc"
> > > +	<0x00900000 0x00020000> - "ocram"
> > > +	<0x00920000 0x00020000> - "ocram-epdc"
> > > +	<0x00940000 0x00008000> - "ocram-pxp"
> > > +	<0x80000000 0x0FFF0000> - "ddr" (code area)
> > > +	<0x80000000 0x60000000> - "ddr" (data area)
> > 
> > There's no reason to list the actual regions in the binding
> > document. Just list the requires regions for each system.
> > 
> > > +- For "fsl,imx6sx-rproc"
> > > +	<0x008F8000 0x00004000> - "ocram-s"
> > > +	<0x80000000 0x0FFF8000> - "ddr" (code area)
> > > +	<0x80000000 0x60000000> - "ddr" (data area)
> > > +
> > > +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
> > > +than data area.  So this range should be carefully chosen according to your
> > > +system and application requirements.
> > > +
> > 
> > This is a source of future issues as this indicates that I should have
> > reg-names list "ddr" twice.
> 
> Then I will name it:
> "ddri" (incstruction/code area),
> "ddrd" (data area)
> 
> unless there are other suggestions.
> 

But are you saying that it's correct that these two memory regions
should overlap?

> > Also, as you warn about the user needing to pick the values for "ddr",
> > does that mean that it's a carveout of System RAM?
> 
> Yes, it is a part of System RAM.
> 

Then there will be an associated reserved-memory region for the
region(s), you should add a label to this and use "memory-region" to get
hold of the addresses instead.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
  2017-07-18 16:38         ` Bjorn Andersson
  (?)
@ 2017-07-24  5:56           ` Oleksij Rempel
  -1 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-24  5:56 UTC (permalink / raw)
  To: Bjorn Andersson, Oleksij Rempel
  Cc: Oleksij Rempel, devicetree, kernel, linux-arm-kernel,
	linux-kernel, Rob Herring, Mark Rutland, Russell King, Shawn Guo,
	Fabio Estevam, Ohad Ben-Cohen, linux-remoteproc


[-- Attachment #1.1: Type: text/plain, Size: 3904 bytes --]

Am 18.07.2017 um 18:38 schrieb Bjorn Andersson:
> On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:
> 
>> Hallo Bjorn,
>>
>> On 11.07.2017 00:14, Bjorn Andersson wrote:
>>> On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>> ---
>>>>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>>>>  1 file changed, 44 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>> new file mode 100644
>>>> index 000000000000..e7c61993e1b8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>> @@ -0,0 +1,44 @@
>>>> +NXP iMX6SX/iMX7D Co-Processor Bindings
>>>> +----------------------------------------
>>>> +
>>>> +This binding provides support for ARM Cortex M4 Co-processor found on some
>>>> +NXP iMX SoCs.
>>>> +
>>>> +Required properties:
>>>> +- compatible		Should be one of:
>>>> +				"fsl,imx7d-rproc"
>>>> +				"fsl,imx6sx-rproc"
>>>> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
>>>> +- syscfg		Phandle to syscon block which provide access to
>>>
>>> This is called "syscon" in the code and the example.
>>
>> done.
>>
>>>> +			System Reset Controller
>>>> +
>>>> +Optional properties:
>>>> +- reg:			Should contain the address ranges for some of internal
>>>> +			memory regions.
>>>
>>> Something less hand-wavy, like: "Should list the memory regions for the
>>> remoteproc"
>>>
>>>> +- reg-names:		Contains the corresponding names for the memory
>>>> +			regions. These should be named "ddr", "ocram", "ocram-s",
>>>> +			"ocram-epdc" or "ocram-pxp".
>>>
>>> Make this comment define which memory regions are required for each of
>>> the systems.
>>
>> done.
>>
>>>> +Fallowing memory ranges are expected:
>>>> +- For "fsl,imx7d-rproc"
>>>> +	<0x00900000 0x00020000> - "ocram"
>>>> +	<0x00920000 0x00020000> - "ocram-epdc"
>>>> +	<0x00940000 0x00008000> - "ocram-pxp"
>>>> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
>>>> +	<0x80000000 0x60000000> - "ddr" (data area)
>>>
>>> There's no reason to list the actual regions in the binding
>>> document. Just list the requires regions for each system.
>>>
>>>> +- For "fsl,imx6sx-rproc"
>>>> +	<0x008F8000 0x00004000> - "ocram-s"
>>>> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
>>>> +	<0x80000000 0x60000000> - "ddr" (data area)
>>>> +
>>>> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
>>>> +than data area.  So this range should be carefully chosen according to your
>>>> +system and application requirements.
>>>> +
>>>
>>> This is a source of future issues as this indicates that I should have
>>> reg-names list "ddr" twice.
>>
>> Then I will name it:
>> "ddri" (incstruction/code area),
>> "ddrd" (data area)
>>
>> unless there are other suggestions.
>>
> 
> But are you saying that it's correct that these two memory regions
> should overlap?

Yes, from Cortex-m4 the same memory regions are aliased to different
address ranges to provide different cache optimizations.
From Cortex-A7 side - not. But on this side we don't care about meaning
of it, it is just some memory region.


>>> Also, as you warn about the user needing to pick the values for "ddr",
>>> does that mean that it's a carveout of System RAM?
>>
>> Yes, it is a part of System RAM.
>>
> 
> Then there will be an associated reserved-memory region for the
> region(s), you should add a label to this and use "memory-region" to get
> hold of the addresses instead.

Ok. Should only system memory region be assigned over reserved-memory or
all of named regions?

-- 
Regards,
Oleksij


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-24  5:56           ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-24  5:56 UTC (permalink / raw)
  To: Bjorn Andersson, Oleksij Rempel
  Cc: Mark Rutland, devicetree, Ohad Ben-Cohen, linux-remoteproc,
	linux-kernel, Russell King, Oleksij Rempel, Rob Herring, kernel,
	Fabio Estevam, Shawn Guo, linux-arm-kernel


[-- Attachment #1.1.1: Type: text/plain, Size: 3904 bytes --]

Am 18.07.2017 um 18:38 schrieb Bjorn Andersson:
> On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:
> 
>> Hallo Bjorn,
>>
>> On 11.07.2017 00:14, Bjorn Andersson wrote:
>>> On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>> ---
>>>>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>>>>  1 file changed, 44 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>> new file mode 100644
>>>> index 000000000000..e7c61993e1b8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>> @@ -0,0 +1,44 @@
>>>> +NXP iMX6SX/iMX7D Co-Processor Bindings
>>>> +----------------------------------------
>>>> +
>>>> +This binding provides support for ARM Cortex M4 Co-processor found on some
>>>> +NXP iMX SoCs.
>>>> +
>>>> +Required properties:
>>>> +- compatible		Should be one of:
>>>> +				"fsl,imx7d-rproc"
>>>> +				"fsl,imx6sx-rproc"
>>>> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
>>>> +- syscfg		Phandle to syscon block which provide access to
>>>
>>> This is called "syscon" in the code and the example.
>>
>> done.
>>
>>>> +			System Reset Controller
>>>> +
>>>> +Optional properties:
>>>> +- reg:			Should contain the address ranges for some of internal
>>>> +			memory regions.
>>>
>>> Something less hand-wavy, like: "Should list the memory regions for the
>>> remoteproc"
>>>
>>>> +- reg-names:		Contains the corresponding names for the memory
>>>> +			regions. These should be named "ddr", "ocram", "ocram-s",
>>>> +			"ocram-epdc" or "ocram-pxp".
>>>
>>> Make this comment define which memory regions are required for each of
>>> the systems.
>>
>> done.
>>
>>>> +Fallowing memory ranges are expected:
>>>> +- For "fsl,imx7d-rproc"
>>>> +	<0x00900000 0x00020000> - "ocram"
>>>> +	<0x00920000 0x00020000> - "ocram-epdc"
>>>> +	<0x00940000 0x00008000> - "ocram-pxp"
>>>> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
>>>> +	<0x80000000 0x60000000> - "ddr" (data area)
>>>
>>> There's no reason to list the actual regions in the binding
>>> document. Just list the requires regions for each system.
>>>
>>>> +- For "fsl,imx6sx-rproc"
>>>> +	<0x008F8000 0x00004000> - "ocram-s"
>>>> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
>>>> +	<0x80000000 0x60000000> - "ddr" (data area)
>>>> +
>>>> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
>>>> +than data area.  So this range should be carefully chosen according to your
>>>> +system and application requirements.
>>>> +
>>>
>>> This is a source of future issues as this indicates that I should have
>>> reg-names list "ddr" twice.
>>
>> Then I will name it:
>> "ddri" (incstruction/code area),
>> "ddrd" (data area)
>>
>> unless there are other suggestions.
>>
> 
> But are you saying that it's correct that these two memory regions
> should overlap?

Yes, from Cortex-m4 the same memory regions are aliased to different
address ranges to provide different cache optimizations.
From Cortex-A7 side - not. But on this side we don't care about meaning
of it, it is just some memory region.


>>> Also, as you warn about the user needing to pick the values for "ddr",
>>> does that mean that it's a carveout of System RAM?
>>
>> Yes, it is a part of System RAM.
>>
> 
> Then there will be an associated reserved-memory region for the
> region(s), you should add a label to this and use "memory-region" to get
> hold of the addresses instead.

Ok. Should only system memory region be assigned over reserved-memory or
all of named regions?

-- 
Regards,
Oleksij


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-24  5:56           ` Oleksij Rempel
  0 siblings, 0 replies; 24+ messages in thread
From: Oleksij Rempel @ 2017-07-24  5:56 UTC (permalink / raw)
  To: linux-arm-kernel

Am 18.07.2017 um 18:38 schrieb Bjorn Andersson:
> On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:
> 
>> Hallo Bjorn,
>>
>> On 11.07.2017 00:14, Bjorn Andersson wrote:
>>> On Mon 10 Jul 07:42 PDT 2017, Oleksij Rempel wrote:
>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>> ---
>>>>  .../devicetree/bindings/remoteproc/imx-rproc.txt   | 44 ++++++++++++++++++++++
>>>>  1 file changed, 44 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>> new file mode 100644
>>>> index 000000000000..e7c61993e1b8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/imx-rproc.txt
>>>> @@ -0,0 +1,44 @@
>>>> +NXP iMX6SX/iMX7D Co-Processor Bindings
>>>> +----------------------------------------
>>>> +
>>>> +This binding provides support for ARM Cortex M4 Co-processor found on some
>>>> +NXP iMX SoCs.
>>>> +
>>>> +Required properties:
>>>> +- compatible		Should be one of:
>>>> +				"fsl,imx7d-rproc"
>>>> +				"fsl,imx6sx-rproc"
>>>> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
>>>> +- syscfg		Phandle to syscon block which provide access to
>>>
>>> This is called "syscon" in the code and the example.
>>
>> done.
>>
>>>> +			System Reset Controller
>>>> +
>>>> +Optional properties:
>>>> +- reg:			Should contain the address ranges for some of internal
>>>> +			memory regions.
>>>
>>> Something less hand-wavy, like: "Should list the memory regions for the
>>> remoteproc"
>>>
>>>> +- reg-names:		Contains the corresponding names for the memory
>>>> +			regions. These should be named "ddr", "ocram", "ocram-s",
>>>> +			"ocram-epdc" or "ocram-pxp".
>>>
>>> Make this comment define which memory regions are required for each of
>>> the systems.
>>
>> done.
>>
>>>> +Fallowing memory ranges are expected:
>>>> +- For "fsl,imx7d-rproc"
>>>> +	<0x00900000 0x00020000> - "ocram"
>>>> +	<0x00920000 0x00020000> - "ocram-epdc"
>>>> +	<0x00940000 0x00008000> - "ocram-pxp"
>>>> +	<0x80000000 0x0FFF0000> - "ddr" (code area)
>>>> +	<0x80000000 0x60000000> - "ddr" (data area)
>>>
>>> There's no reason to list the actual regions in the binding
>>> document. Just list the requires regions for each system.
>>>
>>>> +- For "fsl,imx6sx-rproc"
>>>> +	<0x008F8000 0x00004000> - "ocram-s"
>>>> +	<0x80000000 0x0FFF8000> - "ddr" (code area)
>>>> +	<0x80000000 0x60000000> - "ddr" (data area)
>>>> +
>>>> +Note: the "ddr" code and data ranges are overlapping. Code area is smaller
>>>> +than data area.  So this range should be carefully chosen according to your
>>>> +system and application requirements.
>>>> +
>>>
>>> This is a source of future issues as this indicates that I should have
>>> reg-names list "ddr" twice.
>>
>> Then I will name it:
>> "ddri" (incstruction/code area),
>> "ddrd" (data area)
>>
>> unless there are other suggestions.
>>
> 
> But are you saying that it's correct that these two memory regions
> should overlap?

Yes, from Cortex-m4 the same memory regions are aliased to different
address ranges to provide different cache optimizations.
>From Cortex-A7 side - not. But on this side we don't care about meaning
of it, it is just some memory region.


>>> Also, as you warn about the user needing to pick the values for "ddr",
>>> does that mean that it's a carveout of System RAM?
>>
>> Yes, it is a part of System RAM.
>>
> 
> Then there will be an associated reserved-memory region for the
> region(s), you should add a label to this and use "memory-region" to get
> hold of the addresses instead.

Ok. Should only system memory region be assigned over reserved-memory or
all of named regions?

-- 
Regards,
Oleksij

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/536bdd4b/attachment.sig>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-26 17:11             ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-26 17:11 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Oleksij Rempel, Oleksij Rempel, devicetree, kernel,
	linux-arm-kernel, linux-kernel, Rob Herring, Mark Rutland,
	Russell King, Shawn Guo, Fabio Estevam, Ohad Ben-Cohen,
	linux-remoteproc

On Sun 23 Jul 22:56 PDT 2017, Oleksij Rempel wrote:

> Am 18.07.2017 um 18:38 schrieb Bjorn Andersson:
> > On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:
> >> On 11.07.2017 00:14, Bjorn Andersson wrote:
[..]
> >>> Also, as you warn about the user needing to pick the values for "ddr",
> >>> does that mean that it's a carveout of System RAM?
> >>
> >> Yes, it is a part of System RAM.
> >>
> > 
> > Then there will be an associated reserved-memory region for the
> > region(s), you should add a label to this and use "memory-region" to get
> > hold of the addresses instead.
> 
> Ok. Should only system memory region be assigned over reserved-memory or
> all of named regions?
> 

You can do it either way, but unless you see a strong reason against it
I would recommend just having a single region, for simplicity.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-26 17:11             ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-26 17:11 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Oleksij Rempel, Oleksij Rempel,
	devicetree-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Russell King, Shawn Guo, Fabio Estevam, Ohad Ben-Cohen,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA

On Sun 23 Jul 22:56 PDT 2017, Oleksij Rempel wrote:

> Am 18.07.2017 um 18:38 schrieb Bjorn Andersson:
> > On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:
> >> On 11.07.2017 00:14, Bjorn Andersson wrote:
[..]
> >>> Also, as you warn about the user needing to pick the values for "ddr",
> >>> does that mean that it's a carveout of System RAM?
> >>
> >> Yes, it is a part of System RAM.
> >>
> > 
> > Then there will be an associated reserved-memory region for the
> > region(s), you should add a label to this and use "memory-region" to get
> > hold of the addresses instead.
> 
> Ok. Should only system memory region be assigned over reserved-memory or
> all of named regions?
> 

You can do it either way, but unless you see a strong reason against it
I would recommend just having a single region, for simplicity.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver
@ 2017-07-26 17:11             ` Bjorn Andersson
  0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2017-07-26 17:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun 23 Jul 22:56 PDT 2017, Oleksij Rempel wrote:

> Am 18.07.2017 um 18:38 schrieb Bjorn Andersson:
> > On Tue 18 Jul 01:45 PDT 2017, Oleksij Rempel wrote:
> >> On 11.07.2017 00:14, Bjorn Andersson wrote:
[..]
> >>> Also, as you warn about the user needing to pick the values for "ddr",
> >>> does that mean that it's a carveout of System RAM?
> >>
> >> Yes, it is a part of System RAM.
> >>
> > 
> > Then there will be an associated reserved-memory region for the
> > region(s), you should add a label to this and use "memory-region" to get
> > hold of the addresses instead.
> 
> Ok. Should only system memory region be assigned over reserved-memory or
> all of named regions?
> 

You can do it either way, but unless you see a strong reason against it
I would recommend just having a single region, for simplicity.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2017-07-26 17:11 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-10 14:42 [PATCH v1 0/2] provide imx rproc driver Oleksij Rempel
2017-07-10 14:42 ` Oleksij Rempel
2017-07-10 14:42 ` Oleksij Rempel
2017-07-10 14:42 ` [PATCH v1 1/2] remoteproc: dt: Provide bindings for iMX6SX/7D Remote Processor Controller driver Oleksij Rempel
2017-07-10 14:42   ` Oleksij Rempel
2017-07-10 22:14   ` Bjorn Andersson
2017-07-10 22:14     ` Bjorn Andersson
2017-07-10 22:14     ` Bjorn Andersson
2017-07-18  8:45     ` Oleksij Rempel
2017-07-18  8:45       ` Oleksij Rempel
2017-07-18  8:45       ` Oleksij Rempel
2017-07-18 16:38       ` Bjorn Andersson
2017-07-18 16:38         ` Bjorn Andersson
2017-07-18 16:38         ` Bjorn Andersson
2017-07-18 16:38         ` Bjorn Andersson
2017-07-24  5:56         ` Oleksij Rempel
2017-07-24  5:56           ` Oleksij Rempel
2017-07-24  5:56           ` Oleksij Rempel
2017-07-26 17:11           ` Bjorn Andersson
2017-07-26 17:11             ` Bjorn Andersson
2017-07-26 17:11             ` Bjorn Andersson
2017-07-10 14:42 ` [PATCH v1 2/2] remoteproc: imx_rproc: add a NXP/Freescale imx_rproc driver Oleksij Rempel
2017-07-10 14:42   ` Oleksij Rempel
2017-07-10 14:42   ` Oleksij Rempel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.