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

From: Mirza Krak <redacted>

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

From: Mirza Krak <redacted>

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

Signed-off-by: Mirza Krak <redacted>
---
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

From: Mirza Krak <redacted>

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

Signed-off-by: Mirza Krak <redacted>
---
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 at 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 at 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 at 0 {
+			reg = <0 0x100>;
+			...
+		};
+
+		can at 40000 {
+			reg = <0x40000 0x100>;
+			...
+		};
+	};
+};
+
+Example with one SJA1000 CAN controller connected to the GMI bus
+on CS4:
+
+gmi at 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 at 4 {
+		reg = <4 0x100>;
+		...
+		nvidia,snor-mux-mode;
+		nvidia,snor-adv-inv;
+	};
+};
--

2.1.4

From: Mirza Krak <redacted>

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

Signed-off-by: Mirza Krak <redacted>
---
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

From: Mirza Krak <redacted>

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

Signed-off-by: Mirza Krak <redacted>
---
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 at 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 at 7000a000 {
 		compatible = "nvidia,tegra20-pwm";
 		reg = <0x7000a000 0x100>;
--

2.1.4

From: Mirza Krak <redacted>

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

Signed-off-by: Mirza Krak <redacted>
---
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 at 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 at 7000a000 {
 		compatible = "nvidia,tegra30-pwm", "nvidia,tegra20-pwm";
 		reg = <0x7000a000 0x100>;
--

2.1.4

From: Mirza Krak <redacted>

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

Signed-off-by: Mirza Krak <redacted>
---
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

On 24/08/16 14:37, Mirza Krak wrote:

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

Warning (reg_format): "reg" property in /gmi at 70009000/bus at 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 at 0 {
			reg = <0 0x100>;
			...
		};

		...
	};

Cheers
Jon

-- 
nvpublic

2016-08-24 17:56 GMT+02:00 Jon Hunter [off-list ref]:
+

quoted

+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 at 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 at 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 at 70009000/bus at 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 at 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 at 0 {
         reg = <0 0x100>;
         ...
};

Would this really translate correctly? In the pl172 example they have
multiple ranges and address with "flash at 0,0" which a range defined in
parent node. "can at 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 at 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 at 0 {
                         compatible = "nxp,sja1000";
                         reg = <4 0 0x100>;
                         ...
                 };


                 can at 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

2016-08-24 21:54 GMT+02:00 Mirza Krak [off-list ref]:

2016-08-24 17:56 GMT+02:00 Jon Hunter [off-list ref]:
+

quoted

quoted

+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 at 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 at 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 at 70009000/bus at 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.

quoted

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 at 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 at 0 {
         reg = <0 0x100>;
         ...
};

Would this really translate correctly? In the pl172 example they have
multiple ranges and address with "flash at 0,0" which a range defined in
parent node. "can at 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 at 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 at 0 {
                         compatible = "nxp,sja1000";
                         reg = <4 0 0x100>;
                         ...
                 };


                 can at 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

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

...

quoted

I have an idea which is following:

gmi at 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 at 0 {
                         compatible = "nxp,sja1000";
                         reg = <4 0 0x100>;
                         ...
                 };


                 can at 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

On 24/08/16 14:37, Mirza Krak wrote:

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

2016-08-26 9:25 GMT+02:00 Jon Hunter [off-list ref]:

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

...

quoted

quoted

I have an idea which is following:

gmi at 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 at 0 {
                         compatible = "nxp,sja1000";
                         reg = <4 0 0x100>;
                         ...
                 };


                 can at 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

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 <redacted>

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 <redacted>
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 at vger.kernel.org
More majordomo info at??http://vger.kernel.org/majordomo-info.html

Cheers

Marcel

On Wed, Aug 24, 2016 at 09:54:47PM +0200, Mirza Krak wrote:

2016-08-24 17:56 GMT+02:00 Jon Hunter [off-list ref]:
+

quoted

quoted

+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 at 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 at 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 at 70009000/bus at 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).

quoted

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.

quoted

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

                can at 0 {
                        reg = <0 0x100>;

This can be 1 cell with just the offset.

quoted

                        ...
                };

                ...
        };

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 at 0 {
         reg = <0 0x100>;
         ...
};

Would this really translate correctly? In the pl172 example they have
multiple ranges and address with "flash at 0,0" which a range defined in
parent node. "can at 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 at 70090000 {
         status = "okay";
         #address-cells = <2>;
         #size-cells = <1>;
         ranges = <4 0 0x48000000 0x00040000>;

         cs4 {

cs at 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 at 0 {
                         compatible = "nxp,sja1000";
                         reg = <4 0 0x100>;
                         ...
                 };


                 can at 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

On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote:

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 at 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 at 0 {
+			reg = <0 0x100>;
+			...
+		};
+
+		can at 40000 {
+			reg = <0x40000 0x100>;
+			...
+		};
+	};
+};
+
+Example with one SJA1000 CAN controller connected to the GMI bus
+on CS4:
+
+gmi at 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 at 4 {
+		reg = <4 0x100>;
+		...
+		nvidia,snor-mux-mode;
+		nvidia,snor-adv-inv;
+	};
+};
--
2.1.4

Hi Marcel.

2016-08-30 17:01 GMT+02:00 Marcel Ziswiler [off-list ref]:

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:

quoted

From: Mirza Krak <redacted>

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 <redacted>
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

2016-08-30 17:02 GMT+02:00 Marcel Ziswiler [off-list ref]:

On Wed, 2016-08-24 at 15:37 +0200, Mirza Krak wrote:

quoted

From: Mirza Krak <redacted>

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

Signed-off-by: Mirza Krak <redacted>
---
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 at 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

2016-08-30 19:06 GMT+02:00 Rob Herring [off-list ref]:

On Wed, Aug 24, 2016 at 09:54:47PM +0200, Mirza Krak wrote:

quoted

2016-08-24 17:56 GMT+02:00 Jon Hunter [off-list ref]:
+

quoted

quoted

+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 at 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 at 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 at 70009000/bus at 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).

quoted

quoted

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.

quoted

quoted

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

                can at 0 {
                        reg = <0 0x100>;

This can be 1 cell with just the offset.

quoted

quoted

                        ...
                };

                ...
        };

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 at 0 {
         reg = <0 0x100>;
         ...
};

Would this really translate correctly? In the pl172 example they have
multiple ranges and address with "flash at 0,0" which a range defined in
parent node. "can at 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.

quoted

I have an idea which is following:

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

         cs4 {

cs at 4,0

quoted

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

1 cell here.

quoted

                 #size-cells = <1>;
                 ranges;

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

quoted

                 nvidia,snor-cs = <4>;

NAK, no custom CS properties.

quoted

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

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


                 can at 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 at 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 at 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 at 0 {
                        reg = <0 0x100>;
                        ...
                };

                can at 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 at 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 at 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

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

2016-08-30 19:06 GMT+02:00 Rob Herring [off-list ref]:

...

quoted

quoted

                 nvidia,snor-cs = <4>;

NAK, no custom CS properties.

Ok, so ...

gmi at 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 at 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 at 0 {
                        reg = <0 0x100>;
                        ...
                };

                can at 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 at 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 at 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

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

...

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

gmi at 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

2016-09-06 12:32 GMT+02:00 Jon Hunter [off-list ref]:

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

quoted

2016-08-30 19:06 GMT+02:00 Rob Herring [off-list ref]:

...

quoted

quoted

quoted

                 nvidia,snor-cs = <4>;

NAK, no custom CS properties.

Ok, so ...

quoted

gmi at 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 at 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 at 0 {
                        reg = <0 0x100>;
                        ...
                };

                can at 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 at 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 at 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

Rob,

On 19/09/16 08:21, Mirza Krak wrote:

2016-09-06 12:32 GMT+02:00 Jon Hunter [off-list ref]:

quoted

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

quoted

2016-08-30 19:06 GMT+02:00 Rob Herring [off-list ref]:

...

quoted

quoted

quoted

                 nvidia,snor-cs = <4>;

NAK, no custom CS properties.

Ok, so ...

quoted

gmi at 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 at 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 at 0 {
                        reg = <0 0x100>;
                        ...
                };

                can at 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 at 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 at 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