[PATCH v3 0/4] Initial DT support for MSM8660

[PATCH v3 0/4] Initial DT support for MSM8660

[David Brown]

This series adds initial support for booting the MSM8660 via
devicetree.  The target boots with a serial console, and no platform
data.

In order to test this with a devicetree boot, it is necessary to
include patches allowing the devicetree to be appended to the kernel,
and merge ATAG values into the DT.

Once these patches reach mainline, I intend to remove the non-DT boot
for this target.

History:
  v2 - Consistent name for msm-uart devices.
  v3 - Use separate names for hsuart and lsuart, and common name.
       Clean up io mapping for FPGA registers.

David Brown (4):
  msm_serial: Use relative resources for iomem
  msm_serial: Add devicetree support
  ARM: msm: Add devicetree support for msm8660-surf
  ARM: msm: Describe MSM 8660 SURF FPGA registers in DT

 .../devicetree/bindings/tty/serial/msm_serial.txt  |   22 +++++
 arch/arm/boot/dts/msm8660-surf.dts                 |   29 ++++++
 arch/arm/mach-msm/board-msm8x60.c                  |   98 ++++++++++++++++++--
 drivers/tty/serial/msm_serial.c                    |   30 ++++--
 4 files changed, 162 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/msm_serial.txt
 create mode 100644 arch/arm/boot/dts/msm8660-surf.dts

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH v3 1/4] msm_serial: Use relative resources for iomem

[David Brown]

Device tree iomem resources are only accessible by index, and not by
name.  The msm_serial devices always have either 1 or 2 iomem
resources, that are always in the same order.  Convert the
platform_get_resource_byname into just platform_get_resource to
facilitate device tree conversion.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/tty/serial/msm_serial.c |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index e6ba838..d9863b2 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -589,9 +589,8 @@ static void msm_release_port(struct uart_port *port)
 		iowrite32(GSBI_PROTOCOL_IDLE, msm_port->gsbi_base +
 			  GSBI_CONTROL);
 
-		gsbi_resource = platform_get_resource_byname(pdev,
-							     IORESOURCE_MEM,
-							     "gsbi_resource");
+		gsbi_resource = platform_get_resource(pdev,
+							IORESOURCE_MEM, 1);
 
 		if (unlikely(!gsbi_resource))
 			return;
@@ -612,8 +611,7 @@ static int msm_request_port(struct uart_port *port)
 	resource_size_t size;
 	int ret;
 
-	uart_resource = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						     "uart_resource");
+	uart_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (unlikely(!uart_resource))
 		return -ENXIO;
 
@@ -628,8 +626,7 @@ static int msm_request_port(struct uart_port *port)
 		goto fail_release_port;
 	}
 
-	gsbi_resource = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						     "gsbi_resource");
+	gsbi_resource = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	/* Is this a GSBI-based port? */
 	if (gsbi_resource) {
 		size = resource_size(gsbi_resource);
@@ -875,7 +872,7 @@ static int __init msm_serial_probe(struct platform_device *pdev)
 	port->dev = &pdev->dev;
 	msm_port = UART_TO_MSM(port);
 
-	if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "gsbi_resource"))
+	if (platform_get_resource(pdev, IORESOURCE_MEM, 1))
 		msm_port->is_uartdm = 1;
 	else
 		msm_port->is_uartdm = 0;
@@ -899,8 +896,7 @@ static int __init msm_serial_probe(struct platform_device *pdev)
 	printk(KERN_INFO "uartclk = %d\n", port->uartclk);
 
 
-	resource = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						     "uart_resource");
+	resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (unlikely(!resource))
 		return -ENXIO;
 	port->mapbase = resource->start;
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH v3 2/4] msm_serial: Add devicetree support

[David Brown]

Add devicetree support to the msm_serial driver.  Clocks are still
queried by direct name from the driver until device tree clock support
is implemented.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 .../devicetree/bindings/tty/serial/msm_serial.txt  |   27 ++++++++++++++++++++
 drivers/tty/serial/msm_serial.c                    |   14 ++++++++++
 2 files changed, 41 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/msm_serial.txt

diff --git a/Documentation/devicetree/bindings/tty/serial/msm_serial.txt b/Documentation/devicetree/bindings/tty/serial/msm_serial.txt
new file mode 100644
index 0000000..aef383e
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/msm_serial.txt
@@ -0,0 +1,27 @@
+* Qualcomm MSM UART
+
+Required properties:
+- compatible :
+	- "qcom,msm-uart", and one of "qcom,msm-hsuart" or
+	  "qcom,msm-lsuart".
+- reg : offset and length of the register set for the device
+	for the hsuart operating in compatible mode, there should be a
+	second pair describing the gsbi registers.
+- interrupts : should contain the uart interrupt.
+
+There are two different UART blocks used in MSM devices,
+"qcom,msm-hsuart" and "qcom,msm-lsuart".  The msm-serial driver is
+able to handle both of these, and matches against the "qcom,msm-uart"
+as the compatibility.
+
+The registers for the "qcom,msm-hsuart" device need to specify both
+register blocks, even for the common driver.
+
+Example:
+
+	uart at 19c400000 {
+		compatible = "qcom,msm-hsuart", "qcom,msm-uart";
+		reg = <0x19c40000 0x1000>,
+		      <0x19c00000 0x1000>;
+		interrupts = <195>;
+	};
diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index d9863b2..7767902 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -19,6 +19,7 @@
 # define SUPPORT_SYSRQ
 #endif
 
+#include <linux/atomic.h>
 #include <linux/hrtimer.h>
 #include <linux/module.h>
 #include <linux/io.h>
@@ -33,6 +34,8 @@
 #include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include "msm_serial.h"
 
@@ -856,6 +859,8 @@ static struct uart_driver msm_uart_driver = {
 	.cons = MSM_CONSOLE,
 };
 
+static atomic_t msm_uart_next_id = ATOMIC_INIT(0);
+
 static int __init msm_serial_probe(struct platform_device *pdev)
 {
 	struct msm_port *msm_port;
@@ -863,6 +868,9 @@ static int __init msm_serial_probe(struct platform_device *pdev)
 	struct uart_port *port;
 	int irq;
 
+	if (pdev->id == -1)
+		pdev->id = atomic_inc_return(&msm_uart_next_id) - 1;
+
 	if (unlikely(pdev->id < 0 || pdev->id >= UART_NR))
 		return -ENXIO;
 
@@ -920,11 +928,17 @@ static int __devexit msm_serial_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id msm_match_table[] = {
+	{ .compatible = "qcom,msm-uart" },
+	{}
+};
+
 static struct platform_driver msm_platform_driver = {
 	.remove = msm_serial_remove,
 	.driver = {
 		.name = "msm_serial",
 		.owner = THIS_MODULE,
+		.of_match_table = msm_match_table,
 	},
 };
 
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH v3 2/4] msm_serial: Add devicetree support

[Arnd Bergmann]

On Thursday 18 August 2011, David Brown wrote:
> Add devicetree support to the msm_serial driver.  Clocks are still
> queried by direct name from the driver until device tree clock support
> is implemented.
> 
> Signed-off-by: David Brown <davidb@codeaurora.org>

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v3 3/4] ARM: msm: Add devicetree support for msm8660-surf

[David Brown]

Adds support for booting via device tree with a simple serial console.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 arch/arm/boot/dts/msm8660-surf.dts |   24 +++++++++++++++
 arch/arm/mach-msm/board-msm8x60.c  |   58 +++++++++++++++++++++++++++++++----
 2 files changed, 75 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm/boot/dts/msm8660-surf.dts

diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
new file mode 100644
index 0000000..15ded0d
--- /dev/null
+++ b/arch/arm/boot/dts/msm8660-surf.dts
@@ -0,0 +1,24 @@
+/dts-v1/;
+
+/include/ "skeleton.dtsi"
+
+/ {
+	model = "Qualcomm MSM8660 SURF";
+	compatible = "qcom,msm8660-surf", "qcom,msm8660";
+	interrupt-parent = <&intc>;
+
+	intc: interrupt-controller at 02080000 {
+		compatible = "qcom,msm-8660-qgic";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		reg = < 0x02080000 0x1000 >,
+		      < 0x02081000 0x1000 >;
+	};
+
+	serial at 19c400000 {
+		compatible = "qcom,msm-hsuart", "qcom,msm-uart";
+		reg = <0x19c40000 0x1000>,
+		      <0x19c00000 0x1000>;
+		interrupts = <195>;
+	};
+};
diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
index 1163b6f..10fa8f6 100644
--- a/arch/arm/mach-msm/board-msm8x60.c
+++ b/arch/arm/mach-msm/board-msm8x60.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+/* Copyright (c) 2010, 2011, Code Aurora Forum. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 and
@@ -8,18 +8,16 @@
  * 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, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- *
  */
 
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -64,6 +62,41 @@ static void __init msm8x60_init(void)
 {
 }
 
+#ifdef CONFIG_OF
+static struct of_dev_auxdata msm_auxdata_lookup[] __initdata = {
+	{}
+};
+
+static struct of_device_id msm_dt_gic_match[] __initdata = {
+	{ .compatible = "qcom,msm-8660-qgic", },
+	{}
+};
+
+static void __init msm8x60_dt_init(void)
+{
+	struct device_node *node;
+
+	node = of_find_matching_node_by_address(NULL, msm_dt_gic_match,
+			MSM8X60_QGIC_DIST_PHYS);
+	if (node)
+		irq_domain_add_simple(node, GIC_SPI_START);
+
+	if (of_machine_is_compatible("qcom,msm8660-surf")) {
+		printk(KERN_INFO "Init surf UART registers\n");
+		msm8x60_init_uart12dm();
+	}
+
+	of_platform_populate(NULL, of_default_bus_match_table,
+			msm_auxdata_lookup, NULL);
+}
+
+static const char *msm8x60_fluid_match[] __initdata = {
+	"qcom,msm8660-fluid",
+	"qcom,msm8660-surf",
+	NULL
+};
+#endif /* CONFIG_OF */
+
 MACHINE_START(MSM8X60_RUMI3, "QCT MSM8X60 RUMI3")
 	.map_io = msm8x60_map_io,
 	.init_irq = msm8x60_init_irq,
@@ -91,3 +124,14 @@ MACHINE_START(MSM8X60_FFA, "QCT MSM8X60 FFA")
 	.init_machine = msm8x60_init,
 	.timer = &msm_timer,
 MACHINE_END
+
+#ifdef CONFIG_OF
+/* TODO: General device tree support for all MSM. */
+DT_MACHINE_START(MSM_DT, "Qualcomm MSM (Flattened Device Tree)")
+	.map_io = msm8x60_map_io,
+	.init_irq = msm8x60_init_irq,
+	.init_machine = msm8x60_dt_init,
+	.timer = &msm_timer,
+	.dt_compat = msm8x60_fluid_match,
+MACHINE_END
+#endif /* CONFIG_OF */
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH v3 3/4] ARM: msm: Add devicetree support for msm8660-surf

[Arnd Bergmann]

On Thursday 18 August 2011, David Brown wrote:
> Adds support for booting via device tree with a simple serial console.
> 
> Signed-off-by: David Brown <davidb@codeaurora.org>

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v3 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT

[David Brown]

The MSM 8660 SURF development board contains a register-accessible
FPGA.  By default, this FPGA configures the first UART in output-only
mode.  On this target, reconfigure this FPGA to enable the UART to be
bidirectional.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 arch/arm/boot/dts/msm8660-surf.dts |    5 +++
 arch/arm/mach-msm/board-msm8x60.c  |   49 +++++++++++++++++++++++++++++++++---
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
index 15ded0d..3591c94 100644
--- a/arch/arm/boot/dts/msm8660-surf.dts
+++ b/arch/arm/boot/dts/msm8660-surf.dts
@@ -21,4 +21,9 @@
 		      <0x19c00000 0x1000>;
 		interrupts = <195>;
 	};
+
+	qcom,fpga at 1d000000 {
+		compatible = "qcom,msm8660-surf-fpga";
+		reg = < 0x1d000000 0x1000 >;
+	};
 };
diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
index 10fa8f6..f2dacfe 100644
--- a/arch/arm/mach-msm/board-msm8x60.c
+++ b/arch/arm/mach-msm/board-msm8x60.c
@@ -58,8 +58,52 @@ static void __init msm8x60_init_irq(void)
 	}
 }
 
+static void __init msm8660_surf_fpga_init(void __iomem *fpga_mem)
+{
+	/* Advanced mode */
+	writew(0xFFFF, fpga_mem + 0x15C);
+	/* FPGA_UART_SEL */
+	writew(0, fpga_mem + 0x172);
+	/* FPGA_GPIO_CONFIG_117 */
+	writew(1, fpga_mem + 0xEA);
+	/* FPGA_GPIO_CONFIG_118 */
+	writew(1, fpga_mem + 0xEC);
+	dmb();
+}
+
+static void __init msm8660_surf_fpga_init_platform(void)
+{
+	/* 0x1D000000 now belongs to EBI2:CS3 i.e. USB ISP Controller */
+	void __iomem *fpga_mem = ioremap(0x1D000000, SZ_4K);
+	msm8660_surf_fpga_init(fpga_mem);
+	iounmap(fpga_mem);
+}
+
+#ifdef CONFIG_OF
+static void __init msm8660_surf_fpga_init_dt(void)
+{
+	struct device_node *node;
+	void __iomem *fpga_mem;
+
+	node = of_find_compatible_node(NULL, NULL, "qcom,msm8660-surf-fpga");
+	if (!node)
+		return;
+
+	fpga_mem = of_iomap(node, 0);
+	of_node_put(node);
+	if (!fpga_mem) {
+		printk(KERN_ERR "%s: Can't map fpga registers\n", __func__);
+		return;
+	}
+
+	msm8660_surf_fpga_init(fpga_mem);
+	iounmap(fpga_mem);
+}
+#endif
+
 static void __init msm8x60_init(void)
 {
+	msm8660_surf_fpga_init_platform();
 }
 
 #ifdef CONFIG_OF
@@ -81,10 +125,7 @@ static void __init msm8x60_dt_init(void)
 	if (node)
 		irq_domain_add_simple(node, GIC_SPI_START);
 
-	if (of_machine_is_compatible("qcom,msm8660-surf")) {
-		printk(KERN_INFO "Init surf UART registers\n");
-		msm8x60_init_uart12dm();
-	}
+	msm8660_surf_fpga_init_dt();
 
 	of_platform_populate(NULL, of_default_bus_match_table,
 			msm_auxdata_lookup, NULL);
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH v3 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT

[Arnd Bergmann]

On Thursday 18 August 2011, David Brown wrote:
> +static void __init msm8660_surf_fpga_init(void __iomem *fpga_mem)
> +{
> +       /* Advanced mode */
> +       writew(0xFFFF, fpga_mem + 0x15C);
> +       /* FPGA_UART_SEL */
> +       writew(0, fpga_mem + 0x172);
> +       /* FPGA_GPIO_CONFIG_117 */
> +       writew(1, fpga_mem + 0xEA);
> +       /* FPGA_GPIO_CONFIG_118 */
> +       writew(1, fpga_mem + 0xEC);
> +       dmb();
> +}

Does the dmb() do the right thing here? It seems strange to combine a strictly
ordered I/O instruction with another ordering instruction, and I think it would
be better to use writew_relaxed for the first one, followed by a 'wmb()'.

> +#ifdef CONFIG_OF
> +static void __init msm8660_surf_fpga_init_dt(void)
> +{
> +       struct device_node *node;
> +       void __iomem *fpga_mem;
> +
> +       node = of_find_compatible_node(NULL, NULL, "qcom,msm8660-surf-fpga");
> +       if (!node)
> +               return;
> +
> +       fpga_mem = of_iomap(node, 0);
> +       of_node_put(node);
> +       if (!fpga_mem) {
> +               printk(KERN_ERR "%s: Can't map fpga registers\n", __func__);
> +               return;
> +       }
> +
> +       msm8660_surf_fpga_init(fpga_mem);
> +       iounmap(fpga_mem);
> +}
> +#endif

Is the serial port connected through the FPGA or just configured by it?

In the former case, I think it would be better to make this a proper device driver that
binds to the qcom,msm8660-surf-fpga device, configures it and then creates the
platform_devices for the child nodes (the serial port, possibly others) by calling
of_platform_bus_probe.

	Arnd

[PATCH v3 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT

[David Brown]

On Thu, Aug 25, 2011 at 01:27:12PM +0200, Arnd Bergmann wrote:
> On Thursday 18 August 2011, David Brown wrote:
> > +static void __init msm8660_surf_fpga_init(void __iomem *fpga_mem)
> > +{
> > +       /* Advanced mode */
> > +       writew(0xFFFF, fpga_mem + 0x15C);
> > +       /* FPGA_UART_SEL */
> > +       writew(0, fpga_mem + 0x172);
> > +       /* FPGA_GPIO_CONFIG_117 */
> > +       writew(1, fpga_mem + 0xEA);
> > +       /* FPGA_GPIO_CONFIG_118 */
> > +       writew(1, fpga_mem + 0xEC);
> > +       dmb();
> > +}
> 
> Does the dmb() do the right thing here? It seems strange to combine a strictly
> ordered I/O instruction with another ordering instruction, and I think it would
> be better to use writew_relaxed for the first one, followed by a 'wmb()'.

I guess I didn't really think about that, I just kind of kept the
code.  I'll ask Stepan why he did it that way, and come up with a
cleaner solution.

> > +#ifdef CONFIG_OF
> > +static void __init msm8660_surf_fpga_init_dt(void)
> > +{
> > +       struct device_node *node;
> > +       void __iomem *fpga_mem;
> > +
> > +       node = of_find_compatible_node(NULL, NULL, "qcom,msm8660-surf-fpga");
> > +       if (!node)
> > +               return;
> > +
> > +       fpga_mem = of_iomap(node, 0);
> > +       of_node_put(node);
> > +       if (!fpga_mem) {
> > +               printk(KERN_ERR "%s: Can't map fpga registers\n", __func__);
> > +               return;
> > +       }
> > +
> > +       msm8660_surf_fpga_init(fpga_mem);
> > +       iounmap(fpga_mem);
> > +}
> > +#endif
> 
> Is the serial port connected through the FPGA or just configured by it?

The FPGA controls how the UART pins are connected on the development
board.  The serial port itself is in the MSM, not the FPGA, and on
other dev boards this isn't needed for the serial port to work.

> In the former case, I think it would be better to make this a proper
> device driver that binds to the qcom,msm8660-surf-fpga device,
> configures it and then creates the platform_devices for the child
> nodes (the serial port, possibly others) by calling
> of_platform_bus_probe.

It might make sense to have the FPGA as a driver.  I believe it was
done early to make sure that the pins were configured correctly before
the serial driver came up.  As far as I can tell, the output pin is
already configured correctly, so this can actually happen completely
independently, since early usage of the UART is really only for
console messages.

I don't think it makes sense for the serial to be a child node, this
FPGA configuration is more along the lines of pinmux.  Most
configurations of this SOC don't have or need this fpga.

So, if I made it a separate driver, where would it go?  Since this
board still has platform device support, I suspect the platform data
needed to describe this device would end up being larger than the
driver itself.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH v3 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT

[Arnd Bergmann]

On Thursday 25 August 2011, David Brown wrote:
> On Thu, Aug 25, 2011 at 01:27:12PM +0200, Arnd Bergmann wrote:
> > On Thursday 18 August 2011, David Brown wrote:
> > > +static void __init msm8660_surf_fpga_init(void __iomem *fpga_mem)
> > > +{
> > > +       /* Advanced mode */
> > > +       writew(0xFFFF, fpga_mem + 0x15C);
> > > +       /* FPGA_UART_SEL */
> > > +       writew(0, fpga_mem + 0x172);
> > > +       /* FPGA_GPIO_CONFIG_117 */
> > > +       writew(1, fpga_mem + 0xEA);
> > > +       /* FPGA_GPIO_CONFIG_118 */
> > > +       writew(1, fpga_mem + 0xEC);
> > > +       dmb();
> > > +}
> > 
> > Does the dmb() do the right thing here? It seems strange to combine a strictly
> > ordered I/O instruction with another ordering instruction, and I think it would
> > be better to use writew_relaxed for the first one, followed by a 'wmb()'.
> 
> I guess I didn't really think about that, I just kind of kept the
> code.  I'll ask Stepan why he did it that way, and come up with a
> cleaner solution.

Yes, no worries. I saw later that the code already exists in similar form,
so it is not urgent to change.

> > > +#ifdef CONFIG_OF
> > > +static void __init msm8660_surf_fpga_init_dt(void)
> > > +{
> > > +       struct device_node *node;
> > > +       void __iomem *fpga_mem;
> > > +
> > > +       node = of_find_compatible_node(NULL, NULL, "qcom,msm8660-surf-fpga");
> > > +       if (!node)
> > > +               return;
> > > +
> > > +       fpga_mem = of_iomap(node, 0);
> > > +       of_node_put(node);
> > > +       if (!fpga_mem) {
> > > +               printk(KERN_ERR "%s: Can't map fpga registers\n", __func__);
> > > +               return;
> > > +       }
> > > +
> > > +       msm8660_surf_fpga_init(fpga_mem);
> > > +       iounmap(fpga_mem);
> > > +}
> > > +#endif
> > 
> > Is the serial port connected through the FPGA or just configured by it?
> 
> The FPGA controls how the UART pins are connected on the development
> board.  The serial port itself is in the MSM, not the FPGA, and on
> other dev boards this isn't needed for the serial port to work.

ok.

> > In the former case, I think it would be better to make this a proper
> > device driver that binds to the qcom,msm8660-surf-fpga device,
> > configures it and then creates the platform_devices for the child
> > nodes (the serial port, possibly others) by calling
> > of_platform_bus_probe.
> 
> It might make sense to have the FPGA as a driver.  I believe it was
> done early to make sure that the pins were configured correctly before
> the serial driver came up.  As far as I can tell, the output pin is
> already configured correctly, so this can actually happen completely
> independently, since early usage of the UART is really only for
> console messages.
> 
> I don't think it makes sense for the serial to be a child node, this
> FPGA configuration is more along the lines of pinmux.  Most
> configurations of this SOC don't have or need this fpga.

Agreed.

> So, if I made it a separate driver, where would it go?  Since this
> board still has platform device support, I suspect the platform data
> needed to describe this device would end up being larger than the
> driver itself.

Excellent question ;-)

When the driver is really small, I would just leave it in the board
file for now, although that might not be a good long-term strategy.
Do we have any similar cases that we can group together with the
fpga to make a subsystem? Maybe it could be a small driver in the
pinmux subsystem when that is established.

	Arnd

[PATCH v3 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT

[David Brown]

> > So, if I made it a separate driver, where would it go?  Since this
> > board still has platform device support, I suspect the platform data
> > needed to describe this device would end up being larger than the
> > driver itself.
> 
> Excellent question ;-)
> 
> When the driver is really small, I would just leave it in the board
> file for now, although that might not be a good long-term strategy.
> Do we have any similar cases that we can group together with the
> fpga to make a subsystem? Maybe it could be a small driver in the
> pinmux subsystem when that is established.

At this point, I think I'm just going to leave this FPGA
initialization out.  It seems that newer versions of the bootloader
have made the FPGA entirely inaccessible.  This has the unfortunate
consequence of making the UART unusable on the MSM8660 SURF.  I'll see
if I can get this fixed at a bootloader level, or come up with another
solution.

So, meanwhile, I'll include the other patches, which work fine on the
MSM 8660 Fluid target, and I'll just make an additional dsb file for
this.  Patch soon...

The FLUID has the advantage of being a device that is technically
available to people (although a bit pricey)

  http://www.bsquare.com/snapdragon-mobile-development-platform.aspx

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.