[PATCH v2 0/6] Add support for Tegra GMI bus controller

[PATCH v2 0/6] Add support for Tegra GMI bus controller

[Mirza Krak]

From: Mirza Krak <mirza.krak@gmail.com>

Hi.

This is a follow up to my previous RFC to add support for Tegra GMI bus
controller.

I have tested this series on a Tegra30 using a Colibri T30 SOM on a custom
carrier board which has multiple CAN controllers (SJA1000) connected to the
GMI bus.

I have rebased on top of latest tegra/for-next in V2. Also see individual
patches for changes in V2.

See below links for previous discussions.

Comments on RFC:
https://marc.info/?l=linux-clk&m=146893557629903&w=2
https://marc.info/?l=linux-tegra&m=146893541829801&w=2
https://marc.info/?l=linux-tegra&m=146893542429814&w=2

Comments on V1:
https://marc.info/?l=linux-arm-kernel&m=147051551821122&w=2
https://marc.info/?l=linux-arm-kernel&m=147051553121150&w=2
https://marc.info/?l=linux-arm-kernel&m=147194856600627&w=2
https://marc.info/?l=linux-arm-kernel&m=147072742432211&w=2


Mirza Krak (6):
  clk: tegra: add TEGRA20_CLK_NOR to init table
  clk: tegra: add TEGRA30_CLK_NOR to init table
  dt/bindings: Add bindings for Tegra GMI controller
  ARM: tegra: Add Tegra30 GMI support
  ARM: tegra: Add Tegra20 GMI support
  bus: Add support for Tegra Generic Memory Interface

 .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 ++++++++++++
 arch/arm/boot/dts/tegra20.dtsi                     |  13 ++
 arch/arm/boot/dts/tegra30.dtsi                     |  12 ++
 drivers/bus/Kconfig                                |   8 +
 drivers/bus/Makefile                               |   1 +
 drivers/bus/tegra-gmi.c                            | 231 +++++++++++++++++++++
 drivers/clk/tegra/clk-tegra20.c                    |   1 +
 drivers/clk/tegra/clk-tegra30.c                    |   1 +
 8 files changed, 399 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
 create mode 100644 drivers/bus/tegra-gmi.c

--
2.1.4

[PATCH v2 1/6] clk: tegra: add TEGRA20_CLK_NOR to init table

[Mirza Krak]

From: Mirza Krak <mirza.krak@gmail.com>

Add TEGRA20_CLK_NOR to init tabel and set default rate to 92 MHz which
is max rate.

Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
---
Changes in v2:
- no changes

 drivers/clk/tegra/clk-tegra20.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 837e5cb..13d3b5a 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1047,6 +1047,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA20_CLK_SDMMC3, TEGRA20_CLK_PLL_P, 48000000, 0 },
 	{ TEGRA20_CLK_SDMMC4, TEGRA20_CLK_PLL_P, 48000000, 0 },
 	{ TEGRA20_CLK_SPI, TEGRA20_CLK_PLL_P, 20000000, 0 },
+	{ TEGRA20_CLK_NOR, TEGRA20_CLK_PLL_P, 92000000, 0 },
 	{ TEGRA20_CLK_SBC1, TEGRA20_CLK_PLL_P, 100000000, 0 },
 	{ TEGRA20_CLK_SBC2, TEGRA20_CLK_PLL_P, 100000000, 0 },
 	{ TEGRA20_CLK_SBC3, TEGRA20_CLK_PLL_P, 100000000, 0 },
--
2.1.4

[PATCH v2 2/6] clk: tegra: add TEGRA30_CLK_NOR to init table

[Mirza Krak]

From: Mirza Krak <mirza.krak@gmail.com>

Add TEGRA30_CLK_NOR to init table and set default rate to 127 MHz which
is max rate.

Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
---
Changes in v2:
- no changes

 drivers/clk/tegra/clk-tegra30.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 8e2db5e..67f1677 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1252,6 +1252,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA30_CLK_SDMMC1, TEGRA30_CLK_PLL_P, 48000000, 0 },
 	{ TEGRA30_CLK_SDMMC2, TEGRA30_CLK_PLL_P, 48000000, 0 },
 	{ TEGRA30_CLK_SDMMC3, TEGRA30_CLK_PLL_P, 48000000, 0 },
+	{ TEGRA30_CLK_NOR, TEGRA30_CLK_PLL_P, 127000000, 0 },
 	{ TEGRA30_CLK_PLL_M, TEGRA30_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA30_CLK_PCLK, TEGRA30_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA30_CLK_CSITE, TEGRA30_CLK_CLK_MAX, 0, 1 },
--
2.1.4

[PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Mirza Krak]

From: Mirza Krak <mirza.krak@gmail.com>

Document the devicetree bindings for the Generic Memory Interface (GMI)
bus driver found on Tegra SOCs.

Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
---
Changes in v2:
- Updated examples and some information based on comments from Jon Hunter.

 .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 +++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt

diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
new file mode 100644
index 0000000..8c1e15f
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
@@ -0,0 +1,132 @@
+Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
+
+The Generic Memory Interface bus enables memory transfers between internal and
+external memory. Can be used to attach various high speed devices such as
+synchronous/asynchronous NOR, FPGA, UARTS and more.
+
+The actual devices are instantiated from the child nodes of a GMI node.
+
+Required properties:
+ - compatible : Should contain one of the following:
+        For Tegra20 must contain "nvidia,tegra20-gmi".
+        For Tegra30 must contain "nvidia,tegra30-gmi".
+ - reg: Should contain GMI controller registers location and length.
+ - clocks: Must contain an entry for each entry in clock-names.
+ - clock-names: Must include the following entries: "gmi"
+ - resets : Must contain an entry for each entry in reset-names.
+ - reset-names : Must include the following entries: "gmi"
+ - #address-cells: The number of cells used to represent physical base
+   addresses in the GMI address space. Should be 1.
+ - #size-cells: The number of cells used to represent the size of an address
+   range in the GMI address space. Should be 1.
+ - ranges: Must be set up to reflect the memory layout with three integer values
+   for each chip-select line in use (only one entry is supported, see below
+   comments):
+   <cs-number> <physical address of mapping> <size>
+
+Note that the GMI controller does not have any internal chip-select address
+decoding, because of that chip-selects either need to be managed via software
+or by employing external chip-select decoding logic.
+
+If external chip-select logic is used to support multiple devices it is assumed
+that the devices use the same timing and so are probably the same type. It also
+assumes that they can fit in the 256MB address range. In this case only one
+child device is supported which represents the active chip-select line, see
+examples for more insight.
+
+Required child cs node properties:
+ - reg: First entry should contain the active chip-select number
+
+Optional child cs node properties:
+
+ - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is 16bit.
+ - nvidia,snor-mux-mode: Enable address/data MUX mode.
+ - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle before data.
+   If omitted it will be asserted with data.
+ - nvidia,snor-rdy-inv: RDY signal is active high
+ - nvidia,snor-adv-inv: ADV signal is active high
+ - nvidia,snor-oe-inv: WE/OE signal is active high
+ - nvidia,snor-cs-inv: CS signal is active high
+
+  Note that there is some special handling for the timing values.
+  From Tegra TRM:
+  Programming 0 means 1 clock cycle: actual cycle = programmed cycle + 1
+
+ - nvidia,snor-muxed-width: Number of cycles MUX address/data asserted on the
+   bus. Valid values are 0-15, default is 1
+ - nvidia,snor-hold-width: Number of cycles CE stays asserted after the
+   de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
+   (in case of MASTER Request). Valid values are 0-15, default is 1
+ - nvidia,snor-adv-width: Number of cycles during which ADV stays asserted.
+   Valid values are 0-15, default is 1.
+ - nvidia,snor-ce-width: Number of cycles before CE is asserted.
+   Valid values are 0-15, default is 4
+ - nvidia,snor-we-width: Number of cycles during which WE stays asserted.
+   Valid values are 0-15, default is 1
+ - nvidia,snor-oe-width: Number of cycles during which OE stays asserted.
+   Valid values are 0-255, default is 1
+ - nvidia,snor-wait-width: Number of cycles before READY is asserted.
+   Valid values are 0-255, default is 3
+
+Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
+controllers with a simple-bus node since they are all connected to the same
+chip-select (CS4), in this example external address decoding is provided:
+
+gmi@70090000 {
+	compatible = "nvidia,tegra20-gmi";
+	reg = <0x70009000 0x1000>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+	clocks = <&tegra_car TEGRA20_CLK_NOR>;
+	clock-names = "gmi";
+	resets = <&tegra_car 42>;
+	reset-names = "gmi";
+	ranges = <4 0x48000000 0x7ffffff>;
+
+	status = "disabled";
+
+	bus@4 {
+		compatible = "simple-bus";
+		reg = <4>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 4 0x40100>;
+
+		nvidia,snor-mux-mode;
+		nvidia,snor-adv-inv;
+
+		can@0 {
+			reg = <0 0x100>;
+			...
+		};
+
+		can@40000 {
+			reg = <0x40000 0x100>;
+			...
+		};
+	};
+};
+
+Example with one SJA1000 CAN controller connected to the GMI bus
+on CS4:
+
+gmi@70090000 {
+	compatible = "nvidia,tegra20-gmi";
+	reg = <0x70009000 0x1000>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+	clocks = <&tegra_car TEGRA20_CLK_NOR>;
+	clock-names = "gmi";
+	resets = <&tegra_car 42>;
+	reset-names = "gmi";
+	ranges = <4 0x48000000 0x7ffffff>;
+
+	status = "disabled";
+
+	can@4 {
+		reg = <4 0x100>;
+		...
+		nvidia,snor-mux-mode;
+		nvidia,snor-adv-inv;
+	};
+};
--
2.1.4

[PATCH v2 4/6] ARM: tegra: Add Tegra30 GMI support

[Mirza Krak]

From: Mirza Krak <mirza.krak@gmail.com>

Add a device node for the GMI controller found on Tegra30.

Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
---
Changes in v2:
- added address-cells, size-cells and ranges properties

 arch/arm/boot/dts/tegra30.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 5030065..1b26308 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -439,6 +439,19 @@
 		status = "disabled";
 	};

+	gmi@70009000 {
+		compatible = "nvidia,tegra30-gmi";
+		reg = <0x70009000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x48000000 0x7ffffff>;
+		clocks = <&tegra_car TEGRA30_CLK_NOR>;
+		clock-names = "gmi";
+		resets = <&tegra_car 42>;
+		reset-names = "gmi";
+		status = "disabled";
+	};
+
 	pwm: pwm@7000a000 {
 		compatible = "nvidia,tegra30-pwm", "nvidia,tegra20-pwm";
 		reg = <0x7000a000 0x100>;
--
2.1.4

[PATCH v2 5/6] ARM: tegra: Add Tegra20 GMI support

[Mirza Krak]

From: Mirza Krak <mirza.krak@gmail.com>

Add a device node for the GMI controller found on Tegra20.

Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
---
Changes in v2:
- added address-cells, size-cells and ranges properties

 arch/arm/boot/dts/tegra20.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 2207c08..03eb029 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -376,6 +376,20 @@
 		status = "disabled";
 	};

+
+	gmi@70009000 {
+		compatible = "nvidia,tegra20-gmi";
+		reg = <70009000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x48000000 0x7ffffff>;
+		clocks = <&tegra_car TEGRA20_CLK_NOR>;
+		clock-names = "gmi";
+		resets = < &tegra_car 42>;
+		reset-names = "gmi";
+		status = "disabled";
+	};
+
 	pwm: pwm@7000a000 {
 		compatible = "nvidia,tegra20-pwm";
 		reg = <0x7000a000 0x100>;
--
2.1.4

[PATCH v2 6/6] bus: Add support for Tegra Generic Memory Interface

[Mirza Krak]

From: Mirza Krak <mirza.krak@gmail.com>

The Generic Memory Interface bus can be used to connect high-speed
devices such as NOR flash, FPGAs, DSPs...

Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
---
Changes in v2:
- Fixed some checkpatch errors
- Re-ordered probe to get rid of local variables
- Moved of_platform_default_populate call to the end of probe
- Use the timing and configuration properties from the child device
- Added warning if more then 1 child device exist


 drivers/bus/Kconfig     |   8 ++
 drivers/bus/Makefile    |   1 +
 drivers/bus/tegra-gmi.c | 231 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 240 insertions(+)
 create mode 100644 drivers/bus/tegra-gmi.c

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 4ed7d26..2e75a7f 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -141,6 +141,14 @@ config TEGRA_ACONNECT
 	  Driver for the Tegra ACONNECT bus which is used to interface with
 	  the devices inside the Audio Processing Engine (APE) for Tegra210.

+config TEGRA_GMI
+	tristate "Tegra Generic Memory Interface bus driver"
+	depends on ARCH_TEGRA
+	help
+	  Driver for the Tegra Generic Memory Interface bus which can be used
+	  to attach devices such as NOR, UART, FPGA and more.
+
+
 config UNIPHIER_SYSTEM_BUS
 	tristate "UniPhier System Bus driver"
 	depends on ARCH_UNIPHIER && OF
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index ac84cc4..34e2bab 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -18,5 +18,6 @@ obj-$(CONFIG_OMAP_OCP2SCP)	+= omap-ocp2scp.o
 obj-$(CONFIG_SUNXI_RSB)		+= sunxi-rsb.o
 obj-$(CONFIG_SIMPLE_PM_BUS)	+= simple-pm-bus.o
 obj-$(CONFIG_TEGRA_ACONNECT)	+= tegra-aconnect.o
+obj-$(CONFIG_TEGRA_GMI)		+= tegra-gmi.o
 obj-$(CONFIG_UNIPHIER_SYSTEM_BUS)	+= uniphier-system-bus.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
diff --git a/drivers/bus/tegra-gmi.c b/drivers/bus/tegra-gmi.c
new file mode 100644
index 0000000..b068ef9
--- /dev/null
+++ b/drivers/bus/tegra-gmi.c
@@ -0,0 +1,231 @@
+/*
+ * Driver for NVIDIA Generic Memory Interface
+ *
+ * Copyright (C) 2016 Host Mobility AB. All rights reserved.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/reset.h>
+
+#define TEGRA_GMI_CONFIG		0x00
+#define TEGRA_GMI_CONFIG_GO		BIT(31)
+#define TEGRA_GMI_BUS_WIDTH_32BIT	BIT(30)
+#define TEGRA_GMI_MUX_MODE		BIT(28)
+#define TEGRA_GMI_RDY_BEFORE_DATA	BIT(24)
+#define TEGRA_GMI_RDY_ACTIVE_HIGH	BIT(23)
+#define TEGRA_GMI_ADV_ACTIVE_HIGH	BIT(22)
+#define TEGRA_GMI_OE_ACTIVE_HIGH	BIT(21)
+#define TEGRA_GMI_CS_ACTIVE_HIGH	BIT(20)
+#define TEGRA_GMI_CS_SELECT(x)		((x & 0x7) << 4)
+
+#define TEGRA_GMI_TIMING0		0x10
+#define TEGRA_GMI_MUXED_WIDTH(x)	((x & 0xf) << 12)
+#define TEGRA_GMI_HOLD_WIDTH(x)		((x & 0xf) << 8)
+#define TEGRA_GMI_ADV_WIDTH(x)		((x & 0xf) << 4)
+#define TEGRA_GMI_CE_WIDTH(x)		(x & 0xf)
+
+#define TEGRA_GMI_TIMING1		0x14
+#define TEGRA_GMI_WE_WIDTH(x)		((x & 0xff) << 16)
+#define TEGRA_GMI_OE_WIDTH(x)		((x & 0xff) << 8)
+#define TEGRA_GMI_WAIT_WIDTH(x)		(x & 0xff)
+
+struct tegra_gmi_priv {
+	void __iomem *base;
+	struct reset_control *rst;
+	struct clk *clk;
+
+	u32 snor_config;
+	u32 snor_timing0;
+	u32 snor_timing1;
+};
+
+static void tegra_gmi_init(struct device *dev, struct tegra_gmi_priv *priv)
+{
+	writel(priv->snor_timing0, priv->base + TEGRA_GMI_TIMING0);
+	writel(priv->snor_timing1, priv->base + TEGRA_GMI_TIMING1);
+
+	priv->snor_config |= TEGRA_GMI_CONFIG_GO;
+	writel(priv->snor_config, priv->base + TEGRA_GMI_CONFIG);
+}
+
+static void tegra_gmi_parse_dt(struct device *dev, struct tegra_gmi_priv *priv)
+{
+	struct device_node *child = of_get_next_available_child(dev->of_node,
+		NULL);
+	u32 property;
+
+	if (!child) {
+		dev_warn(dev, "no child nodes found\n");
+		return;
+	}
+
+	/*
+	 * We currently only support one child device due to lack of
+	 * chip-select address decoding. Which means that we only have one
+	 * chip-select line from the GMI controller.
+	 */
+	if (of_get_child_count(dev->of_node) > 1)
+		dev_warn(dev, "only one child device is supported.");
+
+	if (of_property_read_bool(child, "nvidia,snor-data-width-32bit"))
+		priv->snor_config |= TEGRA_GMI_BUS_WIDTH_32BIT;
+
+	if (of_property_read_bool(child, "nvidia,snor-mux-mode"))
+		priv->snor_config |= TEGRA_GMI_MUX_MODE;
+
+	if (of_property_read_bool(child, "nvidia,snor-rdy-active-before-data"))
+		priv->snor_config |= TEGRA_GMI_RDY_BEFORE_DATA;
+
+	if (of_property_read_bool(child, "nvidia,snor-rdy-inv"))
+		priv->snor_config |= TEGRA_GMI_RDY_ACTIVE_HIGH;
+
+	if (of_property_read_bool(child, "nvidia,snor-adv-inv"))
+		priv->snor_config |= TEGRA_GMI_ADV_ACTIVE_HIGH;
+
+	if (of_property_read_bool(child, "nvidia,snor-oe-inv"))
+		priv->snor_config |= TEGRA_GMI_OE_ACTIVE_HIGH;
+
+	if (of_property_read_bool(child, "nvidia,snor-cs-inv"))
+		priv->snor_config |= TEGRA_GMI_CS_ACTIVE_HIGH;
+
+	/* Use the reg property to determine which CS is to be used. */
+	if (!of_property_read_u32(child, "reg", &property))
+		priv->snor_config |= TEGRA_GMI_CS_SELECT(property);
+
+	/* The default values that are provided below are reset values */
+	if (!of_property_read_u32(child, "nvidia,snor-muxed-width", &property))
+		priv->snor_timing0 |= TEGRA_GMI_MUXED_WIDTH(property);
+	else
+		priv->snor_timing0 |= TEGRA_GMI_MUXED_WIDTH(1);
+
+	if (!of_property_read_u32(child, "nvidia,snor-hold-width", &property))
+		priv->snor_timing0 |= TEGRA_GMI_HOLD_WIDTH(property);
+	else
+		priv->snor_timing0 |= TEGRA_GMI_HOLD_WIDTH(1);
+
+	if (!of_property_read_u32(child, "nvidia,snor-adv-width", &property))
+		priv->snor_timing0 |= TEGRA_GMI_ADV_WIDTH(property);
+	else
+		priv->snor_timing0 |= TEGRA_GMI_ADV_WIDTH(1);
+
+	if (!of_property_read_u32(child, "nvidia,snor-ce-width", &property))
+		priv->snor_timing0 |= TEGRA_GMI_CE_WIDTH(property);
+	else
+		priv->snor_timing0 |= TEGRA_GMI_CE_WIDTH(4);
+
+	if (!of_property_read_u32(child, "nvidia,snor-we-width", &property))
+		priv->snor_timing1 |= TEGRA_GMI_WE_WIDTH(property);
+	else
+		priv->snor_timing1 |= TEGRA_GMI_WE_WIDTH(1);
+
+	if (!of_property_read_u32(child, "nvidia,snor-oe-width", &property))
+		priv->snor_timing1 |= TEGRA_GMI_OE_WIDTH(property);
+	else
+		priv->snor_timing1 |= TEGRA_GMI_OE_WIDTH(1);
+
+	if (!of_property_read_u32(child, "nvidia,snor-wait-width", &property))
+		priv->snor_timing1 |= TEGRA_GMI_WAIT_WIDTH(property);
+	else
+		priv->snor_timing1 |= TEGRA_GMI_WAIT_WIDTH(3);
+
+	of_node_put(child);
+}
+
+static int tegra_gmi_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	struct tegra_gmi_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->clk = devm_clk_get(dev, "gmi");
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "can not get clock\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	priv->rst = devm_reset_control_get(dev, "gmi");
+	if (IS_ERR(priv->rst)) {
+		dev_err(dev, "can not get reset\n");
+		return PTR_ERR(priv->rst);
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "fail to enable clock.\n");
+		return ret;
+	}
+
+	reset_control_assert(priv->rst);
+	udelay(2);
+	reset_control_deassert(priv->rst);
+
+	tegra_gmi_parse_dt(dev, priv);
+	tegra_gmi_init(dev, priv);
+
+	ret = of_platform_default_populate(dev->of_node, NULL, dev);
+	if (ret < 0) {
+		dev_err(dev, "fail to create devices.\n");
+		clk_disable_unprepare(priv->clk);
+		reset_control_assert(priv->rst);
+		return ret;
+	}
+
+	dev_set_drvdata(dev, priv);
+
+	return 0;
+}
+
+static int tegra_gmi_remove(struct platform_device *pdev)
+{
+	struct tegra_gmi_priv *priv = dev_get_drvdata(&pdev->dev);
+	u32 config;
+
+	of_platform_depopulate(&pdev->dev);
+
+	config = readl(priv->base + TEGRA_GMI_CONFIG);
+	config &= ~TEGRA_GMI_CONFIG_GO;
+	writel(config, priv->base + TEGRA_GMI_CONFIG);
+
+	clk_disable_unprepare(priv->clk);
+	reset_control_assert(priv->rst);
+
+	return 0;
+}
+
+static const struct of_device_id tegra_gmi_id_table[] = {
+	{ .compatible = "nvidia,tegra20-gmi", },
+	{ .compatible = "nvidia,tegra30-gmi", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tegra_gmi_id_table);
+
+static struct platform_driver tegra_gmi_driver = {
+	.probe = tegra_gmi_probe,
+	.remove = tegra_gmi_remove,
+	.driver = {
+		.name		= "tegra-gmi",
+		.of_match_table	= tegra_gmi_id_table,
+	},
+};
+module_platform_driver(tegra_gmi_driver);
+
+MODULE_AUTHOR("Mirza Krak <mirza.krak@gmail.com");
+MODULE_DESCRIPTION("NVIDIA Tegra GMI Bus Driver");
+MODULE_LICENSE("GPL v2");
--
2.1.4

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Jon Hunter]

On 24/08/16 14:37, Mirza Krak wrote:
> From: Mirza Krak <mirza.krak@gmail.com>
> 
> Document the devicetree bindings for the Generic Memory Interface (GMI)
> bus driver found on Tegra SOCs.
> 
> Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
> ---
> Changes in v2:
> - Updated examples and some information based on comments from Jon Hunter.
> 
>  .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 +++++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
> 
> diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
> new file mode 100644
> index 0000000..8c1e15f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
> @@ -0,0 +1,132 @@
> +Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
> +
> +The Generic Memory Interface bus enables memory transfers between internal and
> +external memory. Can be used to attach various high speed devices such as
> +synchronous/asynchronous NOR, FPGA, UARTS and more.
> +
> +The actual devices are instantiated from the child nodes of a GMI node.
> +
> +Required properties:
> + - compatible : Should contain one of the following:
> +        For Tegra20 must contain "nvidia,tegra20-gmi".
> +        For Tegra30 must contain "nvidia,tegra30-gmi".
> + - reg: Should contain GMI controller registers location and length.
> + - clocks: Must contain an entry for each entry in clock-names.
> + - clock-names: Must include the following entries: "gmi"
> + - resets : Must contain an entry for each entry in reset-names.
> + - reset-names : Must include the following entries: "gmi"
> + - #address-cells: The number of cells used to represent physical base
> +   addresses in the GMI address space. Should be 1.
> + - #size-cells: The number of cells used to represent the size of an address
> +   range in the GMI address space. Should be 1.
> + - ranges: Must be set up to reflect the memory layout with three integer values
> +   for each chip-select line in use (only one entry is supported, see below
> +   comments):
> +   <cs-number> <physical address of mapping> <size>
> +
> +Note that the GMI controller does not have any internal chip-select address
> +decoding, because of that chip-selects either need to be managed via software
> +or by employing external chip-select decoding logic.
> +
> +If external chip-select logic is used to support multiple devices it is assumed
> +that the devices use the same timing and so are probably the same type. It also
> +assumes that they can fit in the 256MB address range. In this case only one
> +child device is supported which represents the active chip-select line, see
> +examples for more insight.
> +
> +Required child cs node properties:
> + - reg: First entry should contain the active chip-select number
> +
> +Optional child cs node properties:
> +
> + - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is 16bit.
> + - nvidia,snor-mux-mode: Enable address/data MUX mode.
> + - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle before data.
> +   If omitted it will be asserted with data.
> + - nvidia,snor-rdy-inv: RDY signal is active high
> + - nvidia,snor-adv-inv: ADV signal is active high
> + - nvidia,snor-oe-inv: WE/OE signal is active high
> + - nvidia,snor-cs-inv: CS signal is active high
> +
> +  Note that there is some special handling for the timing values.
> +  From Tegra TRM:
> +  Programming 0 means 1 clock cycle: actual cycle = programmed cycle + 1
> +
> + - nvidia,snor-muxed-width: Number of cycles MUX address/data asserted on the
> +   bus. Valid values are 0-15, default is 1
> + - nvidia,snor-hold-width: Number of cycles CE stays asserted after the
> +   de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
> +   (in case of MASTER Request). Valid values are 0-15, default is 1
> + - nvidia,snor-adv-width: Number of cycles during which ADV stays asserted.
> +   Valid values are 0-15, default is 1.
> + - nvidia,snor-ce-width: Number of cycles before CE is asserted.
> +   Valid values are 0-15, default is 4
> + - nvidia,snor-we-width: Number of cycles during which WE stays asserted.
> +   Valid values are 0-15, default is 1
> + - nvidia,snor-oe-width: Number of cycles during which OE stays asserted.
> +   Valid values are 0-255, default is 1
> + - nvidia,snor-wait-width: Number of cycles before READY is asserted.
> +   Valid values are 0-255, default is 3
> +
> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
> +controllers with a simple-bus node since they are all connected to the same
> +chip-select (CS4), in this example external address decoding is provided:
> +
> +gmi@70090000 {
> +	compatible = "nvidia,tegra20-gmi";
> +	reg = <0x70009000 0x1000>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	clocks = <&tegra_car TEGRA20_CLK_NOR>;
> +	clock-names = "gmi";
> +	resets = <&tegra_car 42>;
> +	reset-names = "gmi";
> +	ranges = <4 0x48000000 0x7ffffff>;
> +
> +	status = "disabled";
> +
> +	bus@4 {
> +		compatible = "simple-bus";
> +		reg = <4>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 4 0x40100>;

Does this work? I tried to add an example like this and I got ...

Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
length (4 bytes) (#address-cells == 1, #size-cells == 1)

I am wondering if we should just following the arm,pl172 example and
have ...

	cs4 {
		compatible = "simple-bus";
		#address-cells = <1>;
		#size-cells = <1>;
		ranges;

		nvidia,snor-cs = <4>;
		nvidia,snor-mux-mode;
		nvidia,snor-adv-inv;

		can@0 {
			reg = <0 0x100>;
			...
		};

		...
	};

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Mirza Krak]

2016-08-24 17:56 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
+
>> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
>> +controllers with a simple-bus node since they are all connected to the same
>> +chip-select (CS4), in this example external address decoding is provided:
>> +
>> +gmi@70090000 {
>> +     compatible = "nvidia,tegra20-gmi";
>> +     reg = <0x70009000 0x1000>;
>> +     #address-cells = <1>;
>> +     #size-cells = <1>;
>> +     clocks = <&tegra_car TEGRA20_CLK_NOR>;
>> +     clock-names = "gmi";
>> +     resets = <&tegra_car 42>;
>> +     reset-names = "gmi";
>> +     ranges = <4 0x48000000 0x7ffffff>;
>> +
>> +     status = "disabled";
>> +
>> +     bus@4 {
>> +             compatible = "simple-bus";
>> +             reg = <4>;
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             ranges = <0 4 0x40100>;
>
> Does this work? I tried to add an example like this and I got ...
>
> Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
> length (4 bytes) (#address-cells == 1, #size-cells == 1)

Shoot, to get rid of the warning it should be

reg = <4 0 >;

But it works either way.

>
> I am wondering if we should just following the arm,pl172 example and
> have ...
>
>         cs4 {
>                 compatible = "simple-bus";
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 ranges;
>
>                 nvidia,snor-cs = <4>;
>                 nvidia,snor-mux-mode;
>                 nvidia,snor-adv-inv;
>
>                 can@0 {
>                         reg = <0 0x100>;
>                         ...
>                 };
>
>                 ...
>         };
>

That means to go back to V1 really (almost :)). Which I do not mind.
Will give it a test run.

But I am a little hesitant if will be any better/cleaner. In your example above:

can@0 {
         reg = <0 0x100>;
         ...
};

Would this really translate correctly? In the pl172 example they have
multiple ranges and address with "flash@0,0" which a range defined in
parent node. "can@0" does not have valid match in parent node in our
example. So I probably need add some more logic for it to properly
translate.

I have an idea which is following:

gmi@70090000 {
         status = "okay";
         #address-cells = <2>;
         #size-cells = <1>;
         ranges = <4 0 0x48000000 0x00040000>;

         cs4 {
                 compatible = "simple-bus";
                 #address-cells = <2>;
                 #size-cells = <1>;
                 ranges;

                 nvidia,snor-cs = <4>;
                 nvidia,snor-mux-mode;
                 nvidia,snor-adv-inv;

                 can@0 {
                         compatible = "nxp,sja1000";
                         reg = <4 0 0x100>;
                         ...
                 };


                 can@40000 {
                         compatible = "nxp,sja1000";
                         reg = <4 0x40000 0x100>;
                         ...
                 };
         };
};

Do not know if above will work at all (not able to test at current
location), anyway I will play around with it some more and get back to
you.

Best Regards
Mirza

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Mirza Krak]

2016-08-24 21:54 GMT+02:00 Mirza Krak <mirza.krak@gmail.com>:
> 2016-08-24 17:56 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
> +
>>> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
>>> +controllers with a simple-bus node since they are all connected to the same
>>> +chip-select (CS4), in this example external address decoding is provided:
>>> +
>>> +gmi@70090000 {
>>> +     compatible = "nvidia,tegra20-gmi";
>>> +     reg = <0x70009000 0x1000>;
>>> +     #address-cells = <1>;
>>> +     #size-cells = <1>;
>>> +     clocks = <&tegra_car TEGRA20_CLK_NOR>;
>>> +     clock-names = "gmi";
>>> +     resets = <&tegra_car 42>;
>>> +     reset-names = "gmi";
>>> +     ranges = <4 0x48000000 0x7ffffff>;
>>> +
>>> +     status = "disabled";
>>> +
>>> +     bus@4 {
>>> +             compatible = "simple-bus";
>>> +             reg = <4>;
>>> +             #address-cells = <1>;
>>> +             #size-cells = <1>;
>>> +             ranges = <0 4 0x40100>;
>>
>> Does this work? I tried to add an example like this and I got ...
>>
>> Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
>> length (4 bytes) (#address-cells == 1, #size-cells == 1)
>
> Shoot, to get rid of the warning it should be
>
> reg = <4 0 >;
>
> But it works either way.
>
>>
>> I am wondering if we should just following the arm,pl172 example and
>> have ...
>>
>>         cs4 {
>>                 compatible = "simple-bus";
>>                 #address-cells = <1>;
>>                 #size-cells = <1>;
>>                 ranges;
>>
>>                 nvidia,snor-cs = <4>;
>>                 nvidia,snor-mux-mode;
>>                 nvidia,snor-adv-inv;
>>
>>                 can@0 {
>>                         reg = <0 0x100>;
>>                         ...
>>                 };
>>
>>                 ...
>>         };
>>
>
> That means to go back to V1 really (almost :)). Which I do not mind.
> Will give it a test run.
>
> But I am a little hesitant if will be any better/cleaner. In your example above:
>
> can@0 {
>          reg = <0 0x100>;
>          ...
> };
>
> Would this really translate correctly? In the pl172 example they have
> multiple ranges and address with "flash@0,0" which a range defined in
> parent node. "can@0" does not have valid match in parent node in our
> example. So I probably need add some more logic for it to properly
> translate.
>
> I have an idea which is following:
>
> gmi@70090000 {
>          status = "okay";
>          #address-cells = <2>;
>          #size-cells = <1>;
>          ranges = <4 0 0x48000000 0x00040000>;
>
>          cs4 {
>                  compatible = "simple-bus";
>                  #address-cells = <2>;
>                  #size-cells = <1>;
>                  ranges;
>
>                  nvidia,snor-cs = <4>;
>                  nvidia,snor-mux-mode;
>                  nvidia,snor-adv-inv;
>
>                  can@0 {
>                          compatible = "nxp,sja1000";
>                          reg = <4 0 0x100>;
>                          ...
>                  };
>
>
>                  can@40000 {
>                          compatible = "nxp,sja1000";
>                          reg = <4 0x40000 0x100>;
>                          ...
>                  };
>          };
> };
>
> Do not know if above will work at all (not able to test at current
> location), anyway I will play around with it some more and get back to
> you.

Gave above a test run and it works like a charm. Are we happy with that?

Best Regards
Mirza

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Jon Hunter]

On 26/08/16 05:53, Mirza Krak wrote:

...

>> I have an idea which is following:
>>
>> gmi@70090000 {
>>          status = "okay";
>>          #address-cells = <2>;
>>          #size-cells = <1>;
>>          ranges = <4 0 0x48000000 0x00040000>;
>>
>>          cs4 {
>>                  compatible = "simple-bus";
>>                  #address-cells = <2>;
>>                  #size-cells = <1>;
>>                  ranges;
>>
>>                  nvidia,snor-cs = <4>;
>>                  nvidia,snor-mux-mode;
>>                  nvidia,snor-adv-inv;
>>
>>                  can@0 {
>>                          compatible = "nxp,sja1000";
>>                          reg = <4 0 0x100>;
>>                          ...
>>                  };
>>
>>
>>                  can@40000 {
>>                          compatible = "nxp,sja1000";
>>                          reg = <4 0x40000 0x100>;
>>                          ...
>>                  };
>>          };
>> };
>>
>> Do not know if above will work at all (not able to test at current
>> location), anyway I will play around with it some more and get back to
>> you.
> 
> Gave above a test run and it works like a charm. Are we happy with that?

Does it not work with #address-cells = <1>? Seems odd to have the cs in
the reg for the device.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 6/6] bus: Add support for Tegra Generic Memory Interface

[Jon Hunter]

On 24/08/16 14:37, Mirza Krak wrote:
> From: Mirza Krak <mirza.krak@gmail.com>
> 
> The Generic Memory Interface bus can be used to connect high-speed
> devices such as NOR flash, FPGAs, DSPs...
> 
> Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
> ---
> Changes in v2:
> - Fixed some checkpatch errors
> - Re-ordered probe to get rid of local variables
> - Moved of_platform_default_populate call to the end of probe
> - Use the timing and configuration properties from the child device
> - Added warning if more then 1 child device exist
> 
> 
>  drivers/bus/Kconfig     |   8 ++
>  drivers/bus/Makefile    |   1 +
>  drivers/bus/tegra-gmi.c | 231 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 240 insertions(+)
>  create mode 100644 drivers/bus/tegra-gmi.c
> 
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 4ed7d26..2e75a7f 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -141,6 +141,14 @@ config TEGRA_ACONNECT
>  	  Driver for the Tegra ACONNECT bus which is used to interface with
>  	  the devices inside the Audio Processing Engine (APE) for Tegra210.
> 
> +config TEGRA_GMI
> +	tristate "Tegra Generic Memory Interface bus driver"
> +	depends on ARCH_TEGRA
> +	help
> +	  Driver for the Tegra Generic Memory Interface bus which can be used
> +	  to attach devices such as NOR, UART, FPGA and more.
> +
> +
>  config UNIPHIER_SYSTEM_BUS
>  	tristate "UniPhier System Bus driver"
>  	depends on ARCH_UNIPHIER && OF
> diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
> index ac84cc4..34e2bab 100644
> --- a/drivers/bus/Makefile
> +++ b/drivers/bus/Makefile
> @@ -18,5 +18,6 @@ obj-$(CONFIG_OMAP_OCP2SCP)	+= omap-ocp2scp.o
>  obj-$(CONFIG_SUNXI_RSB)		+= sunxi-rsb.o
>  obj-$(CONFIG_SIMPLE_PM_BUS)	+= simple-pm-bus.o
>  obj-$(CONFIG_TEGRA_ACONNECT)	+= tegra-aconnect.o
> +obj-$(CONFIG_TEGRA_GMI)		+= tegra-gmi.o
>  obj-$(CONFIG_UNIPHIER_SYSTEM_BUS)	+= uniphier-system-bus.o
>  obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
> diff --git a/drivers/bus/tegra-gmi.c b/drivers/bus/tegra-gmi.c
> new file mode 100644
> index 0000000..b068ef9
> --- /dev/null
> +++ b/drivers/bus/tegra-gmi.c
> @@ -0,0 +1,231 @@
> +/*
> + * Driver for NVIDIA Generic Memory Interface
> + *
> + * Copyright (C) 2016 Host Mobility AB. All rights reserved.
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/reset.h>
> +
> +#define TEGRA_GMI_CONFIG		0x00
> +#define TEGRA_GMI_CONFIG_GO		BIT(31)
> +#define TEGRA_GMI_BUS_WIDTH_32BIT	BIT(30)
> +#define TEGRA_GMI_MUX_MODE		BIT(28)
> +#define TEGRA_GMI_RDY_BEFORE_DATA	BIT(24)
> +#define TEGRA_GMI_RDY_ACTIVE_HIGH	BIT(23)
> +#define TEGRA_GMI_ADV_ACTIVE_HIGH	BIT(22)
> +#define TEGRA_GMI_OE_ACTIVE_HIGH	BIT(21)
> +#define TEGRA_GMI_CS_ACTIVE_HIGH	BIT(20)
> +#define TEGRA_GMI_CS_SELECT(x)		((x & 0x7) << 4)
> +
> +#define TEGRA_GMI_TIMING0		0x10
> +#define TEGRA_GMI_MUXED_WIDTH(x)	((x & 0xf) << 12)
> +#define TEGRA_GMI_HOLD_WIDTH(x)		((x & 0xf) << 8)
> +#define TEGRA_GMI_ADV_WIDTH(x)		((x & 0xf) << 4)
> +#define TEGRA_GMI_CE_WIDTH(x)		(x & 0xf)
> +
> +#define TEGRA_GMI_TIMING1		0x14
> +#define TEGRA_GMI_WE_WIDTH(x)		((x & 0xff) << 16)
> +#define TEGRA_GMI_OE_WIDTH(x)		((x & 0xff) << 8)
> +#define TEGRA_GMI_WAIT_WIDTH(x)		(x & 0xff)
> +
> +struct tegra_gmi_priv {
> +	void __iomem *base;
> +	struct reset_control *rst;
> +	struct clk *clk;
> +
> +	u32 snor_config;
> +	u32 snor_timing0;
> +	u32 snor_timing1;
> +};
> +
> +static void tegra_gmi_init(struct device *dev, struct tegra_gmi_priv *priv)
> +{
> +	writel(priv->snor_timing0, priv->base + TEGRA_GMI_TIMING0);
> +	writel(priv->snor_timing1, priv->base + TEGRA_GMI_TIMING1);
> +
> +	priv->snor_config |= TEGRA_GMI_CONFIG_GO;
> +	writel(priv->snor_config, priv->base + TEGRA_GMI_CONFIG);
> +}
> +
> +static void tegra_gmi_parse_dt(struct device *dev, struct tegra_gmi_priv *priv)
> +{
> +	struct device_node *child = of_get_next_available_child(dev->of_node,
> +		NULL);
> +	u32 property;
> +
> +	if (!child) {
> +		dev_warn(dev, "no child nodes found\n");
> +		return;
> +	}
> +
> +	/*
> +	 * We currently only support one child device due to lack of
> +	 * chip-select address decoding. Which means that we only have one
> +	 * chip-select line from the GMI controller.
> +	 */
> +	if (of_get_child_count(dev->of_node) > 1)
> +		dev_warn(dev, "only one child device is supported.");
> +
> +	if (of_property_read_bool(child, "nvidia,snor-data-width-32bit"))
> +		priv->snor_config |= TEGRA_GMI_BUS_WIDTH_32BIT;
> +
> +	if (of_property_read_bool(child, "nvidia,snor-mux-mode"))
> +		priv->snor_config |= TEGRA_GMI_MUX_MODE;
> +
> +	if (of_property_read_bool(child, "nvidia,snor-rdy-active-before-data"))
> +		priv->snor_config |= TEGRA_GMI_RDY_BEFORE_DATA;
> +
> +	if (of_property_read_bool(child, "nvidia,snor-rdy-inv"))
> +		priv->snor_config |= TEGRA_GMI_RDY_ACTIVE_HIGH;
> +
> +	if (of_property_read_bool(child, "nvidia,snor-adv-inv"))
> +		priv->snor_config |= TEGRA_GMI_ADV_ACTIVE_HIGH;
> +
> +	if (of_property_read_bool(child, "nvidia,snor-oe-inv"))
> +		priv->snor_config |= TEGRA_GMI_OE_ACTIVE_HIGH;
> +
> +	if (of_property_read_bool(child, "nvidia,snor-cs-inv"))
> +		priv->snor_config |= TEGRA_GMI_CS_ACTIVE_HIGH;
> +
> +	/* Use the reg property to determine which CS is to be used. */
> +	if (!of_property_read_u32(child, "reg", &property))
> +		priv->snor_config |= TEGRA_GMI_CS_SELECT(property);
> +
> +	/* The default values that are provided below are reset values */
> +	if (!of_property_read_u32(child, "nvidia,snor-muxed-width", &property))
> +		priv->snor_timing0 |= TEGRA_GMI_MUXED_WIDTH(property);
> +	else
> +		priv->snor_timing0 |= TEGRA_GMI_MUXED_WIDTH(1);
> +
> +	if (!of_property_read_u32(child, "nvidia,snor-hold-width", &property))
> +		priv->snor_timing0 |= TEGRA_GMI_HOLD_WIDTH(property);
> +	else
> +		priv->snor_timing0 |= TEGRA_GMI_HOLD_WIDTH(1);
> +
> +	if (!of_property_read_u32(child, "nvidia,snor-adv-width", &property))
> +		priv->snor_timing0 |= TEGRA_GMI_ADV_WIDTH(property);
> +	else
> +		priv->snor_timing0 |= TEGRA_GMI_ADV_WIDTH(1);
> +
> +	if (!of_property_read_u32(child, "nvidia,snor-ce-width", &property))
> +		priv->snor_timing0 |= TEGRA_GMI_CE_WIDTH(property);
> +	else
> +		priv->snor_timing0 |= TEGRA_GMI_CE_WIDTH(4);
> +
> +	if (!of_property_read_u32(child, "nvidia,snor-we-width", &property))
> +		priv->snor_timing1 |= TEGRA_GMI_WE_WIDTH(property);
> +	else
> +		priv->snor_timing1 |= TEGRA_GMI_WE_WIDTH(1);
> +
> +	if (!of_property_read_u32(child, "nvidia,snor-oe-width", &property))
> +		priv->snor_timing1 |= TEGRA_GMI_OE_WIDTH(property);
> +	else
> +		priv->snor_timing1 |= TEGRA_GMI_OE_WIDTH(1);
> +
> +	if (!of_property_read_u32(child, "nvidia,snor-wait-width", &property))
> +		priv->snor_timing1 |= TEGRA_GMI_WAIT_WIDTH(property);
> +	else
> +		priv->snor_timing1 |= TEGRA_GMI_WAIT_WIDTH(3);
> +
> +	of_node_put(child);
> +}
> +
> +static int tegra_gmi_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct device *dev = &pdev->dev;
> +	struct tegra_gmi_priv *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	priv->clk = devm_clk_get(dev, "gmi");
> +	if (IS_ERR(priv->clk)) {
> +		dev_err(dev, "can not get clock\n");
> +		return PTR_ERR(priv->clk);
> +	}
> +
> +	priv->rst = devm_reset_control_get(dev, "gmi");
> +	if (IS_ERR(priv->rst)) {
> +		dev_err(dev, "can not get reset\n");
> +		return PTR_ERR(priv->rst);
> +	}
> +
> +	ret = clk_prepare_enable(priv->clk);
> +	if (ret) {
> +		dev_err(dev, "fail to enable clock.\n");
> +		return ret;
> +	}
> +
> +	reset_control_assert(priv->rst);
> +	udelay(2);
> +	reset_control_deassert(priv->rst);
> +
> +	tegra_gmi_parse_dt(dev, priv);
> +	tegra_gmi_init(dev, priv);
> +
> +	ret = of_platform_default_populate(dev->of_node, NULL, dev);
> +	if (ret < 0) {
> +		dev_err(dev, "fail to create devices.\n");
> +		clk_disable_unprepare(priv->clk);
> +		reset_control_assert(priv->rst);

nit ... I would assert the reset first and then disable the clock. This
allows the reset a few clocks to reset the logic.

Do you also need to clear the GO bit like in the remove here for
consistency? Could be worth adding a helper function to do this.

> +		return ret;
> +	}
> +
> +	dev_set_drvdata(dev, priv);
> +
> +	return 0;
> +}
> +
> +static int tegra_gmi_remove(struct platform_device *pdev)
> +{
> +	struct tegra_gmi_priv *priv = dev_get_drvdata(&pdev->dev);
> +	u32 config;
> +
> +	of_platform_depopulate(&pdev->dev);
> +
> +	config = readl(priv->base + TEGRA_GMI_CONFIG);
> +	config &= ~TEGRA_GMI_CONFIG_GO;
> +	writel(config, priv->base + TEGRA_GMI_CONFIG);
> +
> +	clk_disable_unprepare(priv->clk);
> +	reset_control_assert(priv->rst);

I would re-order the reset and clock here as well.

Otherwise ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Mirza Krak]

2016-08-26 9:25 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>
> On 26/08/16 05:53, Mirza Krak wrote:
>
> ...
>
>>> I have an idea which is following:
>>>
>>> gmi@70090000 {
>>>          status = "okay";
>>>          #address-cells = <2>;
>>>          #size-cells = <1>;
>>>          ranges = <4 0 0x48000000 0x00040000>;
>>>
>>>          cs4 {
>>>                  compatible = "simple-bus";
>>>                  #address-cells = <2>;
>>>                  #size-cells = <1>;
>>>                  ranges;
>>>
>>>                  nvidia,snor-cs = <4>;
>>>                  nvidia,snor-mux-mode;
>>>                  nvidia,snor-adv-inv;
>>>
>>>                  can@0 {
>>>                          compatible = "nxp,sja1000";
>>>                          reg = <4 0 0x100>;
>>>                          ...
>>>                  };
>>>
>>>
>>>                  can@40000 {
>>>                          compatible = "nxp,sja1000";
>>>                          reg = <4 0x40000 0x100>;
>>>                          ...
>>>                  };
>>>          };
>>> };
>>>
>>> Do not know if above will work at all (not able to test at current
>>> location), anyway I will play around with it some more and get back to
>>> you.
>>
>> Gave above a test run and it works like a charm. Are we happy with that?
>
> Does it not work with #address-cells = <1>? Seems odd to have the cs in
> the reg for the device.
>

No it does not work with #address-cells = <1> with the current
structure that we have.

With #address-cells = <2>, we can state that this device is on
chip-select 4 and on this specific offset of chip-select 4 (that is
the second address cell). I do not see how we can specify this with
only one address cell.

And is it really that odd? Looking at other drivers they all use the
same metod, even the arm,pl172 that you where referring to as a base
for our implementation.

Best Regards
Mirza

Re: [PATCH v2 0/6] Add support for Tegra GMI bus controller

[Marcel Ziswiler]

Hi Mirza

Sorry, I long since wanted to give you some feedback on this as well.

BTW: Thank you very much for taking this on!

On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote:
> From: Mirza Krak <mirza.krak@gmail.com>
> 
> Hi.
> 
> This is a follow up to my previous RFC to add support for Tegra GMI
> bus
> controller.
> 
> I have tested this series on a Tegra30 using a Colibri T30 SOM on a
> custom
> carrier board which has multiple CAN controllers (SJA1000) connected
> to the
> GMI bus.

We once did a nice GMI-Memory Board which mates with the extension
connector X3 of our V3.x Colibri Evaluation boards and allows testing
SRAM access not only in muxed but also in non-muxed mode albeit 16-bit
only. I took your driver for a spin both on Colibri T20 as well as
Colibri T30 both in muxed as well as non-muxed mode and it passed all
tests being both manual devmem2 type reads/writes as well as memtester
runs on the full 128K SRAM giving it the physical address using the -p
argument.

So you may add the following to the whole series:

Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board

I will leave further comments on the individual patches.

BTW: Of course for non-muxed mode I also had to adjust the pin muxing
as they default to muxed.

> I have rebased on top of latest tegra/for-next in V2. Also see
> individual
> patches for changes in V2.
> 
> See below links for previous discussions.
> 
> Comments on RFC:
> https://marc.info/?l=linux-clk&m=146893557629903&w=2
> https://marc.info/?l=linux-tegra&m=146893541829801&w=2
> https://marc.info/?l=linux-tegra&m=146893542429814&w=2
> 
> Comments on V1:
> https://marc.info/?l=linux-arm-kernel&m=147051551821122&w=2
> https://marc.info/?l=linux-arm-kernel&m=147051553121150&w=2
> https://marc.info/?l=linux-arm-kernel&m=147194856600627&w=2
> https://marc.info/?l=linux-arm-kernel&m=147072742432211&w=2
> 
> 
> Mirza Krak (6):
>   clk: tegra: add TEGRA20_CLK_NOR to init table
>   clk: tegra: add TEGRA30_CLK_NOR to init table
>   dt/bindings: Add bindings for Tegra GMI controller
>   ARM: tegra: Add Tegra30 GMI support
>   ARM: tegra: Add Tegra20 GMI support
>   bus: Add support for Tegra Generic Memory Interface
> 
>  .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132
> ++++++++++++
>  arch/arm/boot/dts/tegra20.dtsi                     |  13 ++
>  arch/arm/boot/dts/tegra30.dtsi                     |  12 ++
>  drivers/bus/Kconfig                                |   8 +
>  drivers/bus/Makefile                               |   1 +
>  drivers/bus/tegra-gmi.c                            | 231
> +++++++++++++++++++++
>  drivers/clk/tegra/clk-tegra20.c                    |   1 +
>  drivers/clk/tegra/clk-tegra30.c                    |   1 +
>  8 files changed, 399 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
>  create mode 100644 drivers/bus/tegra-gmi.c
> 
> --
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Cheers

Marcel

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Marcel Ziswiler]

On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote:
> From: Mirza Krak <mirza.krak@gmail.com>
> 
> Document the devicetree bindings for the Generic Memory Interface
> (GMI)
> bus driver found on Tegra SOCs.
> 
> Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
> ---
> Changes in v2:
> - Updated examples and some information based on comments from Jon
> Hunter.
> 
>  .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132
> +++++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
> 
> diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-
> gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-
> gmi.txt
> new file mode 100644
> index 0000000..8c1e15f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
> @@ -0,0 +1,132 @@
> +Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
> +
> +The Generic Memory Interface bus enables memory transfers between
> internal and
> +external memory. Can be used to attach various high speed devices
> such as
> +synchronous/asynchronous NOR, FPGA, UARTS and more.
> +
> +The actual devices are instantiated from the child nodes of a GMI
> node.
> +
> +Required properties:
> + - compatible : Should contain one of the following:
> +        For Tegra20 must contain "nvidia,tegra20-gmi".
> +        For Tegra30 must contain "nvidia,tegra30-gmi".
> + - reg: Should contain GMI controller registers location and length.
> + - clocks: Must contain an entry for each entry in clock-names.
> + - clock-names: Must include the following entries: "gmi"
> + - resets : Must contain an entry for each entry in reset-names.
> + - reset-names : Must include the following entries: "gmi"
> + - #address-cells: The number of cells used to represent physical
> base
> +   addresses in the GMI address space. Should be 1.
> + - #size-cells: The number of cells used to represent the size of an
> address
> +   range in the GMI address space. Should be 1.
> + - ranges: Must be set up to reflect the memory layout with three
> integer values
> +   for each chip-select line in use (only one entry is supported,
> see below
> +   comments):
> +   <cs-number> <physical address of mapping> <size>
> +
> +Note that the GMI controller does not have any internal chip-select
> address
> +decoding, because of that chip-selects either need to be managed via
> software
> +or by employing external chip-select decoding logic.
> +
> +If external chip-select logic is used to support multiple devices it
> is assumed
> +that the devices use the same timing and so are probably the same
> type. It also
> +assumes that they can fit in the 256MB address range. In this case
> only one
> +child device is supported which represents the active chip-select
> line, see
> +examples for more insight.
> +
> +Required child cs node properties:
> + - reg: First entry should contain the active chip-select number
> +
> +Optional child cs node properties:
> +
> + - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is
> 16bit.
> + - nvidia,snor-mux-mode: Enable address/data MUX mode.
> + - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle
> before data.
> +   If omitted it will be asserted with data.
> + - nvidia,snor-rdy-inv: RDY signal is active high
> + - nvidia,snor-adv-inv: ADV signal is active high
> + - nvidia,snor-oe-inv: WE/OE signal is active high
> + - nvidia,snor-cs-inv: CS signal is active high
> +
> +  Note that there is some special handling for the timing values.
> +  From Tegra TRM:
> +  Programming 0 means 1 clock cycle: actual cycle = programmed cycle
> + 1
> +
> + - nvidia,snor-muxed-width: Number of cycles MUX address/data
> asserted on the
> +   bus. Valid values are 0-15, default is 1
> + - nvidia,snor-hold-width: Number of cycles CE stays asserted after
> the
> +   de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
> +   (in case of MASTER Request). Valid values are 0-15, default is 1
> + - nvidia,snor-adv-width: Number of cycles during which ADV stays
> asserted.
> +   Valid values are 0-15, default is 1.
> + - nvidia,snor-ce-width: Number of cycles before CE is asserted.
> +   Valid values are 0-15, default is 4
> + - nvidia,snor-we-width: Number of cycles during which WE stays
> asserted.
> +   Valid values are 0-15, default is 1
> + - nvidia,snor-oe-width: Number of cycles during which OE stays
> asserted.
> +   Valid values are 0-255, default is 1
> + - nvidia,snor-wait-width: Number of cycles before READY is
> asserted.
> +   Valid values are 0-255, default is 3
> +
> +Example with two SJA1000 CAN controllers connected to the GMI bus.
> We wrap the
> +controllers with a simple-bus node since they are all connected to
> the same
> +chip-select (CS4), in this example external address decoding is
> provided:
> +
> +gmi@70090000 {

It's actually 70009000.

> +	compatible = "nvidia,tegra20-gmi";
> +	reg = <0x70009000 0x1000>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	clocks = <&tegra_car TEGRA20_CLK_NOR>;
> +	clock-names = "gmi";
> +	resets = <&tegra_car 42>;
> +	reset-names = "gmi";
> +	ranges = <4 0x48000000 0x7ffffff>;
> +
> +	status = "disabled";

I guess in an example one could even set this to okay.

> +
> +	bus@4 {
> +		compatible = "simple-bus";
> +		reg = <4>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 4 0x40100>;
> +
> +		nvidia,snor-mux-mode;
> +		nvidia,snor-adv-inv;
> +
> +		can@0 {
> +			reg = <0 0x100>;
> +			...
> +		};
> +
> +		can@40000 {
> +			reg = <0x40000 0x100>;
> +			...
> +		};
> +	};
> +};
> +
> +Example with one SJA1000 CAN controller connected to the GMI bus
> +on CS4:
> +
> +gmi@70090000 {

Same here.

> +	compatible = "nvidia,tegra20-gmi";
> +	reg = <0x70009000 0x1000>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	clocks = <&tegra_car TEGRA20_CLK_NOR>;
> +	clock-names = "gmi";
> +	resets = <&tegra_car 42>;
> +	reset-names = "gmi";
> +	ranges = <4 0x48000000 0x7ffffff>;
> +
> +	status = "disabled";

Same here.

> +
> +	can@4 {
> +		reg = <4 0x100>;
> +		...
> +		nvidia,snor-mux-mode;
> +		nvidia,snor-adv-inv;
> +	};
> +};
> --
> 2.1.4

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Rob Herring]

On Wed, Aug 24, 2016 at 09:54:47PM +0200, Mirza Krak wrote:
> 2016-08-24 17:56 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
> +
> >> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
> >> +controllers with a simple-bus node since they are all connected to the same
> >> +chip-select (CS4), in this example external address decoding is provided:
> >> +
> >> +gmi@70090000 {
> >> +     compatible = "nvidia,tegra20-gmi";
> >> +     reg = <0x70009000 0x1000>;
> >> +     #address-cells = <1>;
> >> +     #size-cells = <1>;
> >> +     clocks = <&tegra_car TEGRA20_CLK_NOR>;
> >> +     clock-names = "gmi";
> >> +     resets = <&tegra_car 42>;
> >> +     reset-names = "gmi";
> >> +     ranges = <4 0x48000000 0x7ffffff>;
> >> +
> >> +     status = "disabled";
> >> +
> >> +     bus@4 {
> >> +             compatible = "simple-bus";
> >> +             reg = <4>;
> >> +             #address-cells = <1>;
> >> +             #size-cells = <1>;
> >> +             ranges = <0 4 0x40100>;
> >
> > Does this work? I tried to add an example like this and I got ...
> >
> > Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
> > length (4 bytes) (#address-cells == 1, #size-cells == 1)
> 
> Shoot, to get rid of the warning it should be
> 
> reg = <4 0 >;
> 
> But it works either way.

The CS node should have #address-cells=2 with the first being CS# and 
the second being the offset (often 0).

> 
> >
> > I am wondering if we should just following the arm,pl172 example and
> > have ...
> >
> >         cs4 {
> >                 compatible = "simple-bus";
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> >                 ranges;

Empty ranges is typically wrong and due to laziness...

This should have the CS# in it.

> >
> >                 nvidia,snor-cs = <4>;
> >                 nvidia,snor-mux-mode;
> >                 nvidia,snor-adv-inv;
> >
> >                 can@0 {
> >                         reg = <0 0x100>;

This can be 1 cell with just the offset.

> >                         ...
> >                 };
> >
> >                 ...
> >         };
> >
> 
> That means to go back to V1 really (almost :)). Which I do not mind.
> Will give it a test run.
> 
> But I am a little hesitant if will be any better/cleaner. In your example above:
> 
> can@0 {
>          reg = <0 0x100>;
>          ...
> };
> 
> Would this really translate correctly? In the pl172 example they have
> multiple ranges and address with "flash@0,0" which a range defined in
> parent node. "can@0" does not have valid match in parent node in our
> example. So I probably need add some more logic for it to properly
> translate.

pl172 has several things I don't like, so don't follow it. Mainly those 
are custom CS property and 3 levels of nodes. I'm fine with 3 levels if 
there is more than one device, but otherwise 2 levels with timing 
properties in the child device node.


> 
> I have an idea which is following:
> 
> gmi@70090000 {
>          status = "okay";
>          #address-cells = <2>;
>          #size-cells = <1>;
>          ranges = <4 0 0x48000000 0x00040000>;
> 
>          cs4 {

cs@4,0

>                  compatible = "simple-bus";
>                  #address-cells = <2>;

1 cell here.

>                  #size-cells = <1>;
>                  ranges;

Fill this in to drop the 2nd cell on child addresses and just have the 
offset.

> 
>                  nvidia,snor-cs = <4>;

NAK, no custom CS properties.

>                  nvidia,snor-mux-mode;
>                  nvidia,snor-adv-inv;
> 
>                  can@0 {
>                          compatible = "nxp,sja1000";
>                          reg = <4 0 0x100>;
>                          ...
>                  };
> 
> 
>                  can@40000 {
>                          compatible = "nxp,sja1000";
>                          reg = <4 0x40000 0x100>;
>                          ...
>                  };
>          };
> };
> 
> Do not know if above will work at all (not able to test at current
> location), anyway I will play around with it some more and get back to
> you.
> 
> Best Regards
> Mirza

Re: [PATCH v2 0/6] Add support for Tegra GMI bus controller

[Mirza Krak]

Hi Marcel.

2016-08-30 17:01 GMT+02:00 Marcel Ziswiler <marcel.ziswiler@toradex.com>:
> Hi Mirza
>
> Sorry, I long since wanted to give you some feedback on this as well.
>
> BTW: Thank you very much for taking this on!

It has been and still is a fun project. So gladly doing it.
>
> On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote:
>> From: Mirza Krak <mirza.krak@gmail.com>
>>
>> Hi.
>>
>> This is a follow up to my previous RFC to add support for Tegra GMI
>> bus
>> controller.
>>
>> I have tested this series on a Tegra30 using a Colibri T30 SOM on a
>> custom
>> carrier board which has multiple CAN controllers (SJA1000) connected
>> to the
>> GMI bus.
>
> We once did a nice GMI-Memory Board which mates with the extension
> connector X3 of our V3.x Colibri Evaluation boards and allows testing
> SRAM access not only in muxed but also in non-muxed mode albeit 16-bit
> only. I took your driver for a spin both on Colibri T20 as well as
> Colibri T30 both in muxed as well as non-muxed mode and it passed all
> tests being both manual devmem2 type reads/writes as well as memtester
> runs on the full 128K SRAM giving it the physical address using the -p
> argument.
>
> So you may add the following to the whole series:
>
> Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board

Thank you very much for testing. Will add your tags in the upcoming V3.

Best Regards
Mirza

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Mirza Krak]

2016-08-30 17:02 GMT+02:00 Marcel Ziswiler <marcel.ziswiler@toradex.com>:
> On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote:
>> From: Mirza Krak <mirza.krak@gmail.com>
>>
>> Document the devicetree bindings for the Generic Memory Interface
>> (GMI)
>> bus driver found on Tegra SOCs.
>>
>> Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
>> ---
>> Changes in v2:
>> - Updated examples and some information based on comments from Jon
>> Hunter.
>>
>>  .../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132
>> +++++++++++++++++++++
>>  1 file changed, 132 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-
>> gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-
>> gmi.txt
>> new file mode 100644
>> index 0000000..8c1e15f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
>> @@ -0,0 +1,132 @@
>> +Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
>> +
>> +The Generic Memory Interface bus enables memory transfers between
>> internal and
>> +external memory. Can be used to attach various high speed devices
>> such as
>> +synchronous/asynchronous NOR, FPGA, UARTS and more.
>> +
>> +The actual devices are instantiated from the child nodes of a GMI
>> node.
>> +
>> +Required properties:
>> + - compatible : Should contain one of the following:
>> +        For Tegra20 must contain "nvidia,tegra20-gmi".
>> +        For Tegra30 must contain "nvidia,tegra30-gmi".
>> + - reg: Should contain GMI controller registers location and length.
>> + - clocks: Must contain an entry for each entry in clock-names.
>> + - clock-names: Must include the following entries: "gmi"
>> + - resets : Must contain an entry for each entry in reset-names.
>> + - reset-names : Must include the following entries: "gmi"
>> + - #address-cells: The number of cells used to represent physical
>> base
>> +   addresses in the GMI address space. Should be 1.
>> + - #size-cells: The number of cells used to represent the size of an
>> address
>> +   range in the GMI address space. Should be 1.
>> + - ranges: Must be set up to reflect the memory layout with three
>> integer values
>> +   for each chip-select line in use (only one entry is supported,
>> see below
>> +   comments):
>> +   <cs-number> <physical address of mapping> <size>
>> +
>> +Note that the GMI controller does not have any internal chip-select
>> address
>> +decoding, because of that chip-selects either need to be managed via
>> software
>> +or by employing external chip-select decoding logic.
>> +
>> +If external chip-select logic is used to support multiple devices it
>> is assumed
>> +that the devices use the same timing and so are probably the same
>> type. It also
>> +assumes that they can fit in the 256MB address range. In this case
>> only one
>> +child device is supported which represents the active chip-select
>> line, see
>> +examples for more insight.
>> +
>> +Required child cs node properties:
>> + - reg: First entry should contain the active chip-select number
>> +
>> +Optional child cs node properties:
>> +
>> + - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is
>> 16bit.
>> + - nvidia,snor-mux-mode: Enable address/data MUX mode.
>> + - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle
>> before data.
>> +   If omitted it will be asserted with data.
>> + - nvidia,snor-rdy-inv: RDY signal is active high
>> + - nvidia,snor-adv-inv: ADV signal is active high
>> + - nvidia,snor-oe-inv: WE/OE signal is active high
>> + - nvidia,snor-cs-inv: CS signal is active high
>> +
>> +  Note that there is some special handling for the timing values.
>> +  From Tegra TRM:
>> +  Programming 0 means 1 clock cycle: actual cycle = programmed cycle
>> + 1
>> +
>> + - nvidia,snor-muxed-width: Number of cycles MUX address/data
>> asserted on the
>> +   bus. Valid values are 0-15, default is 1
>> + - nvidia,snor-hold-width: Number of cycles CE stays asserted after
>> the
>> +   de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
>> +   (in case of MASTER Request). Valid values are 0-15, default is 1
>> + - nvidia,snor-adv-width: Number of cycles during which ADV stays
>> asserted.
>> +   Valid values are 0-15, default is 1.
>> + - nvidia,snor-ce-width: Number of cycles before CE is asserted.
>> +   Valid values are 0-15, default is 4
>> + - nvidia,snor-we-width: Number of cycles during which WE stays
>> asserted.
>> +   Valid values are 0-15, default is 1
>> + - nvidia,snor-oe-width: Number of cycles during which OE stays
>> asserted.
>> +   Valid values are 0-255, default is 1
>> + - nvidia,snor-wait-width: Number of cycles before READY is
>> asserted.
>> +   Valid values are 0-255, default is 3
>> +
>> +Example with two SJA1000 CAN controllers connected to the GMI bus.
>> We wrap the
>> +controllers with a simple-bus node since they are all connected to
>> the same
>> +chip-select (CS4), in this example external address decoding is
>> provided:
>> +
>> +gmi@70090000 {
>
> It's actually 70009000.

I actually noticed this all ready, so should be fixed in the upcoming V3.

Thank you for your review.

Best Regards
Mirza

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Mirza Krak]

2016-08-30 19:06 GMT+02:00 Rob Herring <robh@kernel.org>:
> On Wed, Aug 24, 2016 at 09:54:47PM +0200, Mirza Krak wrote:
>> 2016-08-24 17:56 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>> +
>> >> +Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
>> >> +controllers with a simple-bus node since they are all connected to the same
>> >> +chip-select (CS4), in this example external address decoding is provided:
>> >> +
>> >> +gmi@70090000 {
>> >> +     compatible = "nvidia,tegra20-gmi";
>> >> +     reg = <0x70009000 0x1000>;
>> >> +     #address-cells = <1>;
>> >> +     #size-cells = <1>;
>> >> +     clocks = <&tegra_car TEGRA20_CLK_NOR>;
>> >> +     clock-names = "gmi";
>> >> +     resets = <&tegra_car 42>;
>> >> +     reset-names = "gmi";
>> >> +     ranges = <4 0x48000000 0x7ffffff>;
>> >> +
>> >> +     status = "disabled";
>> >> +
>> >> +     bus@4 {
>> >> +             compatible = "simple-bus";
>> >> +             reg = <4>;
>> >> +             #address-cells = <1>;
>> >> +             #size-cells = <1>;
>> >> +             ranges = <0 4 0x40100>;
>> >
>> > Does this work? I tried to add an example like this and I got ...
>> >
>> > Warning (reg_format): "reg" property in /gmi@70009000/bus@4 has invalid
>> > length (4 bytes) (#address-cells == 1, #size-cells == 1)
>>
>> Shoot, to get rid of the warning it should be
>>
>> reg = <4 0 >;
>>
>> But it works either way.
>
> The CS node should have #address-cells=2 with the first being CS# and
> the second being the offset (often 0).
>
>>
>> >
>> > I am wondering if we should just following the arm,pl172 example and
>> > have ...
>> >
>> >         cs4 {
>> >                 compatible = "simple-bus";
>> >                 #address-cells = <1>;
>> >                 #size-cells = <1>;
>> >                 ranges;
>
> Empty ranges is typically wrong and due to laziness...
>
> This should have the CS# in it.
>
>> >
>> >                 nvidia,snor-cs = <4>;
>> >                 nvidia,snor-mux-mode;
>> >                 nvidia,snor-adv-inv;
>> >
>> >                 can@0 {
>> >                         reg = <0 0x100>;
>
> This can be 1 cell with just the offset.
>
>> >                         ...
>> >                 };
>> >
>> >                 ...
>> >         };
>> >
>>
>> That means to go back to V1 really (almost :)). Which I do not mind.
>> Will give it a test run.
>>
>> But I am a little hesitant if will be any better/cleaner. In your example above:
>>
>> can@0 {
>>          reg = <0 0x100>;
>>          ...
>> };
>>
>> Would this really translate correctly? In the pl172 example they have
>> multiple ranges and address with "flash@0,0" which a range defined in
>> parent node. "can@0" does not have valid match in parent node in our
>> example. So I probably need add some more logic for it to properly
>> translate.
>
> pl172 has several things I don't like, so don't follow it. Mainly those
> are custom CS property and 3 levels of nodes. I'm fine with 3 levels if
> there is more than one device, but otherwise 2 levels with timing
> properties in the child device node.
>
>
>>
>> I have an idea which is following:
>>
>> gmi@70090000 {
>>          status = "okay";
>>          #address-cells = <2>;
>>          #size-cells = <1>;
>>          ranges = <4 0 0x48000000 0x00040000>;
>>
>>          cs4 {
>
> cs@4,0
>
>>                  compatible = "simple-bus";
>>                  #address-cells = <2>;
>
> 1 cell here.
>
>>                  #size-cells = <1>;
>>                  ranges;
>
> Fill this in to drop the 2nd cell on child addresses and just have the
> offset.
>
>>
>>                  nvidia,snor-cs = <4>;
>
> NAK, no custom CS properties.
>
>>                  nvidia,snor-mux-mode;
>>                  nvidia,snor-adv-inv;
>>
>>                  can@0 {
>>                          compatible = "nxp,sja1000";
>>                          reg = <4 0 0x100>;
>>                          ...
>>                  };
>>
>>
>>                  can@40000 {
>>                          compatible = "nxp,sja1000";
>>                          reg = <4 0x40000 0x100>;
>>                          ...
>>                  };
>>          };
>> };
>>

Thank you for your review Rob.

Taking your comments in to account I end up with this:

gmi@70090000 {
        compatible = "nvidia,tegra20-gmi";
        reg = <0x70009000 0x1000>;
        #address-cells = <2>;
        #size-cells = <1>;
        clocks = <&tegra_car TEGRA20_CLK_NOR>;
        clock-names = "gmi";
        resets = <&tegra_car 42>;
        reset-names = "gmi";
        ranges = <4 0 0xd0000000 0xfffffff>;

        status = "okay";

        bus@4,0 {
                compatible = "simple-bus";
                #address-cells = <1>;
                #size-cells = <1>;
                ranges = <0 4 0 0x40000>;

                nvidia,snor-mux-mode;
                nvidia,snor-adv-inv;

                can@0 {
                        reg = <0 0x100>;
                        ...
                };

                can@40000 {
                        reg = <0x40000 0x100>;
                        ...
                };
        };
};

Have I understood you correct?

Also wanted to verify the example case where you only have on device
connected to one CS#, from what I see in other implementations it
seems OK to put the CS# in the reg property in that case. Is this
correct?

Example with one SJA1000 CAN controller connected to the GMI bus
on CS4:

gmi@70090000 {
        compatible = "nvidia,tegra20-gmi";
        reg = <0x70009000 0x1000>;
        #address-cells = <2>;
        #size-cells = <1>;
        clocks = <&tegra_car TEGRA20_CLK_NOR>;
        clock-names = "gmi";
        resets = <&tegra_car 42>;
        reset-names = "gmi";
        ranges = <4 0 0xd0000000 0xfffffff>;

        status = "okay";

        can@4,0 {
                reg = <4 0 0x100>;
                nvidia,snor-mux-mode;
                nvidia,snor-adv-inv;
                ...
        };
};

Jon, to be able to handle both cases in the driver we would first
attempt to decode the CS# from the ranges property, and fallback to
reg property if no ranges are defined. Does that sound reasonable?

Best Regards
Mirza

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Jon Hunter]

On 31/08/16 12:22, Mirza Krak wrote:
> 2016-08-30 19:06 GMT+02:00 Rob Herring <robh@kernel.org>:

...

>>>                  nvidia,snor-cs = <4>;
>>
>> NAK, no custom CS properties.

Ok, so ...

> gmi@70090000 {
>         compatible = "nvidia,tegra20-gmi";
>         reg = <0x70009000 0x1000>;
>         #address-cells = <2>;
>         #size-cells = <1>;
>         clocks = <&tegra_car TEGRA20_CLK_NOR>;
>         clock-names = "gmi";
>         resets = <&tegra_car 42>;
>         reset-names = "gmi";
>         ranges = <4 0 0xd0000000 0xfffffff>;
> 
>         status = "okay";
> 
>         bus@4,0 {
>                 compatible = "simple-bus";
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 ranges = <0 4 0 0x40000>;
> 
>                 nvidia,snor-mux-mode;
>                 nvidia,snor-adv-inv;
> 
>                 can@0 {
>                         reg = <0 0x100>;
>                         ...
>                 };
> 
>                 can@40000 {
>                         reg = <0x40000 0x100>;
>                         ...
>                 };
>         };
> };
> 
> Have I understood you correct?
> 
> Also wanted to verify the example case where you only have on device
> connected to one CS#, from what I see in other implementations it
> seems OK to put the CS# in the reg property in that case. Is this
> correct?
> 
> Example with one SJA1000 CAN controller connected to the GMI bus
> on CS4:
> 
> gmi@70090000 {
>         compatible = "nvidia,tegra20-gmi";
>         reg = <0x70009000 0x1000>;
>         #address-cells = <2>;
>         #size-cells = <1>;
>         clocks = <&tegra_car TEGRA20_CLK_NOR>;
>         clock-names = "gmi";
>         resets = <&tegra_car 42>;
>         reset-names = "gmi";
>         ranges = <4 0 0xd0000000 0xfffffff>;
> 
>         status = "okay";
> 
>         can@4,0 {
>                 reg = <4 0 0x100>;
>                 nvidia,snor-mux-mode;
>                 nvidia,snor-adv-inv;
>                 ...
>         };
> };
> 
> Jon, to be able to handle both cases in the driver we would first
> attempt to decode the CS# from the ranges property, and fallback to
> reg property if no ranges are defined. Does that sound reasonable?

Given the above examples that may be supported, is there a
better/simpler way to extract the CS# than what Mirza is proposing? For
example, from the node-name unit-address?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Jon Hunter]

On 31/08/16 12:22, Mirza Krak wrote:

...

> Taking your comments in to account I end up with this:
> 
> gmi@70090000 {
>         compatible = "nvidia,tegra20-gmi";
>         reg = <0x70009000 0x1000>;
>         #address-cells = <2>;

Does this need to be 2? Seems simpler if this is 1.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Mirza Krak]

2016-09-06 12:32 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>
> On 31/08/16 12:22, Mirza Krak wrote:
>> 2016-08-30 19:06 GMT+02:00 Rob Herring <robh@kernel.org>:
>
> ...
>
>>>>                  nvidia,snor-cs = <4>;
>>>
>>> NAK, no custom CS properties.
>
> Ok, so ...
>
>> gmi@70090000 {
>>         compatible = "nvidia,tegra20-gmi";
>>         reg = <0x70009000 0x1000>;
>>         #address-cells = <2>;
>>         #size-cells = <1>;
>>         clocks = <&tegra_car TEGRA20_CLK_NOR>;
>>         clock-names = "gmi";
>>         resets = <&tegra_car 42>;
>>         reset-names = "gmi";
>>         ranges = <4 0 0xd0000000 0xfffffff>;
>>
>>         status = "okay";
>>
>>         bus@4,0 {
>>                 compatible = "simple-bus";
>>                 #address-cells = <1>;
>>                 #size-cells = <1>;
>>                 ranges = <0 4 0 0x40000>;
>>
>>                 nvidia,snor-mux-mode;
>>                 nvidia,snor-adv-inv;
>>
>>                 can@0 {
>>                         reg = <0 0x100>;
>>                         ...
>>                 };
>>
>>                 can@40000 {
>>                         reg = <0x40000 0x100>;
>>                         ...
>>                 };
>>         };
>> };
>>
>> Have I understood you correct?
>>
>> Also wanted to verify the example case where you only have on device
>> connected to one CS#, from what I see in other implementations it
>> seems OK to put the CS# in the reg property in that case. Is this
>> correct?
>>
>> Example with one SJA1000 CAN controller connected to the GMI bus
>> on CS4:
>>
>> gmi@70090000 {
>>         compatible = "nvidia,tegra20-gmi";
>>         reg = <0x70009000 0x1000>;
>>         #address-cells = <2>;
>>         #size-cells = <1>;
>>         clocks = <&tegra_car TEGRA20_CLK_NOR>;
>>         clock-names = "gmi";
>>         resets = <&tegra_car 42>;
>>         reset-names = "gmi";
>>         ranges = <4 0 0xd0000000 0xfffffff>;
>>
>>         status = "okay";
>>
>>         can@4,0 {
>>                 reg = <4 0 0x100>;
>>                 nvidia,snor-mux-mode;
>>                 nvidia,snor-adv-inv;
>>                 ...
>>         };
>> };
>>
>> Jon, to be able to handle both cases in the driver we would first
>> attempt to decode the CS# from the ranges property, and fallback to
>> reg property if no ranges are defined. Does that sound reasonable?
>
> Given the above examples that may be supported, is there a
> better/simpler way to extract the CS# than what Mirza is proposing? For
> example, from the node-name unit-address?
>

Hi.

I have been on vacation and now I am back and wanted to finalize these
patch series.

So pinging this thread to see I we can agree on a solution.

Rob any comments to my proposal and Jon`s comment?

Best Regards
Mirza Krak

Re: [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Jon Hunter]

Rob,

On 19/09/16 08:21, Mirza Krak wrote:
> 2016-09-06 12:32 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>>
>> On 31/08/16 12:22, Mirza Krak wrote:
>>> 2016-08-30 19:06 GMT+02:00 Rob Herring <robh@kernel.org>:
>>
>> ...
>>
>>>>>                  nvidia,snor-cs = <4>;
>>>>
>>>> NAK, no custom CS properties.
>>
>> Ok, so ...
>>
>>> gmi@70090000 {
>>>         compatible = "nvidia,tegra20-gmi";
>>>         reg = <0x70009000 0x1000>;
>>>         #address-cells = <2>;
>>>         #size-cells = <1>;
>>>         clocks = <&tegra_car TEGRA20_CLK_NOR>;
>>>         clock-names = "gmi";
>>>         resets = <&tegra_car 42>;
>>>         reset-names = "gmi";
>>>         ranges = <4 0 0xd0000000 0xfffffff>;
>>>
>>>         status = "okay";
>>>
>>>         bus@4,0 {
>>>                 compatible = "simple-bus";
>>>                 #address-cells = <1>;
>>>                 #size-cells = <1>;
>>>                 ranges = <0 4 0 0x40000>;
>>>
>>>                 nvidia,snor-mux-mode;
>>>                 nvidia,snor-adv-inv;
>>>
>>>                 can@0 {
>>>                         reg = <0 0x100>;
>>>                         ...
>>>                 };
>>>
>>>                 can@40000 {
>>>                         reg = <0x40000 0x100>;
>>>                         ...
>>>                 };
>>>         };
>>> };
>>>
>>> Have I understood you correct?
>>>
>>> Also wanted to verify the example case where you only have on device
>>> connected to one CS#, from what I see in other implementations it
>>> seems OK to put the CS# in the reg property in that case. Is this
>>> correct?
>>>
>>> Example with one SJA1000 CAN controller connected to the GMI bus
>>> on CS4:
>>>
>>> gmi@70090000 {
>>>         compatible = "nvidia,tegra20-gmi";
>>>         reg = <0x70009000 0x1000>;
>>>         #address-cells = <2>;
>>>         #size-cells = <1>;
>>>         clocks = <&tegra_car TEGRA20_CLK_NOR>;
>>>         clock-names = "gmi";
>>>         resets = <&tegra_car 42>;
>>>         reset-names = "gmi";
>>>         ranges = <4 0 0xd0000000 0xfffffff>;
>>>
>>>         status = "okay";
>>>
>>>         can@4,0 {
>>>                 reg = <4 0 0x100>;
>>>                 nvidia,snor-mux-mode;
>>>                 nvidia,snor-adv-inv;
>>>                 ...
>>>         };
>>> };
>>>
>>> Jon, to be able to handle both cases in the driver we would first
>>> attempt to decode the CS# from the ranges property, and fallback to
>>> reg property if no ranges are defined. Does that sound reasonable?
>>
>> Given the above examples that may be supported, is there a
>> better/simpler way to extract the CS# than what Mirza is proposing? For
>> example, from the node-name unit-address?
>>
> 
> Hi.
> 
> I have been on vacation and now I am back and wanted to finalize these
> patch series.
> 
> So pinging this thread to see I we can agree on a solution.
> 
> Rob any comments to my proposal and Jon`s comment?

Can you comment on the above?

Cheers
Jon

-- 
nvpublic