ARM: orion5x: big step towards DT conversion

In commit 4ca2c04085a1caa903e92a5fc0da25362150aac2 ('ARM: orion5x:
Move to ID based window creation'), the mach-orion5x code was changed to use the new mvebu-mbus API. However, in the process, a mistake was made on the crypto SRAM window target ID: it should have been 0x9 (verified in the datasheet) and not 0x0.

Signed-off-by Thomas Petazzoni <[email protected]>

Cc stable at vger.kernel.org

---
This should be backported to stable all the way to v3.12.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  arch/arm/mach-orion5x/common.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/mach-orion5x/common.h +1 -1

index f565f99..7548db2 100644
@@ -21,7 +21,7 @@ struct mv_sata_platform_data;
  #define ORION_MBUS_DEVBUS_BOOT_ATTR   0x0f
  #define ORION_MBUS_DEVBUS_TARGET(cs)  0x01
  #define ORION_MBUS_DEVBUS_ATTR(cs)    (~(1 << cs))
-#define ORION_MBUS_SRAM_TARGET        0x00
+#define ORION_MBUS_SRAM_TARGET        0x09
  #define ORION_MBUS_SRAM_ATTR          0x00
 
  /*

In commit 4ca2c04085a1caa903e92a5fc0da25362150aac2 ('ARM: orion5x:
Move to ID based window creation'), the mach-orion5x code was changed to use the new mvebu-mbus API. However, in the process, a mistake was made on the crypto SRAM window target ID: it should have been 0x9 (verified in the datasheet) and not 0x0.

Signed-off-by Thomas Petazzoni <[email protected]>

Cc stable at vger.kernel.org

---
This should be backported to stable all the way to v3.12.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 arch/arm/mach-orion5x/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

This commit extends the pinctrl mvebu logic with a new driver to cover Orion5x SoC. It supports the definitions for the 5181l, 5182 and 5281 variants of Orion5x, which are the three ones supported by the old style MPP code in arch/arm/mach-orion5x/.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  .../bindings/pinctrl/marvell,orion-pinctrl.txt     |  91 +++++++
  drivers/pinctrl/mvebu/Kconfig                      |   4 +
  drivers/pinctrl/mvebu/Makefile                     |   1 +
  drivers/pinctrl/mvebu/pinctrl-orion.c              | 261 +++++++++++++++++++++
  4 files changed, 357 insertions(+)

create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt create mode 100644 drivers/pinctrl/mvebu/pinctrl-orion.c

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt new file +37 -0

new file mode 100644
index 0000000..1748209
@@ -0,0 +1,91 @@
+* Marvell Orion SoC pinctrl driver for mpp
+
+Please refer to marvell,mvebu-pinctrl.txt in this directory for common binding
+part and usage.
+
+Required properties:
+- compatible: "marvell,88f5181l-pinctrl", "marvell,88f5182-pinctrl",
+              "marvell,88f5281-pinctrl"
+
+- reg: two register areas, the first one describing the first two
+  contiguous MPP registers, and the second one describing the single
+  final MPP register, separated from the previous one.
+
+Available mpp pins/groups and functions:
+Note: brackets (x) are not part of the mpp name for marvell,function and given
+only for more detailed description in this document.
+
+* Marvell Orion 88f5181l
+
+name          pins     functions
+================================================================================
+mpp0          0        pci(rstout), pci(req2), gpio
+mpp1          1        gpio, pci(gnt2)
+mpp2          2        gpio, pci(req3), pci(pme)
+mpp3          3        gpio, pci(gnt3)
+mpp4          4        gpio, pci(req4)
+mpp5          5        gpio, pci(gnt4)
+mpp6          6        gpio, pci(req5), pci(clk)
+mpp7          7        gpio, pci(gnt5), pci(clk)
+mpp8          8        gpio, ge(col)
+mpp9          9        gpio, ge(rxerr)
+mpp10         10       gpio, ge(crs)
+mpp11         11       gpio, ge(txerr)
+mpp12         12       gpio, ge(txd4)
+mpp13         13       gpio, ge(txd5)
+mpp14         14       gpio, ge(txd6)
+mpp15         15       gpio, ge(txd7)

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt new file +29 -0

+mpp16         16       uart1(rxd), ge(rxd4), gpio
+mpp17         17       uart1(txd), ge(rxd5), gpio
+mpp18         18       uart1(cts), ge(rxd6), gpio
+mpp19         19       uart1(rts), ge(rxd7), gpio
+
+* Marvell Orion 88f5281
+
+name          pins     functions
+================================================================================
+mpp0          0        pci(rstout), pci(req2), gpio
+mpp1          1        gpio, pci(gnt2)
+mpp2          2        gpio, pci(req3), pci(pme)
+mpp3          3        gpio, pci(gnt3)
+mpp4          4        gpio, pci(req4), bootnand(re)
+mpp5          5        gpio, pci(gnt4), bootnand(we)
+mpp6          6        gpio, pci(req5), nand(re0)
+mpp7          7        gpio, pci(gnt5), nand(we0)
+mpp8          8        gpio, ge(col)
+mpp9          9        gpio, ge(rxerr)
+mpp10         10       gpio, ge(crs)
+mpp11         11       gpio, ge(txerr)
+mpp12         12       gpio, ge(txd4), nand(re1)
+mpp13         13       gpio, ge(txd5), nand(we1)
+mpp14         14       gpio, ge(txd6), nand(re2)
+mpp15         15       gpio, ge(txd7), nand(we2)
+mpp16         16       uart1(rxd), ge(rxd4)
+mpp17         17       uart1(txd), ge(rxd5)
+mpp18         18       uart1(cts), ge(rxd6)
+mpp19         19       uart1(rts), ge(rxd7)

drivers/pinctrl/mvebu/Kconfig +4 -0

index cc298fad..d6dd835 100644
@@ -30,4 +30,8 @@ config PINCTRL_ARMADA_XP
  	bool
  	select PINCTRL_MVEBU
 
+config PINCTRL_ORION
+	bool
+	select PINCTRL_MVEBU
+
  endif

drivers/pinctrl/mvebu/Makefile +1 -0

index bc1b9f1..a0818e9 100644
@@ -5,3 +5,4 @@ obj-$(CONFIG_PINCTRL_ARMADA_370) += pinctrl-armada-370.o
  obj-$(CONFIG_PINCTRL_ARMADA_375) += pinctrl-armada-375.o
  obj-$(CONFIG_PINCTRL_ARMADA_38X) += pinctrl-armada-38x.o
  obj-$(CONFIG_PINCTRL_ARMADA_XP)  += pinctrl-armada-xp.o
+obj-$(CONFIG_PINCTRL_ORION)  += pinctrl-orion.o

drivers/pinctrl/mvebu/pinctrl-orion.c new file +70 -0

new file mode 100644
index 0000000..8312fb3
@@ -0,0 +1,261 @@
+/*
+ * Marvell Orion pinctrl driver based on mvebu pinctrl core
+ *
+ * Author: Thomas Petazzoni <[email protected]>
+ *
+ * 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; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * The first 16 MPP pins on Orion are easy to handle: they are
+ * configured through 2 consecutive registers, located at the base
+ * address of the MPP device.
+ *
+ * However the last 4 MPP pins are handled by a register at offset
+ * 0x50 from the base address, so it is not consecutive with the first
+ * two registers.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-mvebu.h"
+
+static void __iomem *mpp_base;
+static void __iomem *high_mpp_base;
+
+static int orion_mpp_ctrl_get(unsigned pid, unsigned long *config)
+{
+	unsigned shift = (pid % MVEBU_MPPS_PER_REG) * MVEBU_MPP_BITS;
+
+	if (pid < 16) {
+		unsigned off = (pid / MVEBU_MPPS_PER_REG) * MVEBU_MPP_BITS;
+		*config = (readl(mpp_base + off) >> shift) & MVEBU_MPP_MASK;
+	}
+	else {
+		*config = (readl(high_mpp_base) >> shift) & MVEBU_MPP_MASK;
+	}
+
+	return 0;
+}
+
+static int orion_mpp_ctrl_set(unsigned pid, unsigned long config)
+{
+	unsigned shift = (pid % MVEBU_MPPS_PER_REG) * MVEBU_MPP_BITS;
+
+	if (pid < 16) {
+		unsigned off = (pid / MVEBU_MPPS_PER_REG) * MVEBU_MPP_BITS;
+		u32 reg = readl(mpp_base + off) & ~(MVEBU_MPP_MASK << shift);
+		writel(reg | (config << shift), mpp_base + off);
+	}
+	else {
+		u32 reg = readl(high_mpp_base) & ~(MVEBU_MPP_MASK << shift);
+		writel(reg | (config << shift), high_mpp_base);
+	}
+
+	return 0;
+}
+
+#define V(f5181l, f5182, f5281) \
+	((f5181l << 0) | (f5182 << 1) | (f5281 << 2))
+
+enum orion_variant {

drivers/pinctrl/mvebu/pinctrl-orion.c new file +40 -0

+		 MPP_VAR_FUNCTION(0x3, "gpio", NULL,        V_ALL)),
+	MPP_MODE(1,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x2, "pci", "gnt2",       V_ALL)),
+	MPP_MODE(2,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x2, "pci", "req3",       V_ALL),
+		 MPP_VAR_FUNCTION(0x3, "pci", "pme",        V_ALL)),
+	MPP_MODE(3,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x2, "pci", "gnt3",       V_ALL)),
+	MPP_MODE(4,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x2, "pci", "req4",       V_ALL),
+		 MPP_VAR_FUNCTION(0x4, "bootnand", "re",    V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x5, "sata0", "prsnt",    V_5182)),
+	MPP_MODE(5,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x2, "pci", "gnt4",       V_ALL),
+		 MPP_VAR_FUNCTION(0x4, "bootnand", "we",    V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x5, "sata1", "prsnt",    V_5182)),
+	MPP_MODE(6,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x2, "pci", "req5",       V_ALL),
+		 MPP_VAR_FUNCTION(0x4, "nand", "re0",       V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x5, "pci", "clk",        V_5181L),
+		 MPP_VAR_FUNCTION(0x5, "sata0", "act",      V_5182)),
+	MPP_MODE(7,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x2, "pci", "gnt5",       V_ALL),
+		 MPP_VAR_FUNCTION(0x4, "nand", "we0",       V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x5, "pci", "clk",        V_5181L),
+		 MPP_VAR_FUNCTION(0x5, "sata1", "act",      V_5182)),
+	MPP_MODE(8,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x1, "ge", "col",         V_ALL)),
+	MPP_MODE(9,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x1, "ge", "rxerr",       V_ALL)),
+	MPP_MODE(10,

drivers/pinctrl/mvebu/pinctrl-orion.c new file +38 -0

+	MPP_MODE(13,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x1, "ge", "txd5",        V_ALL),
+		 MPP_VAR_FUNCTION(0x4, "nand", "we1",       V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x5, "sata1", "ledprsnt", V_5182)),
+	MPP_MODE(14,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x1, "ge", "txd6",        V_ALL),
+		 MPP_VAR_FUNCTION(0x4, "nand", "re2",       V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x5, "sata0", "ledact",   V_5182)),
+	MPP_MODE(15,
+		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
+		 MPP_VAR_FUNCTION(0x1, "ge", "txd7",        V_ALL),
+		 MPP_VAR_FUNCTION(0x4, "nand", "we2",       V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x5, "sata1", "ledact",   V_5182)),
+	MPP_MODE(16,
+		 MPP_VAR_FUNCTION(0x0, "uart1", "rxd",      V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x1, "ge", "rxd4",        V_ALL),
+		 MPP_VAR_FUNCTION(0x5, "gpio", NULL,        V_5182)),
+	MPP_MODE(17,
+		 MPP_VAR_FUNCTION(0x0, "uart1", "txd",      V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x1, "ge", "rxd5",        V_ALL),
+		 MPP_VAR_FUNCTION(0x5, "gpio", NULL,        V_5182)),
+	MPP_MODE(18,
+		 MPP_VAR_FUNCTION(0x0, "uart1", "cts",      V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x1, "ge", "rxd6",        V_ALL),
+		 MPP_VAR_FUNCTION(0x5, "gpio", NULL,        V_5182)),
+	MPP_MODE(19,
+		 MPP_VAR_FUNCTION(0x0, "uart1", "rts",      V_5182 | V_5281),
+		 MPP_VAR_FUNCTION(0x1, "ge", "rxd7",        V_ALL),
+		 MPP_VAR_FUNCTION(0x5, "gpio", NULL,        V_5182)),
+};
+
+static struct mvebu_mpp_ctrl orion_mpp_controls[] = {
+	MPP_FUNC_CTRL(0, 19, NULL, orion_mpp_ctrl),
+};
+
+static struct pinctrl_gpio_range mv88f5181l_gpio_ranges[] = {

drivers/pinctrl/mvebu/pinctrl-orion.c new file +39 -0

+
+static struct mvebu_pinctrl_soc_info mv88f5181l_info = {
+	.variant = V_5181L,
+	.controls = orion_mpp_controls,
+	.ncontrols = ARRAY_SIZE(orion_mpp_controls),
+	.modes = orion_mpp_modes,
+	.nmodes = ARRAY_SIZE(orion_mpp_modes),
+	.gpioranges = mv88f5181l_gpio_ranges,
+	.ngpioranges = ARRAY_SIZE(mv88f5181l_gpio_ranges),
+};
+
+static struct mvebu_pinctrl_soc_info mv88f5182_info = {
+	.variant = V_5182,
+	.controls = orion_mpp_controls,
+	.ncontrols = ARRAY_SIZE(orion_mpp_controls),
+	.modes = orion_mpp_modes,
+	.nmodes = ARRAY_SIZE(orion_mpp_modes),
+	.gpioranges = mv88f5182_gpio_ranges,
+	.ngpioranges = ARRAY_SIZE(mv88f5182_gpio_ranges),
+};
+
+static struct mvebu_pinctrl_soc_info mv88f5281_info = {
+	.variant = V_5281,
+	.controls = orion_mpp_controls,
+	.ncontrols = ARRAY_SIZE(orion_mpp_controls),
+	.modes = orion_mpp_modes,
+	.nmodes = ARRAY_SIZE(orion_mpp_modes),
+	.gpioranges = mv88f5281_gpio_ranges,
+	.ngpioranges = ARRAY_SIZE(mv88f5281_gpio_ranges),
+};
+
+/*
+ * There are multiple variants of the Orion SoCs, but in terms of pin
+ * muxing, they are identical.
+ */
+static struct of_device_id orion_pinctrl_of_match[] = {
+	{ .compatible = "marvell,88f5181l-pinctrl", .data = &mv88f5181l_info },
+	{ .compatible = "marvell,88f5182-pinctrl", .data = &mv88f5182_info },
+	{ .compatible = "marvell,88f5281-pinctrl", .data = &mv88f5281_info },

drivers/pinctrl/mvebu/pinctrl-orion.c new file +34 -0

+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mpp_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mpp_base))
+		return PTR_ERR(mpp_base);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	high_mpp_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(high_mpp_base))
+		return PTR_ERR(high_mpp_base);
+
+	return mvebu_pinctrl_probe(pdev);
+}
+
+static int orion_pinctrl_remove(struct platform_device *pdev)
+{
+	return mvebu_pinctrl_remove(pdev);
+}
+
+static struct platform_driver orion_pinctrl_driver = {
+	.driver = {
+		.name = "orion-pinctrl",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(orion_pinctrl_of_match),
+	},
+	.probe = orion_pinctrl_probe,
+	.remove = orion_pinctrl_remove,
+};
+
+module_platform_driver(orion_pinctrl_driver);
+
+MODULE_AUTHOR("Thomas Petazzoni <[email protected]>");
+MODULE_DESCRIPTION("Marvell Orion pinctrl driver");
+MODULE_LICENSE("GPL v2");

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt line 47

Quoted 5 lines

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt new file +5 -0

+* Marvell Orion 88f5182
+
+name          pins     functions
+================================================================================
+mpp0          0        pci(rstout), pci(req2), gpio

Shouldn't the above pcie(rstout)?

The one available datasheet I have for 5182 names the function PEX_RST_OUTn, which is PCIexpress then.

Review

Quoted 2 lines

(quoted diff) +2 -0

+mpp1          1        gpio, pci(gnt2)
+mpp2          2        gpio, pci(req3), pci(pme)

pinctrl-mvebu isn't that good with identically named functions. You need to rename one of the "pci" functions to e.g. "pci-1".

Quoted 7 lines

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt new file +7 -0

+mpp3          3        gpio, pci(gnt3)
+mpp4          4        gpio, pci(req4), bootnand(re), sata0(prsnt)
+mpp5          5        gpio, pci(gnt4), bootnand(we), sata1(prsnt)
+mpp6          6        gpio, pci(req5), nand(re0), sata0(act)
+mpp7          7        gpio, pci(gnt5), nand(we0), sata1(act)
+mpp8          8        gpio, ge(col)
+mpp9          9        gpio, ge(rxerr)

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt line 62

Quoted 6 lines

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt new file +6 -0

+mpp10         10       gpio, ge(crs)
+mpp11         11       gpio, ge(txerr)
+mpp12         12       gpio, ge(txd4), nand(re1), sata0(ledprsnt)
+mpp13         13       gpio, ge(txd5), nand(we1), sata1(ledprsnt)
+mpp14         14       gpio, ge(txd6), nand(re2), sata0(ledact)
+mpp15         15       gpio, ge(txd7), nand(we2), sata1(ledact)

Four "led" prefixes above should be removed.

Quoted 5 lines

drivers/pinctrl/mvebu/pinctrl-orion.c new file +5 -0

+static struct mvebu_mpp_ctrl orion_mpp_controls[] = {
+	MPP_FUNC_CTRL(0, 19, NULL, orion_mpp_ctrl),
+};
+
+static struct pinctrl_gpio_range mv88f5181l_gpio_ranges[] = {

drivers/pinctrl/mvebu/pinctrl-orion.c line 178

Quoted 10 lines

drivers/pinctrl/mvebu/pinctrl-orion.c new file +10 -0

+	MPP_GPIO_RANGE(0, 0, 0, 16),
+};
+
+static struct pinctrl_gpio_range mv88f5182_gpio_ranges[] = {
+	MPP_GPIO_RANGE(0, 0, 0, 19),
+};
+
+static struct pinctrl_gpio_range mv88f5281_gpio_ranges[] = {
+	MPP_GPIO_RANGE(0, 0, 0, 16),
+};

mv88f5181l_gpio_ranges == mv88f5281_gpio_ranges.

You can possibly join them to mv88f5x81_gpio ranges, but I have no strong opinion about it.

drivers/pinctrl/mvebu/pinctrl-orion.c line 227

Quoted 7 lines

drivers/pinctrl/mvebu/pinctrl-orion.c new file +7 -0

+static int orion_pinctrl_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match =
+		of_match_device(orion_pinctrl_of_match, &pdev->dev);
+	struct resource *res;
+
+	pdev->dev.platform_data = (void*) match->data;

Useless (void *) cast?

Quoted 7 lines · Sebastian Hesselbarth

Quoted 7 lines

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt new file +7 -0

+mpp3          3        gpio, pci(gnt3)
+mpp4          4        gpio, pci(req4), bootnand(re), sata0(prsnt)
+mpp5          5        gpio, pci(gnt4), bootnand(we), sata1(prsnt)
+mpp6          6        gpio, pci(req5), nand(re0), sata0(act)
+mpp7          7        gpio, pci(gnt5), nand(we0), sata1(act)
+mpp8          8        gpio, ge(col)
+mpp9          9        gpio, ge(rxerr)

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt line 62

Quoted 6 lines

Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt new file +6 -0

+mpp10         10       gpio, ge(crs)
+mpp11         11       gpio, ge(txerr)
+mpp12         12       gpio, ge(txd4), nand(re1), sata0(ledprsnt)
+mpp13         13       gpio, ge(txd5), nand(we1), sata1(ledprsnt)
+mpp14         14       gpio, ge(txd6), nand(re2), sata0(ledact)
+mpp15         15       gpio, ge(txd7), nand(we2), sata1(ledact)

Four "led" prefixes above should be removed.

I don't agree, because there would then be no difference between sata0(act) and sata0(ledact), even if in the datasheet, their description is different:

* SATA 0 active indication (for which I've used "sata0(act)")
* SATA 0 presence LED indication (Active Low) (for which I've used "sata0(ledact)")

Do you have another suggestion?

Quoted 18 lines

Quoted 5 lines

drivers/pinctrl/mvebu/pinctrl-orion.c new file +5 -0

+static struct mvebu_mpp_ctrl orion_mpp_controls[] = {
+	MPP_FUNC_CTRL(0, 19, NULL, orion_mpp_ctrl),
+};
+
+static struct pinctrl_gpio_range mv88f5181l_gpio_ranges[] = {

drivers/pinctrl/mvebu/pinctrl-orion.c line 178

Quoted 10 lines

drivers/pinctrl/mvebu/pinctrl-orion.c new file +10 -0

+	MPP_GPIO_RANGE(0, 0, 0, 16),
+};
+
+static struct pinctrl_gpio_range mv88f5182_gpio_ranges[] = {
+	MPP_GPIO_RANGE(0, 0, 0, 19),
+};
+
+static struct pinctrl_gpio_range mv88f5281_gpio_ranges[] = {
+	MPP_GPIO_RANGE(0, 0, 0, 16),
+};

mv88f5181l_gpio_ranges == mv88f5281_gpio_ranges.

Quoted 2 lines

You can possibly join them to mv88f5x81_gpio ranges, but I have no strong opinion about it.

I prefer to have them all for each SoC, for consistency.

Quoted 9 lines

drivers/pinctrl/mvebu/pinctrl-orion.c line 227

Quoted 7 lines

drivers/pinctrl/mvebu/pinctrl-orion.c new file +7 -0

+static int orion_pinctrl_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match =
+		of_match_device(orion_pinctrl_of_match, &pdev->dev);
+	struct resource *res;
+
+	pdev->dev.platform_data = (void*) match->data;

Useless (void *) cast?

No: there is the same cast in pinctrl-dove.c and pinctrl-kirkwood.c. The reason is a bit ugly: match->data is "const" but pdev->dev.platform_data is not. Certainly something to fix at some point, but probably not as part of this patch series, since Dove and Kirkwood are already doing the bad thing :)

This commit adds a core clock driver for the Orion5x SoC, with support for the tclk, the CPU frequency and the DDR frequency. All the details about the Sample-At-Reset register were extracted from the U-Boot sources for Orion5x.

Note that Orion5x does not have gatable clocks, so this core clock driver is sufficient to support clocking on Orion5x platforms.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  .../devicetree/bindings/clock/mvebu-core-clock.txt |   8 +
  drivers/clk/mvebu/Kconfig                          |   4 +
  drivers/clk/mvebu/Makefile                         |   1 +
  drivers/clk/mvebu/orion.c                          | 212 +++++++++++++++++++++
  4 files changed, 225 insertions(+)

create mode 100644 drivers/clk/mvebu/orion.c

Documentation/devicetree/bindings/clock/mvebu-core-clock.txt +0 -0

index 307a503..dc5ea5b 100644

Documentation/devicetree/bindings/clock/mvebu-core-clock.txt +2 -0

+	"marvell,mv88f5281-core-clock" - for Orion MV88F5281 SoC
+	"marvell,mv88f6183-core-clock" - for Orion MV88F6183 SoC
  - reg : shall be the register address of the Sample-At-Reset (SAR) register
  - #clock-cells : from common clock binding; shall be set to 1

drivers/clk/mvebu/orion.c new file +17 -0

new file mode 100644
index 0000000..005aa2f
@@ -0,0 +1,212 @@
+/*
+ * Marvell Orion SoC clocks
+ *
+ * Copyright (C) 2014 Thomas Petazzoni
+ *
+ * Thomas Petazzoni <[email protected]>
+ *
+ * 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/kernel.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include "common.h"

drivers/clk/mvebu/orion.c new file +5 -0

+#define SAR_MV88F5182_TCLK_FREQ_MASK 0x3
+
+static u32 __init mv88f5182_get_tclk_freq(void __iomem *sar)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5182_TCLK_FREQ) &

drivers/clk/mvebu/orion.c new file +7 -0

+	else if (opt == 3)
+		return 500000000;
+	else
+		return 0;
+}
+
+static void __init mv88f5182_get_clk_ratio(void __iomem *sar, int id,

drivers/clk/mvebu/orion.c new file +22 -0

+	} else {
+		*mult = 0;
+		*div  = 1;
+	}
+}
+
+static const struct coreclk_soc_desc mv88f5182_coreclks = {
+	.get_tclk_freq = mv88f5182_get_tclk_freq,
+	.get_cpu_freq = mv88f5182_get_cpu_freq,
+	.get_clk_ratio = mv88f5182_get_clk_ratio,
+	.ratios = orion_coreclk_ratios,
+	.num_ratios = ARRAY_SIZE(orion_coreclk_ratios),
+};
+
+static void __init mv88f5182_clk_init(struct device_node *np)
+{
+	return mvebu_coreclk_setup(np, &mv88f5182_coreclks);
+}
+
+CLK_OF_DECLARE(mv88f5182_clk, "marvell,mv88f5182-core-clock", mv88f5182_clk_init);
+
+/*

drivers/clk/mvebu/orion.c new file +41 -0

+#define SAR_MV88F5281_CPU_FREQ_MASK  0xf
+
+static u32 __init mv88f5281_get_cpu_freq(void __iomem *sar)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5281_CPU_FREQ) &
+		SAR_MV88F5281_CPU_FREQ_MASK;
+	if (opt == 1 || opt == 2)
+		return 400000000;
+	else if (opt == 3)
+		return 500000000;
+	else
+		return 0;
+}
+
+static void __init mv88f5281_get_clk_ratio(void __iomem *sar, int id,
+					   int *mult, int *div)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5281_CPU_FREQ) &
+		SAR_MV88F5281_CPU_FREQ_MASK;
+	if (opt == 1) {
+		*mult = 1;
+		*div = 2;
+	} else if (opt == 2 || opt == 3) {
+		*mult = 1;
+		*div = 3;
+	} else {
+		*mult = 0;
+		*div = 1;
+	}
+}
+
+static const struct coreclk_soc_desc mv88f5281_coreclks = {
+	.get_tclk_freq = mv88f5281_get_tclk_freq,
+	.get_cpu_freq = mv88f5281_get_cpu_freq,
+	.get_clk_ratio = mv88f5281_get_clk_ratio,
+	.ratios = orion_coreclk_ratios,
+	.num_ratios = ARRAY_SIZE(orion_coreclk_ratios),
+};
+
+static void __init mv88f5281_clk_init(struct device_node *np)
+{

drivers/clk/mvebu/orion.c new file +5 -0

+#define SAR_MV88F6183_TCLK_FREQ_MASK 0x1
+
+static u32 __init mv88f6183_get_tclk_freq(void __iomem *sar)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F6183_TCLK_FREQ) &

drivers/clk/mvebu/orion.c new file +43 -0

+#define SAR_MV88F6183_CPU_FREQ_MASK  0x3f
+
+static u32 __init mv88f6183_get_cpu_freq(void __iomem *sar)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F6183_CPU_FREQ) &
+		SAR_MV88F6183_CPU_FREQ_MASK;
+	if (opt == 9)
+		return 333333333;
+	else if (opt == 17)
+		return 400000000;
+	else
+		return 0;
+}
+
+static void __init mv88f6183_get_clk_ratio(void __iomem *sar, int id,
+					   int *mult, int *div)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F6183_CPU_FREQ) &
+		SAR_MV88F6183_CPU_FREQ_MASK;
+	if (opt == 9 || opt == 17) {
+		*mult = 1;
+		*div  = 2;
+	} else {
+		*mult = 0;
+		*div  = 1;
+	}
+}
+
+static const struct coreclk_soc_desc mv88f6183_coreclks = {
+	.get_tclk_freq = mv88f6183_get_tclk_freq,
+	.get_cpu_freq = mv88f6183_get_cpu_freq,
+	.get_clk_ratio = mv88f6183_get_clk_ratio,
+	.ratios = orion_coreclk_ratios,
+	.num_ratios = ARRAY_SIZE(orion_coreclk_ratios),
+};
+
+
+static void __init mv88f6183_clk_init(struct device_node *np)
+{
+	return mvebu_coreclk_setup(np, &mv88f6183_coreclks);
+}
+
+CLK_OF_DECLARE(mv88f6183_clk, "marvell,mv88f6183-core-clock", mv88f6183_clk_init);

According to the Armada 370 and Armada XP datasheets, the part of the Device Bus register that configure the bus width should contain 0 for a 8 bits bus width, and 1 for a 16 bits bus width (other values are unsupported/reserved).

However, the current conversion done in the driver to convert from a bus width in bits to the value expected by the register leads to setting the register to 1 for a 8 bits bus, and 2 for a 16 bits bus.

This commit fixes that by adjusting the conversion logic.

This patch fixes a bug that was introduced in 3edad321b1bd2e6c8b5f38146c115c8982438f06 ('drivers: memory: Introduce Marvell EBU Device Bus driver'), which was merged in v3.11.

Signed-off-by Thomas Petazzoni <[email protected]>

Cc stable at vger.kernel.org

---
  drivers/memory/mvebu-devbus.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

drivers/memory/mvebu-devbus.c +0 -0

index 110c036..b82cdf1 100644
@@ -108,8 +108,12 @@ static int devbus_set_timing_params(struct devbus *devbus,
  			node->full_name);

drivers/memory/mvebu-devbus.c +0 -0

  	err = get_timing_param_ps(devbus, node, "devbus,badr-skew-ps",
  				 &r.badr_skew);
 

The mvebu-devbus driver currently only supports the Armada 370/XP family, but it can also cover the Orion5x family. However, the Orion5x family has a different organization of the register. Therefore, in preparation to the introduction of Orion5x support, we rename the Armada 370/XP specific definitions to have an ARMADA_ prefix.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  drivers/memory/mvebu-devbus.c | 48 +++++++++++++++++++++----------------------
  1 file changed, 24 insertions(+), 24 deletions(-)

drivers/memory/mvebu-devbus.c +0 -2

index b82cdf1..84746e3 100644
@@ -30,19 +30,19 @@
  #include <linux/platform_device.h>
 
  /* Register definitions */
-#define DEV_WIDTH_BIT		30
-#define BADR_SKEW_BIT		28

drivers/memory/mvebu-devbus.c +18 -18

-#define SYNC_ENABLE_BIT		24
-#define WR_HIGH_BIT		16
-#define WR_LOW_BIT		8
+#define ARMADA_SYNC_ENABLE_BIT		24
+#define ARMADA_WR_HIGH_BIT		16
+#define ARMADA_WR_LOW_BIT		8
 
-#define READ_PARAM_OFFSET	0x0
-#define WRITE_PARAM_OFFSET	0x4
+#define ARMADA_READ_PARAM_OFFSET	0x0
+#define ARMADA_WRITE_PARAM_OFFSET	0x4
 
  struct devbus_read_params {
  	u32 bus_width;

@@ -171,31 +171,31 @@ static int devbus_set_timing_params(struct devbus *devbus,
  		return err;
 
  	/* Set read timings */
-	value = r.bus_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 |
+	value = r.bus_width << ARMADA_DEV_WIDTH_BIT |
+		r.badr_skew << ARMADA_BADR_SKEW_BIT |
+		r.rd_hold   << ARMADA_RD_HOLD_BIT   |
+		r.acc_next  << ARMADA_ACC_NEXT_BIT  |
+		r.rd_setup  << ARMADA_RD_SETUP_BIT  |
+		r.acc_first << ARMADA_ACC_FIRST_BIT |
  		r.turn_off;
 
  	dev_dbg(devbus->dev, "read parameters register 0x%p = 0x%x\n",
-		devbus->base + READ_PARAM_OFFSET,
+		devbus->base + ARMADA_READ_PARAM_OFFSET,
  		value);
 
-	writel(value, devbus->base + READ_PARAM_OFFSET);
+	writel(value, devbus->base + ARMADA_READ_PARAM_OFFSET);
 
  	/* Set write timings */
-	value = w.sync_enable  << SYNC_ENABLE_BIT |
-		w.wr_low       << WR_LOW_BIT      |
-		w.wr_high      << WR_HIGH_BIT     |
+	value = w.sync_enable  << ARMADA_SYNC_ENABLE_BIT |
+		w.wr_low       << ARMADA_WR_LOW_BIT      |
+		w.wr_high      << ARMADA_WR_HIGH_BIT     |
  		w.ale_wr;
 
  	dev_dbg(devbus->dev, "write parameters register: 0x%p = 0x%x\n",
-		devbus->base + WRITE_PARAM_OFFSET,
+		devbus->base + ARMADA_WRITE_PARAM_OFFSET,
  		value);
 
-	writel(value, devbus->base + WRITE_PARAM_OFFSET);
+	writel(value, devbus->base + ARMADA_WRITE_PARAM_OFFSET);
 
  	return 0;
  }

The mvebu-devbus driver currently only supports the Armada 370/XP family, but it can also cover the Orion5x family. However, the Orion5x family has a different organization of the registers.

Therefore, in preparation to the introduction of Orion5x support, we separate into two functions the code that 1/ retrieves the timing parameters from the Device Tree and 2/ applies those timings parameters into the hardware registers.

Signed-off-by Thomas Petazzoni <[email protected]>

This commit adds support for the Orion5x family of Marvell processors into the mvebu-devbus driver. It differs from the already supported Armada 370/XP by:

* Having a single register (instead of two) for doing all the timing configuration.

* Having a few less timing configuration parameters.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  .../bindings/memory-controllers/mvebu-devbus.txt   | 25 ++++--
  drivers/memory/mvebu-devbus.c                      | 95 +++++++++++++++++-----
  2 files changed, 94 insertions(+), 26 deletions(-)

Documentation/devicetree/bindings/memory-controllers/mvebu-devbus.txt +11 -5

index 653c90c..aff8e9e 100644
@@ -6,10 +6,11 @@ The actual devices are instantiated from the child nodes of a Device Bus node.
 
  Required properties:
 
- - compatible:          Currently only Armada 370/XP SoC are supported,
-                        with this compatible string:
+ - compatible:          Armada 370/XP SoC are supported using the
+                        "marvell,mvebu-devbus" compatible string.
 
-                        marvell,mvebu-devbus
+                        Orion5x SoC are supported using the
+                        "marvell,orion-devbus" compatible string.
 
   - reg:                 A resource specifier for the register space.
                          This is the base address of a chip select within

@@ -22,7 +23,7 @@ Required properties:
                          integer values for each chip-select line in use:
                          0 <physical address of mapping> <size>
 
-Mandatory timing properties for child nodes:
+Timing properties for child nodes:
 
  Read parameters:
 

@@ -30,21 +31,26 @@ Read parameters:
                          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.
+			Mandatory.
 
- - devbus,bus-width:    Defines the bus width (e.g. <16>)
+ - devbus,bus-width:    Defines the bus width, in bits (e.g. <16>).
+   			Mandatory.
 
   - 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.
+			Mandatory.
 
   - 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.
+			Mandatory.
 
   - 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).
+			Mandaory.
 
   - devbus,rd-setup-ps:  Defines the time delay between DEV_CSn assertion to
  			DEV_OEn assertion. If set to 0 (default),

Documentation/devicetree/bindings/memory-controllers/mvebu-devbus.txt +2 -0

  Write parameters:
 
   - devbus,ale-wr-ps:    Defines the time delay from the ALE[0] negation cycle
  			to the DEV_WEn assertion.
+			Mandatory.
 
   - 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.
+			Mandatory.
 
   - devbus,wr-high-ps:   Defines the time during which DEV_WEn is kept
                          inactive (high) between data beats of a burst write.

Documentation/devicetree/bindings/memory-controllers/mvebu-devbus.txt +0 -0

  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

drivers/memory/mvebu-devbus.c +6 -1

index 8df5b2b..59d8151 100644
@@ -2,7 +2,7 @@
   * Marvell EBU SoC Device Bus Controller
   * (memory controller for NOR/NAND/SRAM/FPGA devices)
   *
- * Copyright (C) 2013 Marvell
+ * Copyright (C) 2013-2014 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

@@ -44,6 +44,22 @@
  #define ARMADA_READ_PARAM_OFFSET	0x0
  #define ARMADA_WRITE_PARAM_OFFSET	0x4
 
+#define ORION_RESERVED			(0x2 << 30)
+#define ORION_BADR_SKEW_BIT		28
+#define ORION_WR_HIGH_EXT_BIT		27
+#define ORION_WR_LOW_EXT_BIT		26
+#define ORION_ALE_WR_EXT_BIT		25

drivers/memory/mvebu-devbus.c +37 -19

+
  struct devbus_read_params {
  	u32 bus_width;
  	u32 badr_skew;

@@ -96,7 +112,6 @@ static int devbus_get_timing_params(struct devbus *devbus,
  {
  	int err;
 
-	/* Get read timings */
  	err = of_property_read_u32(node, "devbus,bus-width", &r->bus_width);
  	if (err < 0) {
  		dev_err(devbus->dev,

@@ -131,24 +146,25 @@ static int devbus_get_timing_params(struct devbus *devbus,
  	if (err < 0)
  		return err;
 
-	err = get_timing_param_ps(devbus, node, "devbus,rd-setup-ps",
-				 &r->rd_setup);
-	if (err < 0)
-		return err;
-
-	err = get_timing_param_ps(devbus, node, "devbus,rd-hold-ps",
-				 &r->rd_hold);
-	if (err < 0)
-		return err;
-
-	/* Get write timings */
-	err = of_property_read_u32(node, "devbus,sync-enable",
-				  &w->sync_enable);
-	if (err < 0) {
-		dev_err(devbus->dev,
-			"%s has no 'devbus,sync-enable' property\n",
-			node->full_name);
-		return err;
+	if (of_device_is_compatible(devbus->dev->of_node, "marvell,mvebu-devbus")) {
+		err = get_timing_param_ps(devbus, node, "devbus,rd-setup-ps",
+					  &r->rd_setup);
+		if (err < 0)
+			return err;
+
+		err = get_timing_param_ps(devbus, node, "devbus,rd-hold-ps",
+					  &r->rd_hold);
+		if (err < 0)
+			return err;
+
+		err = of_property_read_u32(node, "devbus,sync-enable",
+					   &w->sync_enable);
+		if (err < 0) {
+			dev_err(devbus->dev,
+				"%s has no 'devbus,sync-enable' property\n",
+				node->full_name);
+			return err;
+		}
  	}
 
  	err = get_timing_param_ps(devbus, node, "devbus,ale-wr-ps",

@@ -169,6 +185,39 @@ static int devbus_get_timing_params(struct devbus *devbus,
  	return 0;
  }
 
+static void devbus_orion_set_timing_params(struct devbus *devbus,
+					  struct device_node *node,
+					  struct devbus_read_params *r,
+					  struct devbus_write_params *w)
+{
+	u32 value;
+
+	/*
+	 * The hardware designers found it would be a good idea to
+	 * split most of the values in the register into two fields:
+	 * one containing all the low-order bits, and another one
+	 * containing just the high-order bit. For all of those
+	 * fields, we have to split the value into these two parts.
+	 */
+	value = (r->turn_off   & 0x7) << ORION_TURN_OFF_BIT  |
+		(r->acc_first  & 0xF) << ORION_ACC_FIRST_BIT |
+		(r->acc_next   & 0xF) << ORION_ACC_NEXT_BIT  |

drivers/memory/mvebu-devbus.c +11 -1

+		(r->badr_skew << ORION_BADR_SKEW_BIT) |
+		ORION_RESERVED;
+
+	writel(value, devbus->base);
+}
+
  static void devbus_armada_set_timing_params(struct devbus *devbus,
  					   struct device_node *node,
  					   struct devbus_read_params *r,

@@ -248,7 +297,10 @@ static int mvebu_devbus_probe(struct platform_device *pdev)
  		return err;
 
  	/* Set the new timing parameters */
-	devbus_armada_set_timing_params(devbus, node, &r, &w);
+	if (of_device_is_compatible(node, "marvell,orion-devbus"))
+		devbus_orion_set_timing_params(devbus, node, &r, &w);
+	else
+		devbus_armada_set_timing_params(devbus, node, &r, &w);
 
  	/*
  	 * We need to create a child device explicitly from here to

@@ -264,6 +316,7 @@ static int mvebu_devbus_probe(struct platform_device *pdev)
 
  static const struct of_device_id mvebu_devbus_of_match[] = {
  	{ .compatible = "marvell,mvebu-devbus" },
+	{ .compatible = "marvell,orion-devbus" },
  	{},
  };
  MODULE_DEVICE_TABLE(of, mvebu_devbus_of_match);

The interrupt controller node was located outside of the ocp at f1000000 node, which doesn't make much sense: like any other device, the interrupt controller has registers located in the "Internal Registers Window", so it is much more logical to have it under the ocp at f1000000 node.

It is even more important as we are going to move Orion5x to use the Device Tree binding of the mvebu-mbus driver.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  arch/arm/boot/dts/orion5x.dtsi | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)

arch/arm/boot/dts/orion5x.dtsi +7 -7

index 174d892..da57fb9 100644
@@ -17,13 +17,6 @@
  		gpio0 = &gpio0;
  	};
 
-	intc: interrupt-controller {
-		compatible = "marvell,orion-intc";
-		interrupt-controller;
-		#interrupt-cells = <1>;
-		reg = <0xf1020200 0x08>;
-	};
-
  	ocp at f1000000 {
  		compatible = "simple-bus";
  		ranges = <0x00000000 0xf1000000 0x4000000

@@ -79,6 +72,13 @@
  			status = "disabled";
  		};
 
+		intc: interrupt-controller at 20200 {
+			compatible = "marvell,orion-intc";
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			reg = <0x20200 0x08>;
+		};
+
  		wdt at 20300 {
  			compatible = "marvell,orion-wdt";
  			reg = <0x20300 0x28>;

This commit switches the Orion5x Device Tree files to use C preprocessor based includes, as it will allow us to use definitions from header files in future commits.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  arch/arm/boot/dts/orion5x-lacie-ethernet-disk-mini-v2.dts | 3 ++-
  arch/arm/boot/dts/orion5x.dtsi                            | 2 +-
  2 files changed, 3 insertions(+), 2 deletions(-)

arch/arm/boot/dts/orion5x-lacie-ethernet-disk-mini-v2.dts +2 -1

index 5ed6c13..0245bb3 100644
@@ -7,7 +7,8 @@
   */
 
  /dts-v1/;
-/include/ "orion5x.dtsi"
+
+#include "orion5x.dtsi"
 
  / {
  	model = "LaCie Ethernet Disk mini V2";

arch/arm/boot/dts/orion5x.dtsi +1 -1

index da57fb9..2364e3d 100644
@@ -6,7 +6,7 @@
   * warranty of any kind, whether express or implied.
   */
 
-/include/ "skeleton.dtsi"
+#include "skeleton.dtsi"
 
  / {
  	model = "Marvell Orion5x SoC";

The orion5x-lacie-ethernet-disk-mini-v2.dts can benefit from using gpio.h and input.h dt-bindings headers to replace hardcoded values by more meaningful macros.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  arch/arm/boot/dts/orion5x-lacie-ethernet-disk-mini-v2.dts | 8 +++++---
  1 file changed, 5 insertions(+), 3 deletions(-)

arch/arm/boot/dts/orion5x-lacie-ethernet-disk-mini-v2.dts +5 -3

index 0245bb3..24f1ce7 100644
@@ -8,6 +8,8 @@
 
  /dts-v1/;
 
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
  #include "orion5x.dtsi"
 
  / {

@@ -40,8 +42,8 @@
  		#size-cells = <0>;
  		button at 1 {
  			label = "Power-on Switch";
-			linux,code = <116>; /* KEY_POWER */
-			gpios = <&gpio0 18 0>;
+			linux,code = <KEY_POWER>;
+			gpios = <&gpio0 18 GPIO_ACTIVE_HIGH>;
  		};
  	};
 

@@ -50,7 +52,7 @@
 
  		led at 1 {
  			label = "power:blue";
-			gpios = <&gpio0 16 1>;
+			gpios = <&gpio0 16 GPIO_ACTIVE_LOW>;
  		};
  	};
  };

This commit switches the Orion5x Device Tree files to use the DT representation and probing for the mvebu-mbus driver. The changes are mainly:

* Re-organize the DT to follow the same organization as the one used on Armada 370/XP, which is needed for mvebu-mbus to work: a top-level soc { ... } node, which corresponds to the MBus bus, and a sub-node internal-regs { ... } for all peripherals whose register sit only in the "Internal Register Window". This change re-indents by one level the definition of all nodes in the Device Tree, which explains the large change.

* Use custom functions orion5x_dt_init_early() and orion5x_dt_init_time() instead of orion5x_init_early() and orion5x_timer_init() as we now want the MBus driver to be probed from the Device Tree. We still use the old-style timer initialization, but that will be changed in a followup commit.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  .../dts/orion5x-lacie-ethernet-disk-mini-v2.dts    |  23 +-
  arch/arm/boot/dts/orion5x-mv88f5182.dtsi           |  24 ++
  arch/arm/boot/dts/orion5x.dtsi                     | 245 +++++++++++----------
  arch/arm/mach-orion5x/board-dt.c                   |  19 +-
  4 files changed, 181 insertions(+), 130 deletions(-)

create mode 100644 arch/arm/boot/dts/orion5x-mv88f5182.dtsi

arch/arm/boot/dts/orion5x-lacie-ethernet-disk-mini-v2.dts +1 -1

index 24f1ce7..d66d2fa 100644
@@ -10,7 +10,7 @@
 
  #include <dt-bindings/gpio/gpio.h>
  #include <dt-bindings/input/input.h>
-#include "orion5x.dtsi"
+#include "orion5x-mv88f5182.dtsi"
 
  / {
  	model = "LaCie Ethernet Disk mini V2";

arch/arm/boot/dts/orion5x-lacie-ethernet-disk-mini-v2.dts +0 -2

  	};
 
-	ocp at f1000000 {
-		serial at 12000 {

arch/arm/boot/dts/orion5x-lacie-ethernet-disk-mini-v2.dts +1 -0

+			};
  		};
  	};
 

arch/arm/boot/dts/orion5x-mv88f5182.dtsi new file +24 -0

new file mode 100644
index 0000000..ddfb4d1
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2014 Thomas Petazzoni <[email protected]>
+ *
+ * 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 "orion5x.dtsi"
+
+/ {
+	compatible = "marvell,orion5x-88f5182", "marvell,orion5x";
+
+	soc {
+		compatible = "marvell,orion5x-88f5182-mbus", "simple-bus";
+
+		internal-regs {
+			mbusc: mbus-controller at 20000 {
+				compatible = "marvell,mbus-controller";
+				reg = <0x20000 0x100>, <0x1500 0x20>;
+			};
+		};
+	};
+};

arch/arm/boot/dts/orion5x.dtsi +20 -33

index 2364e3d..31d46e6 100644
@@ -8,6 +8,8 @@
 
  #include "skeleton.dtsi"
 
+#define MBUS_ID(target,attributes) (((target) << 24) | ((attributes) << 16))
+
  / {
  	model = "Marvell Orion5x SoC";
  	compatible = "marvell,orion5x";

@@ -17,149 +19,154 @@
  		gpio0 = &gpio0;
  	};
 
-	ocp at f1000000 {
-		compatible = "simple-bus";
-		ranges = <0x00000000 0xf1000000 0x4000000
-		          0xf2200000 0xf2200000 0x0000800>;
-		#address-cells = <1>;
+	soc {
+		#address-cells = <2>;
  		#size-cells = <1>;
+		controller = <&mbusc>;
 
-		gpio0: gpio at 10100 {
-			compatible = "marvell,orion-gpio";
-			#gpio-cells = <2>;
-			gpio-controller;
-			reg = <0x10100 0x40>;
-			ngpios = <32>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-			interrupts = <6>, <7>, <8>, <9>;
-		};
-
-		spi at 10600 {
-			compatible = "marvell,orion-spi";
+		internal-regs {
+			compatible = "simple-bus";
  			#address-cells = <1>;
-			#size-cells = <0>;
-			cell-index = <0>;
-			reg = <0x10600 0x28>;
-			status = "disabled";
-		};
-
-		i2c at 11000 {
-			compatible = "marvell,mv64xxx-i2c";
-			reg = <0x11000 0x20>;
-			#address-cells = <1>;
-			#size-cells = <0>;
-			interrupts = <5>;
-			clock-frequency = <100000>;
-			status = "disabled";
-		};
+			#size-cells = <1>;
+			ranges = <0 MBUS_ID(0xf0, 0x01) 0 0x100000>;
+
+			gpio0: gpio at 10100 {
+				compatible = "marvell,orion-gpio";
+				#gpio-cells = <2>;
+				gpio-controller;
+				reg = <0x10100 0x40>;
+				ngpios = <32>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <6>, <7>, <8>, <9>;
+			};

arch/arm/boot/dts/orion5x.dtsi +7 -0

+				compatible = "marvell,orion-spi";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				cell-index = <0>;
+				reg = <0x10600 0x28>;
+				status = "disabled";
+			};

arch/arm/boot/dts/orion5x.dtsi +6 -0

+				compatible = "marvell,mv64xxx-i2c";
+				reg = <0x11000 0x20>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				interrupts = <5>;
+				clock-frequency = <100000>;

arch/arm/boot/dts/orion5x.dtsi +5 -6

+				status = "disabled";
+			};
 
-		ehci at 50000 {
-			compatible = "marvell,orion-ehci";
-			reg = <0x50000 0x1000>;
-			interrupts = <17>;
-			status = "disabled";
-		};
+			intc: interrupt-controller at 20200 {
+				compatible = "marvell,orion-intc";
+				interrupt-controller;

arch/arm/boot/dts/orion5x.dtsi +16 -14

+				compatible = "marvell,orion-xor";
+				reg = <0x60900 0x100
+				       0x60b00 0x100>;
+				status = "okay";
+
+				xor00 {
+				      interrupts = <30>;
+				      dmacap,memcpy;
+				      dmacap,xor;
+				};
+				xor01 {
+				      interrupts = <31>;
+				      dmacap,memcpy;
+				      dmacap,xor;
+				      dmacap,memset;
+				};
  			};
-		};
 
-		eth: ethernet-controller at 72000 {
-			compatible = "marvell,orion-eth";
-			#address-cells = <1>;
-			#size-cells = <0>;
-			reg = <0x72000 0x4000>;
-			marvell,tx-checksum-limit = <1600>;
-			status = "disabled";
-
-			ethernet-port at 0 {
-				compatible = "marvell,orion-eth-port";
-				reg = <0>;
-				/* overwrite MAC address in bootloader */
-				local-mac-address = [00 00 00 00 00 00];

arch/arm/boot/dts/orion5x.dtsi +10 -8

+					compatible = "marvell,orion-eth-port";
+					reg = <0>;
+					/* overwrite MAC address in bootloader */
+					local-mac-address = [00 00 00 00 00 00];
+					/* set phy-handle property in board file */
+				};
  			};
-		};
 
-		mdio: mdio-bus at 72004 {
-			compatible = "marvell,orion-mdio";
-			#address-cells = <1>;
-			#size-cells = <0>;
-			reg = <0x72004 0x84>;
-			interrupts = <22>;
-			status = "disabled";
+			mdio: mdio-bus at 72004 {
+				compatible = "marvell,orion-mdio";
+				#address-cells = <1>;
+				#size-cells = <0>;

arch/arm/boot/dts/orion5x.dtsi +5 -4

+				compatible = "marvell,orion-sata";
+				reg = <0x80000 0x5000>;
+				interrupts = <29>;
+				status = "disabled";
+			};
 
-		sata at 80000 {
-			compatible = "marvell,orion-sata";
-			reg = <0x80000 0x5000>;
-			interrupts = <29>;

arch/arm/boot/dts/orion5x.dtsi +2 -9

  			compatible = "marvell,orion-crypto";
-			reg = <0x90000 0x10000>,
-			      <0xf2200000 0x800>;
+			reg = <MBUS_ID(0xf0, 0x01) 0x90000 0x10000>,
+			      <MBUS_ID(0x09, 0x00) 0x0 0x800>;
  			reg-names = "regs", "sram";
  			interrupts = <28>;
  			status = "okay";
  		};
-
-		ehci at a0000 {
-			compatible = "marvell,orion-ehci";
-			reg = <0xa0000 0x1000>;
-			interrupts = <12>;
-			status = "disabled";
-		};
  	};
  };

arch/arm/mach-orion5x/board-dt.c +17 -2

index c134a82..f6a388a 100644
@@ -15,10 +15,14 @@
  #include <linux/of.h>
  #include <linux/of_platform.h>
  #include <linux/cpu.h>
+#include <linux/mbus.h>
  #include <asm/system_misc.h>
  #include <asm/mach/arch.h>
+#include <asm/mach/map.h>
  #include <mach/orion5x.h>
+#include <mach/bridge-regs.h>
  #include <plat/irq.h>
+#include <plat/time.h>
  #include "common.h"
 
  static struct of_dev_auxdata orion5x_auxdata_lookup[] __initdata = {

@@ -31,6 +35,17 @@ static struct of_dev_auxdata orion5x_auxdata_lookup[] __initdata = {
  	{},
  };
 
+static void orion5x_dt_init_early(void)
+{
+	orion_time_set_base(TIMER_VIRT_BASE);
+}
+
+static void orion5x_dt_init_time(void)
+{
+	orion5x_timer_init();
+	BUG_ON(mvebu_mbus_dt_init());
+}
+
  static void __init orion5x_dt_init(void)
  {
  	char *dev_name;

@@ -71,9 +86,9 @@ static const char *orion5x_dt_compat[] = {
  DT_MACHINE_START(ORION5X_DT, "Marvell Orion5x (Flattened Device Tree)")
  	/* Maintainer: Thomas Petazzoni <[email protected]> */
  	.map_io		= orion5x_map_io,
-	.init_early	= orion5x_init_early,
+	.init_early	= orion5x_dt_init_early,
  	.init_irq	= orion_dt_init_irq,
-	.init_time	= orion5x_timer_init,
+	.init_time	= orion5x_dt_init_time,
  	.init_machine	= orion5x_dt_init,
  	.restart	= orion5x_restart,
  	.dt_compat	= orion5x_dt_compat,

For some reason, the Ethernet interrupt was missing in the Orion5x Device Tree definition.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  arch/arm/boot/dts/orion5x.dtsi | 1 +
  1 file changed, 1 insertion(+)

arch/arm/boot/dts/orion5x.dtsi +1 -0

index 31d46e6..ac2ec8f 100644
@@ -128,6 +128,7 @@
  				ethernet-port at 0 {
  					compatible = "marvell,orion-eth-port";
  					reg = <0>;
+					interrupts = <21>;
  					/* overwrite MAC address in bootloader */
  					local-mac-address = [00 00 00 00 00 00];
  					/* set phy-handle property in board file */

This commit moves the Orion5x platforms using the Device Tree to use the recently introduced clock driver for Orion5x. To achieve that, it:

* Adds the necessary DT description of the clock.

* Selects ORION_CLK to enable the compilation of the clock driver.

* Call of_clk_init() instead of the Orion5x-specific clock initialization function.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  arch/arm/boot/dts/orion5x-mv88f5182.dtsi | 6 ++++++
  arch/arm/mach-orion5x/Kconfig            | 1 +
  arch/arm/mach-orion5x/board-dt.c         | 5 ++---
  3 files changed, 9 insertions(+), 3 deletions(-)

arch/arm/boot/dts/orion5x-mv88f5182.dtsi +6 -0

index ddfb4d1..a56f9c8 100644
@@ -15,6 +15,12 @@
  		compatible = "marvell,orion5x-88f5182-mbus", "simple-bus";
 
  		internal-regs {
+			core_clk: core-clocks at 10030 {
+				compatible = "marvell,mv88f5182-core-clock";
+				reg = <0x10010 0x4>;
+				#clock-cells = <1>;
+			};
+
  			mbusc: mbus-controller at 20000 {
  				compatible = "marvell,mbus-controller";
  				reg = <0x20000 0x100>, <0x1500 0x20>;

arch/arm/mach-orion5x/Kconfig +1 -0

index 14f2cae..4f51132 100644
@@ -5,6 +5,7 @@ menu "Orion Implementations"
  config ARCH_ORION5X_DT
  	bool "Marvell Orion5x Flattened Device Tree"
  	select USE_OF
+	select ORION_CLK
  	help
  	  Say 'Y' here if you want your kernel to support the
  	  Marvell Orion5x using flattened device tree.

arch/arm/mach-orion5x/board-dt.c +2 -3

index f6a388a..c489661 100644
@@ -16,6 +16,7 @@
  #include <linux/of_platform.h>
  #include <linux/cpu.h>
  #include <linux/mbus.h>
+#include <linux/clk-provider.h>
  #include <asm/system_misc.h>
  #include <asm/mach/arch.h>
  #include <asm/mach/map.h>

@@ -43,6 +44,7 @@ static void orion5x_dt_init_early(void)
  static void orion5x_dt_init_time(void)
  {
  	orion5x_timer_init();
+	of_clk_init(NULL);
  	BUG_ON(mvebu_mbus_dt_init());
  }
 

@@ -59,9 +61,6 @@ static void __init orion5x_dt_init(void)
  	 */
  	orion5x_setup_wins();
 
-	/* Setup root of clk tree */
-	clk_init();
-
  	/*
  	 * Don't issue "Wait for Interrupt" instruction if we are
  	 * running on D0 5281 silicon.

Moving to the Device Tree implies having CONFIG_MULTI_IRQ_HANDLER enabled, even for non-DT platforms (if we want both DT and non-DT platforms to be supported in a single kernel).

However, the common CONFIG_MULTI_IRQ_HANDLER handler for non-DT platforms in plat-orion/irq.c doesn't match the needs of Orion5x. Also, it doesn't make much sense for orion_irq_init() to register the multi-IRQ handler: orion_irq_init() is called once for each IRQ cause/mask tuple, while the multi-IRQ handler only needs to be registered once.

To solve this problem, we move the multi-IRQ handle in per-platform
code: mach-kirkwood/irq.c and mach-dove/irq.c. The Orion5x variant will be introduced in a followup commit. Of course, this code will ultimately be completely removed once all boards are converted to the Device Tree.

Signed-off-by Thomas Petazzoni <[email protected]>

---
  arch/arm/mach-dove/irq.c     | 36 +++++++++++++++++++++++++++++++++++
  arch/arm/mach-kirkwood/irq.c | 36 +++++++++++++++++++++++++++++++++++
  arch/arm/plat-orion/irq.c    | 45 --------------------------------------------
  3 files changed, 72 insertions(+), 45 deletions(-)

arch/arm/mach-dove/irq.c +12 -0

index bc4344a..4a5a7ae 100644
@@ -108,6 +108,38 @@ static int __initdata gpio2_irqs[4] = {
  	0,
  };
 
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+/*
+ * Compiling with both non-DT and DT support enabled, will
+ * break asm irq handler used by non-DT boards. Therefore,
+ * we provide a C-style irq handler even for non-DT boards,
+ * if MULTI_IRQ_HANDLER is set.
+ */
+
+static void __iomem *dove_irq_base = IRQ_VIRT_BASE;
+
+static asmlinkage void
+__exception_irq_entry dove_legacy_handle_irq(struct pt_regs *regs)

arch/arm/mach-dove/irq.c +7 -0

+}
+#endif
+
  void __init dove_init_irq(void)
  {
  	int i;

@@ -115,6 +147,10 @@ void __init dove_init_irq(void)
  	orion_irq_init(0, IRQ_VIRT_BASE + IRQ_MASK_LOW_OFF);
  	orion_irq_init(32, IRQ_VIRT_BASE + IRQ_MASK_HIGH_OFF);
 
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+	set_handle_irq(dove_legacy_handle_irq);
+#endif
+
  	/*
  	 * Initialize gpiolib for GPIOs 0-71.
  	 */

arch/arm/mach-kirkwood/irq.c +36 -0

index 2a97a2e..c9dd860 100644
@@ -30,11 +30,47 @@ static int __initdata gpio1_irqs[4] = {
  	0,
  };
 
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+/*
+ * Compiling with both non-DT and DT support enabled, will
+ * break asm irq handler used by non-DT boards. Therefore,
+ * we provide a C-style irq handler even for non-DT boards,
+ * if MULTI_IRQ_HANDLER is set.
+ */
+
+static void __iomem *kirkwood_irq_base = IRQ_VIRT_BASE;
+
+asmlinkage void
+__exception_irq_entry kirkwood_legacy_handle_irq(struct pt_regs *regs)
+{
+	u32 stat;
+
+	stat = readl_relaxed(kirkwood_irq_base + IRQ_CAUSE_LOW_OFF);
+	stat &= readl_relaxed(kirkwood_irq_base + IRQ_MASK_LOW_OFF);
+	if (stat) {
+		unsigned int hwirq = __fls(stat);
+		handle_IRQ(hwirq, regs);
+		return;
+	}
+	stat = readl_relaxed(kirkwood_irq_base + IRQ_CAUSE_HIGH_OFF);
+	stat &= readl_relaxed(kirkwood_irq_base + IRQ_MASK_HIGH_OFF);
+	if (stat) {
+		unsigned int hwirq = 32 + __fls(stat);
+		handle_IRQ(hwirq, regs);
+		return;
+	}
+}
+#endif
+
  void __init kirkwood_init_irq(void)
  {
  	orion_irq_init(0, IRQ_VIRT_BASE + IRQ_MASK_LOW_OFF);
  	orion_irq_init(32, IRQ_VIRT_BASE + IRQ_MASK_HIGH_OFF);
 
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+	set_handle_irq(kirkwood_legacy_handle_irq);
+#endif
+
  	/*
  	 * Initialize gpiolib for GPIOs 0-49.
  	 */

arch/arm/plat-orion/irq.c +0 -45

index 807df14..27ec18b 100644
@@ -20,47 +20,6 @@
  #include <plat/orion-gpio.h>
  #include <mach/bridge-regs.h>
 
-#ifdef CONFIG_MULTI_IRQ_HANDLER
-/*
- * Compiling with both non-DT and DT support enabled, will
- * break asm irq handler used by non-DT boards. Therefore,
- * we provide a C-style irq handler even for non-DT boards,
- * if MULTI_IRQ_HANDLER is set.
- *
- * Notes:
- * - this is prepared for Kirkwood and Dove only, update
- *   accordingly if you add Orion5x or MV78x00.
- * - Orion5x uses different macro names and has only one
- *   set of CAUSE/MASK registers.
- * - MV78x00 uses the same macro names but has a third
- *   set of CAUSE/MASK registers.
- *
- */
-
-static void __iomem *orion_irq_base = IRQ_VIRT_BASE;
-
-asmlinkage void
-__exception_irq_entry orion_legacy_handle_irq(struct pt_regs *regs)
-{
-	u32 stat;
-
-	stat = readl_relaxed(orion_irq_base + IRQ_CAUSE_LOW_OFF);
-	stat &= readl_relaxed(orion_irq_base + IRQ_MASK_LOW_OFF);
-	if (stat) {
-		unsigned int hwirq = __fls(stat);
-		handle_IRQ(hwirq, regs);
-		return;
-	}
-	stat = readl_relaxed(orion_irq_base + IRQ_CAUSE_HIGH_OFF);
-	stat &= readl_relaxed(orion_irq_base + IRQ_MASK_HIGH_OFF);
-	if (stat) {
-		unsigned int hwirq = 32 + __fls(stat);
-		handle_IRQ(hwirq, regs);
-		return;
-	}
-}
-#endif
-
  void __init orion_irq_init(unsigned int irq_start, void __iomem *maskaddr)
  {
  	struct irq_chip_generic *gc;

@@ -78,10 +37,6 @@ void __init orion_irq_init(unsigned int irq_start, void __iomem *maskaddr)
  	ct->chip.irq_unmask = irq_gc_mask_set_bit;
  	irq_setup_generic_chip(gc, IRQ_MSK(32), IRQ_GC_INIT_MASK_CACHE,
  			       IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
-
-#ifdef CONFIG_MULTI_IRQ_HANDLER
-	set_handle_irq(orion_legacy_handle_irq);
-#endif
  }
 
  #ifdef CONFIG_OF

arch/arm/mach-dove/irq.c static int __initdata gpio2_irqs[4] = { · line 132

Quoted 12 lines

arch/arm/mach-dove/irq.c +12 -0

+static asmlinkage void
+__exception_irq_entry dove_legacy_handle_irq(struct pt_regs *regs)
+{
+	u32 stat;
+
+	stat = readl_relaxed(dove_irq_base + IRQ_CAUSE_LOW_OFF);
+	stat &= readl_relaxed(dove_irq_base + IRQ_MASK_LOW_OFF);
+	if (stat) {
+		unsigned int hwirq = __fls(stat);
+		handle_IRQ(hwirq, regs);
+		return;
+	}

I remember we talked about it already, but IMHO copying the multi-irq handler to mach-{kirkwood,dove} isn't necessary. The only situation we need this is when you compile _one_ mach-{kirkwood,dove,orion5x} with both DT and non-DT.

So, I think it is fine to just add

#if !defined(CONFIG_MACH_ORION5X)

arch/arm/mach-dove/irq.c static int __initdata gpio2_irqs[4] = { · line 139

Quoted 7 lines

arch/arm/mach-dove/irq.c +7 -0

+	stat = readl_relaxed(dove_irq_base + IRQ_CAUSE_HIGH_OFF);
+	stat &= readl_relaxed(dove_irq_base + IRQ_MASK_HIGH_OFF);
+	if (stat) {
+		unsigned int hwirq = 32 + __fls(stat);
+		handle_IRQ(hwirq, regs);
+		return;
+	}

#endif

in the original handler?

Dear Sebastian Hesselbarth,

Quoted 30 lines · Sebastian Hesselbarth

arch/arm/mach-dove/irq.c static int __initdata gpio2_irqs[4] = { · line 132

Quoted 12 lines

arch/arm/mach-dove/irq.c +12 -0

+static asmlinkage void
+__exception_irq_entry dove_legacy_handle_irq(struct pt_regs *regs)
+{
+	u32 stat;
+
+	stat = readl_relaxed(dove_irq_base + IRQ_CAUSE_LOW_OFF);
+	stat &= readl_relaxed(dove_irq_base + IRQ_MASK_LOW_OFF);
+	if (stat) {
+		unsigned int hwirq = __fls(stat);
+		handle_IRQ(hwirq, regs);
+		return;
+	}

I remember we talked about it already, but IMHO copying the multi-irq handler to mach-{kirkwood,dove} isn't necessary. The only situation we need this is when you compile _one_ mach-{kirkwood,dove,orion5x} with both DT and non-DT.

So, I think it is fine to just add

#if !defined(CONFIG_MACH_ORION5X)

arch/arm/mach-dove/irq.c static int __initdata gpio2_irqs[4] = { · line 139

Quoted 7 lines

arch/arm/mach-dove/irq.c +7 -0

+	stat = readl_relaxed(dove_irq_base + IRQ_CAUSE_HIGH_OFF);
+	stat &= readl_relaxed(dove_irq_base + IRQ_MASK_HIGH_OFF);
+	if (stat) {
+		unsigned int hwirq = 32 + __fls(stat);
+		handle_IRQ(hwirq, regs);
+		return;
+	}

#endif

Quoted 1 line

in the original handler?

Nope, it doesn't work, because mach-orion5x doesn't define IRQ_VIRT_BASE, IRQ_CAUSE_LOW_OFF and IRQ_MASK_LOW_OFF, so the common handler does not build, even after compiling-out the part you mention here.

Of course, Orion5x code can be changed to define those values, but I found it was not worth the effort, and Arnd Bergmann during an IRC discussion, also suggested to move the handle_irq() function to each individual mach-<foo> directory. And it actually makes sense, because it's only the same between Kirkwood and Dove: Orion5x and mv78xx0 need different implementations.

Also, this approach goes towards the goal of reducing plat-orion/ code. Having the code closer to each SoC is going to make it easier to progressively get rid of it I believe.

Quoted 3 lines

Also, this approach goes towards the goal of reducing plat-orion/ code. Having the code closer to each SoC is going to make it easier to progressively get rid of it I believe.

Ok, I agree. But IIRC the multi-irq handler is only needed, if you compile both DT and non-DT in one kernel, right?

Dove DT just left mach-dove, so there is no way you can compile both. For the sake of simplicity, I'd even agree on adding it now and remove it later again. But at least for Dove, the move is bogus.

Dear Sebastian Hesselbarth,

Quoted 6 lines

Quoted 3 lines

Also, this approach goes towards the goal of reducing plat-orion/ code. Having the code closer to each SoC is going to make it easier to progressively get rid of it I believe.

Ok, I agree. But IIRC the multi-irq handler is only needed, if you compile both DT and non-DT in one kernel, right?

Correct.

Quoted 3 lines

Dove DT just left mach-dove, so there is no way you can compile both. For the sake of simplicity, I'd even agree on adding it now and remove it later again. But at least for Dove, the move is bogus.

I think there's a lot of possible following clean up to be made. But we have to define some limits to the current patch series, otherwise it's going to grow to a 50+ patches monster that will be very hard to merge. I clearly agree some of things are not in their final ideal state, but I believe, overall, this patch series for Orion5x is pushing things in the right direction, no?