[PATCH v2 0/5] Device Bus support for Marvell EBU SoC

[PATCH v2 0/5] Device Bus support for Marvell EBU SoC

[Ezequiel Garcia]

Now that the mbus driver has been added to manage address decoding
windows it's possible to introduce the Device Bus driver.
This driver allows to access several memories and I/O devices
such as NOR, NAND, SRAM and FPGAs.

Currently the SoCs that include this Device Bus controller are:

  * Discovery:    mv78100, mv78200
  * Armada XP:    mv78230, mv78260, mv78460
  * Armada 370:   88f6710. 88f6707, 88f6w11
  * Orion 5x:     88f5182, 88f5181, 88f5281

Please note that this patchset adds proper support for
Armada XP and Armada 370 SoC only.

This applies on top of v3.9-rc5 with mbus patches applied, which can 
be found in Jason Cooper's mvebus/soc branch.
If anyone wants to try this, there's a public branch available at:

  https://github.com/MISL-EBU-System-SW/mainline-public/tree/mvebu-devbus-v2

There has been a 'byzantine' discussion regarding the mbus driver DT binding
and the way to describe address windows, and this discussion is still not
settled.

However, since I'd like to see this in v3.10, and the only thing left
to solve seems to be the device tree binding for the mbus window,
this current patchset attempts a temporary workaround.

The decoding windows are handled by the devbus driver, through the mbus API,
using the information present in the 'ranges' property. For instance:

	device-bus at d0010400 {

		ranges = <0 0 0xf0000000 0x8000000>;
		/* ... */
	}

In the above example, when the device bus driver finds a child
node for chip select '0', he will request the mbus driver to allocate a window
as described by the 'ranges' property, starting at address 0xf0000000.

As this approach has been explicitly rejected by the maintainers,
it will be completely removed, as soon as the mbus bindings are decided
and the address windows are statically describe in the dts files.

Tested on an Armada XP GP board and Plathome Openblocks AX3.

In particular I'd like to know what's the chance of having this included
for v3.10. If the driver looks good and the temporary window handling
is accepted, then I can prepare a v3 rebased on top of Gregory's LPAE work,
or on top of any other required patches.

Changes from v1:

  * Added a remove() function to unregister childs and remove address windows,
    as suggested by Thomas Petazzoni.

  * Remove unneeded 'inline' prefix on functions as suggested by Thomas Petazzoni.

  * Timings parameters are now mandatory and expressed in picoseconds,
    as requested by Jason Gunthorpe.
    This is consistent with OMAP's GPMC dts binding.

  * Fix documentation as suggested by Jason Gunthorpe.

(Please note that I've droped Thomas' Tested-by since the driver has changed
quite a lot since v1).

Thanks!

Ezequiel Garcia (5):
  drivers: memory: Introduce Marvell EBU Device Bus driver
  ARM: mvebu: Add Device Bus support for Armada 370/XP SoC
  ARM: mvebu: Add support for NOR flash device on Armada XP-GP board
  ARM: mvebu: Add support for NOR flash device on Openblocks AX3 board
  ARM: mvebu: Add Device Bus and CFI flash memory support to defconfig

 .../bindings/memory-controllers/mvebu-devbus.txt   | 151 ++++++++++
 arch/arm/boot/dts/armada-370.dtsi                  |   8 +
 arch/arm/boot/dts/armada-xp-gp.dts                 |  29 ++
 arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts   |  29 ++
 arch/arm/boot/dts/armada-xp.dtsi                   |   8 +
 arch/arm/configs/mvebu_defconfig                   |   7 +
 drivers/memory/Kconfig                             |  10 +
 drivers/memory/Makefile                            |   1 +
 drivers/memory/mvebu-devbus.c                      | 330 +++++++++++++++++++++
 9 files changed, 573 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mvebu-devbus.txt
 create mode 100644 drivers/memory/mvebu-devbus.c

-- 
1.8.1.5

[PATCH v2 1/5] drivers: memory: Introduce Marvell EBU Device Bus driver

[Ezequiel Garcia]

Marvell EBU SoCs such as Armada 370/XP, Orion5x (88f5xxx) and
Discovery (mv78xx0) supports a Device Bus controller to access several
kinds of memories and I/O devices (NOR, NAND, SRAM, FPGA).

This commit adds a driver to handle this controller. So far only
Armada 370, Armada XP SoCs are supported.

The driver must be registered through a device tree node;
as explained in the binding document.

For each child node in the device tree, this driver will:
  * set timing parameters
  * register a child device
  * setup an address decoding window, using the mbus driver

Keep in mind the address decoding window setup is only a temporary hack.
This code will be removed from this devbus driver as soon as a proper device
tree binding for the mbus driver is added.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 .../bindings/memory-controllers/mvebu-devbus.txt   | 151 ++++++++++
 drivers/memory/Kconfig                             |  10 +
 drivers/memory/Makefile                            |   1 +
 drivers/memory/mvebu-devbus.c                      | 330 +++++++++++++++++++++
 4 files changed, 492 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mvebu-devbus.txt
 create mode 100644 drivers/memory/mvebu-devbus.c

diff --git a/Documentation/devicetree/bindings/memory-controllers/mvebu-devbus.txt b/Documentation/devicetree/bindings/memory-controllers/mvebu-devbus.txt
new file mode 100644
index 0000000..5b78940
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/mvebu-devbus.txt
@@ -0,0 +1,151 @@
+Device tree bindings for MVEBU Device Bus controllers
+
+The Device Bus controller available in some Marvell's SoC allows to control
+different types of standard memory and I/O devices such as NOR, NAND, and FPGA.
+The actual devices are instantiated from the child nodes of a Device Bus node.
+
+Required properties:
+
+ - compatible:          Should be set to one of the following:
+
+                        marvell,armada370-devbus
+                        marvell,armadaxp-devbus
+                        marvell,mv78xx0-devbus
+
+ - reg:                 A resource specifier for the register space
+                        (see the example below)
+ - #address-cells:      Must be set to 2 to allow memory address translation
+ - #size-cells:         Must be set to 1 to allow CS address passing
+ - ranges:              Must be set up to reflect the memory layout with four
+                        integer values for each chip-select line in use:
+                        <cs-number> 0 <physical address of mapping> <size>
+
+Mandatory timing properties for child nodes:
+
+Read parameters:
+
+ - devbus,turn-off-ps:  Defines the time during which the controller does not
+                        drive the AD bus after the completion of a device read.
+                        This prevents contentions on the Device Bus after a read
+                        cycle from a slow device.
+
+ - devbus,dev-width:    0x0 = 8-bit
+                        0x1 = 16-bit
+                        0x2 = 32-bit
+                        0x3 = Reserved
+
+ - devbus,badr-skew-ps: Defines the time delay from from A[2:0] toggle,
+                        to read data sample. This parameter is useful for
+                        synchronous pipelined devices, where the address
+                        precedes the read data by one or two cycles.
+
+ - devbus,acc-first-ps: Defines the time delay from the negation of
+                        ALE[0] to the cycle that the first read data is sampled
+                        by the controller.
+
+ - devbus,acc-next-ps:  Defines the time delay between the cycle that
+                        samples data N and the cycle that samples data N+1
+                        (in burst accesses).
+
+ - devbus,rd-setup-ps:  Defines the time delay between DEV_CSn assertion to
+			DEV_OEn assertion. If set to 0 (default),
+                        DEV_OEn and DEV_CSn are asserted at the same cycle.
+                        This parameter has no affect on <acc-first-ps> parameter
+                        (no affect on first data sample). Set <rd-setup-ps>
+                        to a value smaller than <acc-first-ps>.
+
+ - devbus,rd-hold-ps:   Defines the time between the last data sample to the
+			de-assertion of DEV_CSn. If set to 0 (default),
+			DEV_OEn and DEV_CSn are de-asserted at the same cycle
+			(the cycle of the last data sample).
+                        This parameter has no affect on DEV_OEn de-assertion.
+                        DEV_OEn is always de-asserted the next cycle after
+                        last data sampled. Also this parameter has no
+                        affect on <turn-off-ps> parameter.
+                        Set <rd-hold-ps> to a value smaller than <turn-off-ps>.
+
+Write parameters:
+
+ - devbus,ale-wr-ps:    Defines the time delay from the ALE[0] negation cycle
+			to the DEV_WEn assertion.
+
+ - devbus,wr-low-ps:    Defines the time during which DEV_WEn is active.
+                        A[2:0] and Data are kept valid as long as DEV_WEn
+                        is active. This parameter defines the setup time of
+                        address and data to DEV_WEn rise.
+
+ - devbus,wr-high-ps:   Defines the time during which DEV_WEn is kept
+                        inactive (high) between data beats of a burst write.
+                        DEV_A[2:0] and Data are kept valid (do not toggle) for
+                        <wr-high-ps> - <tick> ps.
+			This parameter defines the hold time of address and
+			data after DEV_WEn rise.
+
+ - devbus,sync-enable: Synchronous device enable.
+                       1: True
+                       0: False
+
+An example for an Armada XP GP board, with a 16 MiB NOR device as child
+is showed below. Note that the Device Bus driver is in charge of allocating
+the mbus address decoding window for each of its child devices.
+The window is created using the chip select specified in the child
+device node together with the base address and size specified in the ranges
+property. For instance, in the example below the allocated decoding window
+will start at base address 0xf0000000, with a size 0x1000000 (16 MiB)
+for chip select 0 (a.k.a DEV_BOOTCS).
+
+This address window handling is done in this mvebu-devbus only as a temporary
+solution. It will be removed when the support for mbus device tree binding is
+added.
+
+The reg property must specify the chip select as:
+
+  0: DEV_BOOTCS
+  1: DEV_CS0
+  2: DEV_CS1
+  3: DEV_CS2
+  4: DEV_CS3
+
+Example:
+
+	device-bus at d0010400 {
+		status = "okay";
+		ranges = <0 0 0xf0000000 0x1000000>; /* CS0 @addr 0xf0000000, size 0x1000000 */
+		#address-cells = <2>;
+		#size-cells = <1>;
+
+		nor at 0 {
+			compatible = "cfi-flash";
+
+			/* CS0, 16 MiB */
+			reg = <0 0 0x1000000>;
+			bank-width = <2>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* Device Bus parameters are required */
+			devbus,dev-width = <1>;
+
+			/* Read parameters */
+			devbus,turn-off-ps  = <60000>;
+			devbus,badr-skew-ps = <0>;
+			devbus,acc-first-ps = <124000>;
+			devbus,acc-next-ps  = <248000>;
+			devbus,rd-setup-ps  = <0>;
+			devbus,rd-hold-ps   = <0>;
+
+			/*
+			 * We split the 16 MiB in two partitions,
+			 * just as an example.
+			 */
+			partition at 0 {
+				label = "First";
+				reg = <0 0x800000>;
+			};
+
+			partition at 800000 {
+				label = "Second";
+				reg = <0x800000 0x800000>;
+			};
+		};
+	};
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 067f311..9a81ab3 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -20,6 +20,16 @@ config TI_EMIF
 	  parameters and other settings during frequency, voltage and
 	  temperature changes
 
+config MVEBU_DEVBUS
+	tristate "Marvell EBU Device Bus Controller"
+	default y
+	depends on PLAT_ORION && OF
+	help
+	  This driver is for the Device Bus controller available in some
+	  Marvell EBU SoCs such as Discovery (mv78xx0), Orion (88f5xxx) and
+	  Armada 370 and Armada XP. This controller allows to handle flash
+	  devices such as NOR, NAND, SRAM, and FPGA.
+
 config TEGRA20_MC
 	bool "Tegra20 Memory Controller(MC) driver"
 	default y
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 9cce5d7..969d923 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -6,5 +6,6 @@ ifeq ($(CONFIG_DDR),y)
 obj-$(CONFIG_OF)		+= of_memory.o
 endif
 obj-$(CONFIG_TI_EMIF)		+= emif.o
+obj-$(CONFIG_MVEBU_DEVBUS)	+= mvebu-devbus.o
 obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
 obj-$(CONFIG_TEGRA30_MC)	+= tegra30-mc.o
diff --git a/drivers/memory/mvebu-devbus.c b/drivers/memory/mvebu-devbus.c
new file mode 100644
index 0000000..2a4c455
--- /dev/null
+++ b/drivers/memory/mvebu-devbus.c
@@ -0,0 +1,330 @@
+/*
+ * Marvell EBU SoC Device Bus Controller
+ * (memory controller for NOR/NAND/SRAM/FPGA devices)
+ *
+ * Copyright (C) 2013 Marvell
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/mbus.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+/* Register definitions */
+#define DEV_WIDTH_BIT		30
+#define BADR_SKEW_BIT		28
+#define RD_HOLD_BIT		23
+#define ACC_NEXT_BIT		17
+#define RD_SETUP_BIT		12
+#define ACC_FIRST_BIT		6
+
+#define SYNC_ENABLE_BIT		24
+#define WR_HIGH_BIT		16
+#define WR_LOW_BIT		8
+
+#define READ_PARAM_OFFSET(cs)	(0x0 + (cs << 3))
+#define WRITE_PARAM_OFFSET(cs)	(0x4 + (cs << 3))
+
+#define DEVBUS_MAXCHILDS_NR	5
+
+static const char * const devbus_wins[] = {
+	"devbus-boot",
+	"devbus-cs0",
+	"devbus-cs1",
+	"devbus-cs2",
+	"devbus-cs3",
+};
+
+struct devbus_read_params {
+	u32 dev_width;
+	u32 badr_skew;
+	u32 turn_off;
+	u32 acc_first;
+	u32 acc_next;
+	u32 rd_setup;
+	u32 rd_hold;
+};
+
+struct devbus_write_params {
+	u32 sync_enable;
+	u32 wr_high;
+	u32 wr_low;
+	u32 ale_wr;
+};
+
+struct devbus_child {
+	struct platform_device *pdev;
+	struct resource mem_res;
+};
+
+struct devbus {
+	struct device *dev;
+	void __iomem *base;
+	unsigned long tick_ps;
+
+	struct devbus_child childs[DEVBUS_MAXCHILDS_NR];
+};
+
+static void get_timing_param_ps(struct devbus *devbus,
+				struct device_node *node,
+				const char *name,
+				u32 *ticks)
+{
+	u32 time_ps;
+
+	of_property_read_u32(node, name, &time_ps);
+
+	*ticks = (time_ps + devbus->tick_ps - 1) / devbus->tick_ps;
+
+	dev_dbg(devbus->dev, "%s: %u ps -> 0x%x\n",
+		name, time_ps, *ticks);
+}
+
+static void devbus_set_timing_params(struct devbus *devbus,
+				     struct device_node *node,
+				     int cs)
+{
+	struct devbus_read_params r;
+	struct devbus_write_params w;
+	u32 value;
+
+	dev_dbg(devbus->dev, "Setting timing parameter, tick is %lu ps\n",
+		devbus->tick_ps);
+
+	of_property_read_u32(node, "devbus,dev-width", &r.dev_width);
+
+	/* Get read timings */
+	get_timing_param_ps(devbus, node, "devbus,badr-skew-ps", &r.badr_skew);
+	get_timing_param_ps(devbus, node, "devbus,turn-off-ps", &r.turn_off);
+	get_timing_param_ps(devbus, node, "devbus,acc-first-ps", &r.acc_first);
+	get_timing_param_ps(devbus, node, "devbus,acc-next-ps", &r.acc_next);
+	get_timing_param_ps(devbus, node, "devbus,rd-setup-ps", &r.rd_setup);
+	get_timing_param_ps(devbus, node, "devbus,rd-hold-ps", &r.rd_hold);
+
+	/* Get write timings */
+	get_timing_param_ps(devbus, node, "devbus,ale-wr-ps", &w.ale_wr);
+	get_timing_param_ps(devbus, node, "devbus,wr-low-ps", &w.wr_low);
+	get_timing_param_ps(devbus, node, "devbus,wr-high-ps", &w.wr_high);
+
+	of_property_read_u32(node, "devbus,sync-enable", &w.sync_enable);
+
+	/* Set read timings */
+	value = r.dev_width << DEV_WIDTH_BIT |
+		r.badr_skew << BADR_SKEW_BIT |
+		r.rd_hold   << RD_HOLD_BIT   |
+		r.acc_next  << ACC_NEXT_BIT  |
+		r.rd_setup  << RD_SETUP_BIT  |
+		r.acc_first << ACC_FIRST_BIT |
+		r.turn_off;
+
+	dev_dbg(devbus->dev, "CS%d read parameters register 0x%p = 0x%x\n",
+		cs, devbus->base + READ_PARAM_OFFSET(cs),
+		value);
+
+	writel(value, devbus->base + READ_PARAM_OFFSET(cs));
+
+	/* Set write timings */
+	value = w.sync_enable  << SYNC_ENABLE_BIT |
+		w.wr_low       << WR_LOW_BIT      |
+		w.wr_high      << WR_HIGH_BIT     |
+		w.ale_wr;
+
+	dev_dbg(devbus->dev, "CS%d write parameters register: 0x%p = 0x%x\n",
+		cs, devbus->base + WRITE_PARAM_OFFSET(cs),
+		value);
+
+	writel(value, devbus->base + WRITE_PARAM_OFFSET(cs));
+}
+
+static int devbus_probe_nor_child(struct devbus *devbus,
+				  struct platform_device *pdev,
+				  struct device_node *node)
+{
+	struct device *dev = &pdev->dev;
+	struct devbus_child *child;
+	int err;
+	int bank_width;
+	u32 cs;
+
+	/* Read chip select and size */
+	if (of_property_read_u32(node, "reg", &cs) < 0) {
+		dev_err(dev, "%s has no 'reg' property\n", node->full_name);
+		return -ENODEV;
+	}
+
+	/* Sanity checks */
+	if (cs >= DEVBUS_MAXCHILDS_NR)
+		return -ENODEV;
+
+	if (devbus->childs[cs].pdev)
+		return -EBUSY;
+
+	child = &devbus->childs[cs];
+
+	if (of_address_to_resource(node, 0, &child->mem_res)) {
+		dev_err(dev, "%s has malformed 'reg' property\n",
+			node->full_name);
+		return -ENODEV;
+	}
+
+	err = of_property_read_u32(node, "bank-width", &bank_width);
+	if (err) {
+		dev_err(dev, "error %d reading bank-width property\n", err);
+		return err;
+	}
+
+	/*
+	 * Allocate an address window for this device.
+	 * If the device probing fails, then we won't be able to
+	 * remove the allocated address decoding window.
+	 *
+	 * FIXME: This is only a temporary hack! We need to do this here
+	 * because we still don't have device tree bindings for mbus.
+	 * Once that support is added, we will declare these address windows
+	 * statically in the device tree, and remove the window configuration
+	 * from here.
+	 */
+	err = mvebu_mbus_add_window(devbus_wins[cs], child->mem_res.start,
+				    resource_size(&child->mem_res));
+	if (err < 0)
+		return err;
+
+	/* Read the device tree child node and set the new parameters */
+	devbus_set_timing_params(devbus, node, cs);
+
+	/*
+	 * If we manage to set 'simple-bus' compatible string
+	 * to device-bus node, then we don't really need this.
+	 */
+	child->pdev = of_platform_device_create(node, NULL, &pdev->dev);
+	if (!child->pdev) {
+		dev_warn(dev, "cannot create child device %s\n", node->name);
+		/* Remove the allocated window */
+		mvebu_mbus_del_window(child->mem_res.start,
+				      resource_size(&child->mem_res));
+	}
+
+	return 0;
+}
+
+static int mvebu_devbus_probe(struct platform_device *pdev)
+{
+	struct device_node *child;
+	struct devbus *devbus;
+	struct resource *res;
+	struct clk *clk;
+	unsigned long rate;
+	int err;
+
+	devbus = devm_kzalloc(&pdev->dev, sizeof(struct devbus), GFP_KERNEL);
+	if (!devbus)
+		return -ENOMEM;
+
+	devbus->dev = &pdev->dev;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	devbus->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(devbus->base))
+		return PTR_ERR(devbus->base);
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+	clk_prepare_enable(clk);
+
+	/*
+	 * Obtain clock period in picoseconds,
+	 * we need this in order to convert timing
+	 * parameters from cycles to picoseconds.
+	 */
+	rate = clk_get_rate(clk) / 1000;
+	devbus->tick_ps = 1000000000 / rate;
+
+	/*
+	 * We probe NOR/NAND with different functions, because
+	 * we expect them to have some different parameters.
+	 * If this turns out not to be the case, we'll be able
+	 * to use any name for the child, and rename to devbus_probe_child().
+	 */
+	for_each_node_by_name(child, "nor") {
+		err = devbus_probe_nor_child(devbus, pdev, child);
+		if (err < 0) {
+			of_node_put(child);
+			return err;
+		}
+	}
+
+	platform_set_drvdata(pdev, devbus);
+
+	return 0;
+}
+
+static int mvebu_devbus_remove(struct platform_device *pdev)
+{
+	int i;
+	struct devbus *devbus = platform_get_drvdata(pdev);
+
+	for (i = 0; i < DEVBUS_MAXCHILDS_NR; i++) {
+		struct devbus_child *child;
+
+		if (!devbus->childs[i].pdev)
+			continue;
+
+		child = &devbus->childs[i];
+
+		/* Release device */
+		of_device_unregister(child->pdev);
+
+		/* Release window */
+		mvebu_mbus_del_window(child->mem_res.start,
+				      resource_size(&child->mem_res));
+
+		child->pdev = NULL;
+	}
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+/* Perhaps it makes sense to unify both compatible strins? */
+static const struct of_device_id mvebu_devbus_of_match[] = {
+	{ .compatible = "marvell,armada370-devbus"	 },
+	{ .compatible = "marvell,armadaxp-devbus"	 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mvebu_devbus_of_match);
+
+static struct platform_driver mvebu_devbus_driver = {
+	.probe		= mvebu_devbus_probe,
+	.remove		= mvebu_devbus_remove,
+	.driver		= {
+		.name	= "mvebu-devbus",
+		.owner	= THIS_MODULE,
+		.of_match_table = mvebu_devbus_of_match,
+	},
+};
+
+module_platform_driver(mvebu_devbus_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Ezequiel Garcia <ezequiel.garcia@free-electrons.com>");
+MODULE_DESCRIPTION("Marvell EBU SoC Device Bus controller");
-- 
1.8.1.5

[PATCH v2 2/5] ARM: mvebu: Add Device Bus support for Armada 370/XP SoC

[Ezequiel Garcia]

Armada 370 and Armada XP SoC have a Device Bus controller to
handle NOR, NAND, SRAM and FPGA devices.
This patch adds the device tree node to enable the controller.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370.dtsi | 8 ++++++++
 arch/arm/boot/dts/armada-xp.dtsi  | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index 8188d13..c40358c 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -153,5 +153,13 @@
 			clocks = <&coreclk 0>;
 		};
 
+		device-bus at d0010400 {
+			compatible = "marvell,armada370-devbus";
+			reg = <0xd0010400 0xe0>;
+			#address-cells = <2>;
+			#size-cells = <1>;
+			clocks = <&coreclk 0>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index ca00d83..0c56cca 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -151,5 +151,13 @@
 			status = "disabled";
 		};
 
+		device-bus at d0010400 {
+			compatible = "marvell,armadaxp-devbus";
+			reg = <0xd0010400 0xe0>;
+			#address-cells = <2>;
+			#size-cells = <1>;
+			clocks = <&coreclk 0>;
+			status = "disabled";
+		};
 	};
 };
-- 
1.8.1.5

[PATCH v2 3/5] ARM: mvebu: Add support for NOR flash device on Armada XP-GP board

[Ezequiel Garcia]

The Armada XP Development Board DB-MV784MP-GP has a NOR flash device
connected to the Device Bus. This commit adds the device tree node
to support this device.

This SoC supports a flexible and dynamic decoding window allocation
scheme; but since this feature is still not implemented we need
to specify the window base address in the device tree node itself.

This base address has been selected in a completely arbitrary fashion.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-gp.dts | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts
index 1c8afe2..a2c3862 100644
--- a/arch/arm/boot/dts/armada-xp-gp.dts
+++ b/arch/arm/boot/dts/armada-xp-gp.dts
@@ -109,5 +109,34 @@
 				spi-max-frequency = <108000000>;
 			};
 		};
+
+		device-bus at d0010400 {
+			status = "okay";
+			ranges = <0 0 0xf0000000 0x01000000>; /* CS0 @addr 0xf000000, size 0x1000000 */
+
+			/* CS0, 16 MiB */
+			nor at 0 {
+				compatible = "cfi-flash";
+				reg = <0 0 0x01000000>;
+				bank-width = <2>;
+
+				/* Device Bus parameters are required */
+
+				/* Read parameters */
+				devbus,dev-width    = <1>;
+				devbus,turn-off-ps  = <60000>;
+				devbus,badr-skew-ps = <0>;
+				devbus,acc-first-ps = <124000>;
+				devbus,acc-next-ps  = <248000>;
+				devbus,rd-setup-ps  = <0>;
+				devbus,rd-hold-ps   = <0>;
+
+				/* Write parameters */
+				devbus,sync-enable = <0>;
+				devbus,wr-high-ps  = <60000>;
+				devbus,wr-low-ps   = <60000>;
+				devbus,ale-wr-ps   = <60000>;
+			};
+		};
 	};
 };
-- 
1.8.1.5

[PATCH v2 4/5] ARM: mvebu: Add support for NOR flash device on Openblocks AX3 board

[Ezequiel Garcia]

The Plat'home Openblocks AX3 has a 128 MiB NOR flash device connected
to the Device Bus. This commit adds the device tree node to support this device.

The SoC supports a flexible and dynamic decoding window allocation scheme;
but since this feature is still not implemented we need to specify the window
base address in the device tree node itself.
This base address has been selected in a completely arbitrary fashion.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts | 29 ++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts b/arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts
index 3818a82..53ad2a6 100644
--- a/arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts
+++ b/arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts
@@ -139,5 +139,34 @@
 		usb at d0051000 {
 			status = "okay";
 		};
+
+		device-bus at d0010400 {
+			status = "okay";
+			ranges = <0 0 0xf0000000 0x8000000>; /* CS0 @addr 0xf000000, size 0x8000000 */
+
+			/* CS0, 128 MiB */
+			nor at 0 {
+				compatible = "cfi-flash";
+				reg = <0 0 0x8000000>;
+				bank-width = <2>;
+
+				/* Device Bus parameters are required */
+
+				/* Read parameters */
+				devbus,dev-width    = <1>;
+				devbus,turn-off-ps  = <60000>;
+				devbus,badr-skew-ps = <0>;
+				devbus,acc-first-ps = <124000>;
+				devbus,acc-next-ps  = <248000>;
+				devbus,rd-setup-ps  = <0>;
+				devbus,rd-hold-ps   = <0>;
+
+				/* Write parameters */
+				devbus,sync-enable = <0>;
+				devbus,wr-high-ps  = <60000>;
+				devbus,wr-low-ps   = <60000>;
+				devbus,ale-wr-ps   = <60000>;
+			};
+		};
 	};
 };
-- 
1.8.1.5

[PATCH v2 5/5] ARM: mvebu: Add Device Bus and CFI flash memory support to defconfig

[Ezequiel Garcia]

This patch selects the devbus driver as part of the mvebu default
config, along with the required options to detect and support
CFI flash memories.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/configs/mvebu_defconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/configs/mvebu_defconfig b/arch/arm/configs/mvebu_defconfig
index 2ec8119..fe8ba26 100644
--- a/arch/arm/configs/mvebu_defconfig
+++ b/arch/arm/configs/mvebu_defconfig
@@ -46,6 +46,11 @@ CONFIG_I2C_MV64XXX=y
 CONFIG_MTD=y
 CONFIG_MTD_CHAR=y
 CONFIG_MTD_M25P80=y
+CONFIG_MTD_CFI=y
+CONFIG_MTD_CFI_INTELEXT=y
+CONFIG_MTD_CFI_AMDSTD=y
+CONFIG_MTD_CFI_STAA=y
+CONFIG_MTD_PHYSMAP_OF=y
 CONFIG_SERIAL_8250_DW=y
 CONFIG_GPIOLIB=y
 CONFIG_GPIO_SYSFS=y
@@ -65,6 +70,8 @@ CONFIG_RTC_DRV_S35390A=y
 CONFIG_RTC_DRV_MV=y
 CONFIG_DMADEVICES=y
 CONFIG_MV_XOR=y
+CONFIG_MEMORY=y
+CONFIG_MVEBU_DEVBUS=y
 # CONFIG_IOMMU_SUPPORT is not set
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
-- 
1.8.1.5

[PATCH v2 1/5] drivers: memory: Introduce Marvell EBU Device Bus driver

[Jason Gunthorpe]

On Fri, Apr 05, 2013 at 06:11:56PM -0300, Ezequiel Garcia wrote:

> +The reg property must specify the chip select as:
> +
> +  0: DEV_BOOTCS
> +  1: DEV_CS0
> +  2: DEV_CS1
> +  3: DEV_CS2
> +  4: DEV_CS3

I look at this and sort of go 'hmm'. These are basically register
offsets into the block starting at 0xd0010400. I don't see any
registers that are shared between targets. It would be simpler to keep
each target as a seperate node and seperate driver instance.

Combining that idea with the suggestion for target-id centric mbus DT
binding:

bootcs at d0010400 {
   compatible = "marvell,armada370-devbus";
   ranges = <0 MAPDEF_BOOTCS 0x1000>
   reg = <MAPDEF_INTERNAL + 0x10400 0x8>;  // boot cs register set
   devbus,dev-width = <1>;
   [..] etc

   rom at 0 {
      reg = <0 0x1000>
   }
}

bus_cs3 at d0010400 {
   compatible = "marvell,armada370-devbus";
   ranges = <0 MAPDEF_BUS_CS3 0x1000>
   reg = <MAPDEF_INTERNAL + 0x10408 0x8>; // cs3 register set
   devbus,dev-width = <1>;
   [..] etc

   device at 0 {
      reg = <0 0x1000>
   }
}

Which follows the usual DT convention that the parent bus sets up
properties that apply to all children.

This isn't a major point, but give it a think :)

> +static void get_timing_param_ps(struct devbus *devbus,
> +				struct device_node *node,
> +				const char *name,
> +				u32 *ticks)
> +{
> +	u32 time_ps;
> +
> +	of_property_read_u32(node, name, &time_ps);
> +
> +	*ticks = (time_ps + devbus->tick_ps - 1) / devbus->tick_ps;
> +
> +	dev_dbg(devbus->dev, "%s: %u ps -> 0x%x\n",
> +		name, time_ps, *ticks);
> +}

It looks like there is a problem here, if the properties are not
present then what value will be in time_ps?

The driver should probably fail to load if any timing parameter is
missing from the DT??

> +	/*
> +	 * We probe NOR/NAND with different functions, because
> +	 * we expect them to have some different parameters.
> +	 * If this turns out not to be the case, we'll be able
> +	 * to use any name for the child, and rename to devbus_probe_child().
> +	 */

This statement seems confusing, all this driver does is set
READ_PARAM_OFFSET/WRITE_PARAM_OFFSET - are there other NAND specific
registers? What NOR/NAND difference do you imagine?

I gave it all a quick look over and it looks broadly OK to me
otherwise.

Regards,
Jason

[PATCH v2 1/5] drivers: memory: Introduce Marvell EBU Device Bus driver

[Ezequiel Garcia]

Hi Jason,

Thanks for the review.

On Fri, Apr 05, 2013 at 03:45:10PM -0600, Jason Gunthorpe wrote:
> On Fri, Apr 05, 2013 at 06:11:56PM -0300, Ezequiel Garcia wrote:
> 
> > +The reg property must specify the chip select as:
> > +
> > +  0: DEV_BOOTCS
> > +  1: DEV_CS0
> > +  2: DEV_CS1
> > +  3: DEV_CS2
> > +  4: DEV_CS3
> 
> I look at this and sort of go 'hmm'. These are basically register
> offsets into the block starting at 0xd0010400. I don't see any
> registers that are shared between targets. It would be simpler to keep
> each target as a seperate node and seperate driver instance.
> 

Mmm.. sounds sensible.

> Combining that idea with the suggestion for target-id centric mbus DT
> binding:
> 
> bootcs at d0010400 {
>    compatible = "marvell,armada370-devbus";
>    ranges = <0 MAPDEF_BOOTCS 0x1000>
>    reg = <MAPDEF_INTERNAL + 0x10400 0x8>;  // boot cs register set
>    devbus,dev-width = <1>;
>    [..] etc
> 
>    rom at 0 {
>       reg = <0 0x1000>
>    }
> }
> 
> bus_cs3 at d0010400 {
>    compatible = "marvell,armada370-devbus";
>    ranges = <0 MAPDEF_BUS_CS3 0x1000>
>    reg = <MAPDEF_INTERNAL + 0x10408 0x8>; // cs3 register set
>    devbus,dev-width = <1>;
>    [..] etc
> 
>    device at 0 {
>       reg = <0 0x1000>
>    }
> }
> 
> Which follows the usual DT convention that the parent bus sets up
> properties that apply to all children.
> 
> This isn't a major point, but give it a think :)
> 

Okey, I will.

> > +static void get_timing_param_ps(struct devbus *devbus,
> > +				struct device_node *node,
> > +				const char *name,
> > +				u32 *ticks)
> > +{
> > +	u32 time_ps;
> > +
> > +	of_property_read_u32(node, name, &time_ps);
> > +
> > +	*ticks = (time_ps + devbus->tick_ps - 1) / devbus->tick_ps;
> > +
> > +	dev_dbg(devbus->dev, "%s: %u ps -> 0x%x\n",
> > +		name, time_ps, *ticks);
> > +}
> 
> It looks like there is a problem here, if the properties are not
> present then what value will be in time_ps?
> 
> The driver should probably fail to load if any timing parameter is
> missing from the DT??
> 

Of course, I knew I was forgeting something. We should at least
zero every timing parameter by default.

I'll fix this in a soon to come v3.

Do you think it's also important/necessary to fail if *any*
parameter is missing? I guess it makes sense if we are making
these parameters mandatory.

> > +	/*
> > +	 * We probe NOR/NAND with different functions, because
> > +	 * we expect them to have some different parameters.
> > +	 * If this turns out not to be the case, we'll be able
> > +	 * to use any name for the child, and rename to devbus_probe_child().
> > +	 */
> 
> This statement seems confusing, all this driver does is set
> READ_PARAM_OFFSET/WRITE_PARAM_OFFSET - are there other NAND specific
> registers? What NOR/NAND difference do you imagine?
> 

Yes, you're probably right. Let me fix that too.

> I gave it all a quick look over and it looks broadly OK to me
> otherwise.
> 

Great, thanks!

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v2 1/5] drivers: memory: Introduce Marvell EBU Device Bus driver

[Arnd Bergmann]

On Friday 05 April 2013, Jason Gunthorpe wrote:
> I look at this and sort of go 'hmm'. These are basically register
> offsets into the block starting at 0xd0010400. I don't see any
> registers that are shared between targets. It would be simpler to keep
> each target as a seperate node and seperate driver instance.
> 
> Combining that idea with the suggestion for target-id centric mbus DT
> binding:
> 
> bootcs at d0010400 {
>    compatible = "marvell,armada370-devbus";
>    ranges = <0 MAPDEF_BOOTCS 0x1000>
>    reg = <MAPDEF_INTERNAL + 0x10400 0x8>;  // boot cs register set
>    devbus,dev-width = <1>;
>    [..] etc
> 
>    rom at 0 {
>       reg = <0 0x1000>
>    }
> }

+1

Good idea


	Arnd