linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Initial DT support for MSM8660
@ 2011-08-12 23:00 David Brown
  2011-08-12 23:00 ` [PATCH 1/4] msm_serial: Use relative resources for iomem David Brown
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: David Brown @ 2011-08-12 23:00 UTC (permalink / raw)
  To: David Brown, Daniel Walker, Bryan Huntsman, Alan Cox,
	Randy Dunlap, Grant Likely
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-serial,
	linux-doc, devicetree-discuss

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.

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  |   18 ++++
 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, 158 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.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] msm_serial: Use relative resources for iomem
  2011-08-12 23:00 [PATCH 0/4] Initial DT support for MSM8660 David Brown
@ 2011-08-12 23:00 ` David Brown
  2011-08-12 23:00 ` [PATCH 2/4] msm_serial: Add devicetree support David Brown
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: David Brown @ 2011-08-12 23:00 UTC (permalink / raw)
  To: David Brown, Daniel Walker, Bryan Huntsman, Alan Cox
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-serial,
	devicetree-discuss

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.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] msm_serial: Add devicetree support
  2011-08-12 23:00 [PATCH 0/4] Initial DT support for MSM8660 David Brown
  2011-08-12 23:00 ` [PATCH 1/4] msm_serial: Use relative resources for iomem David Brown
@ 2011-08-12 23:00 ` David Brown
  2011-08-13  8:29   ` Arnd Bergmann
  2011-08-12 23:00 ` [PATCH 3/4] ARM: msm: Add devicetree support for msm8660-surf David Brown
  2011-08-12 23:00 ` [PATCH 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT David Brown
  3 siblings, 1 reply; 10+ messages in thread
From: David Brown @ 2011-08-12 23:00 UTC (permalink / raw)
  To: Randy Dunlap, David Brown, Daniel Walker, Bryan Huntsman,
	Alan Cox, Grant Likely
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-doc,
	linux-serial, devicetree-discuss

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  |   18 ++++++++++++++++++
 drivers/tty/serial/msm_serial.c                    |   14 ++++++++++++++
 2 files changed, 32 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..071459c
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/msm_serial.txt
@@ -0,0 +1,18 @@
+* Qualcomm MSM UART
+
+Required properties:
+- compatible :
+	- "qcom,msm-uart"
+- 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.
+
+Example:
+
+	uart@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..d5a6db6 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-hsuart-lite" },
+	{}
+};
+
 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.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] ARM: msm: Add devicetree support for msm8660-surf
  2011-08-12 23:00 [PATCH 0/4] Initial DT support for MSM8660 David Brown
  2011-08-12 23:00 ` [PATCH 1/4] msm_serial: Use relative resources for iomem David Brown
  2011-08-12 23:00 ` [PATCH 2/4] msm_serial: Add devicetree support David Brown
@ 2011-08-12 23:00 ` David Brown
  2011-08-12 23:00 ` [PATCH 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT David Brown
  3 siblings, 0 replies; 10+ messages in thread
From: David Brown @ 2011-08-12 23:00 UTC (permalink / raw)
  To: Russell King, David Brown, Daniel Walker, Bryan Huntsman
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree-discuss

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..308d867
--- /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@02080000 {
+		compatible = "qcom,msm-8660-qgic";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		reg = < 0x02080000 0x1000 >,
+		      < 0x02081000 0x1000 >;
+	};
+
+	serial@19c400000 {
+		compatible = "qcom,msm-hsuart-lite";
+		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.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT
  2011-08-12 23:00 [PATCH 0/4] Initial DT support for MSM8660 David Brown
                   ` (2 preceding siblings ...)
  2011-08-12 23:00 ` [PATCH 3/4] ARM: msm: Add devicetree support for msm8660-surf David Brown
@ 2011-08-12 23:00 ` David Brown
  3 siblings, 0 replies; 10+ messages in thread
From: David Brown @ 2011-08-12 23:00 UTC (permalink / raw)
  To: Russell King, David Brown, Daniel Walker, Bryan Huntsman
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree-discuss

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  |   48 +++++++++++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
index 308d867..13a91f7 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@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..db973bb 100644
--- a/arch/arm/mach-msm/board-msm8x60.c
+++ b/arch/arm/mach-msm/board-msm8x60.c
@@ -58,8 +58,51 @@ static void __init msm8x60_init_irq(void)
 	}
 }
 
+static void __init msm8660_surf_fpga_init(void *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();
+	iounmap(fpga_mem);
+}
+
+static void __init msm8660_surf_fpga_init_platform(void)
+{
+	/* 0x1D000000 now belongs to EBI2:CS3 i.e. USB ISP Controller */
+	void *fpga_mem = ioremap(0x1D000000, SZ_4K);
+	msm8660_surf_fpga_init(fpga_mem);
+}
+
+#ifdef CONFIG_OF
+static void __init msm8660_surf_fpga_init_dt(void)
+{
+	struct device_node *node;
+	void *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);
+}
+#endif
+
 static void __init msm8x60_init(void)
 {
+	msm8660_surf_fpga_init_platform();
 }
 
 #ifdef CONFIG_OF
@@ -81,10 +124,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.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] msm_serial: Add devicetree support
  2011-08-12 23:00 ` [PATCH 2/4] msm_serial: Add devicetree support David Brown
@ 2011-08-13  8:29   ` Arnd Bergmann
  2011-08-13 19:46     ` David Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2011-08-13  8:29 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: David Brown, Randy Dunlap, Daniel Walker, Bryan Huntsman,
	Alan Cox, Grant Likely, linux-doc, linux-arm-msm,
	devicetree-discuss, linux-kernel, linux-serial

On Friday 12 August 2011 16:00:06 David Brown wrote:
> +Required properties:
> +- compatible :
> +       - "qcom,msm-uart"
> +- 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.
> +
> +Example:
> +
> +       uart@19c400000 {
> +               compatible = "qcom,msm-hsuart", "qcom,msm-uart";
> +               reg = <0x19c40000 0x1000">,
> +                     <0x19c00000 0x1000">;
> +               interrupts = <195>;
> +       };



> @@ -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-hsuart-lite" },
> +       {}
> +};
> +

Hi David,

It looks like you changed the value for the "compatible" property in the
process of making the patch, but did not update all places.

Should it be qcom,msm-hsuart-lite or qcom,msm-hsuart?

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] msm_serial: Add devicetree support
  2011-08-13  8:29   ` Arnd Bergmann
@ 2011-08-13 19:46     ` David Brown
  2011-08-13 21:34       ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: David Brown @ 2011-08-13 19:46 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, David Brown, Randy Dunlap, Daniel Walker,
	Bryan Huntsman, Alan Cox, Grant Likely, linux-doc, linux-arm-msm,
	devicetree-discuss, linux-kernel, linux-serial

On Sat, Aug 13, 2011 at 10:29:00AM +0200, Arnd Bergmann wrote:
> On Friday 12 August 2011 16:00:06 David Brown wrote:
> > +Required properties:
> > +- compatible :
> > +       - "qcom,msm-uart"
> > +- 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.
> > +
> > +Example:
> > +
> > +       uart@19c400000 {
> > +               compatible = "qcom,msm-hsuart", "qcom,msm-uart";
> > +               reg = <0x19c40000 0x1000">,
> > +                     <0x19c00000 0x1000">;
> > +               interrupts = <195>;
> > +       };
> 
> 
> 
> > @@ -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-hsuart-lite" },
> > +       {}
> > +};
> > +
> 
> Hi David,
> 
> It looks like you changed the value for the "compatible" property in the
> process of making the patch, but did not update all places.
> 
> Should it be qcom,msm-hsuart-lite or qcom,msm-hsuart?

I guess I got the documentation to be different than the code.
Sadily, I think the documentation is probably what I want instead of
the code.

I'm not sure actually what is best to use here.  I'm thinking that the
'lite' identifier should perhaps go away.  MSM's have two UARTS on
them, an older "simple" PIO type of UART, and a newer one that can do
DMA (called the hsuart for high-speed).  The hsuart can also be used
in a non-DMA driver in a mostly compatible way with the old UART.

For non-high-speed applications, the user will probably just want to
use the non-DMA driver.  My question is then: if the device tree
describes it as

	compatible = "qcom,msm-hsuart", "qcom,msm-uart";

and one driver matches qcom,msm-hsuart and another matches
qcom,msm-uart, which driver will get used.  Ideally, it would use the
earliest one in the list.

If that's the case, I'll get rid of the -lite suffix and just make the
non-DMA driver compatible with the plain "qcom,msm-uart".

David

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] msm_serial: Add devicetree support
  2011-08-13 19:46     ` David Brown
@ 2011-08-13 21:34       ` Arnd Bergmann
  2011-08-16 17:57         ` David Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2011-08-13 21:34 UTC (permalink / raw)
  To: David Brown
  Cc: linux-arm-kernel, Randy Dunlap, Daniel Walker, Bryan Huntsman,
	Alan Cox, Grant Likely, linux-doc, linux-arm-msm,
	devicetree-discuss, linux-kernel, linux-serial

On Saturday 13 August 2011 12:46:45 David Brown wrote:
> 
> I'm not sure actually what is best to use here.  I'm thinking that the
> 'lite' identifier should perhaps go away.  MSM's have two UARTS on
> them, an older "simple" PIO type of UART, and a newer one that can do
> DMA (called the hsuart for high-speed).  The hsuart can also be used
> in a non-DMA driver in a mostly compatible way with the old UART.
> 
> For non-high-speed applications, the user will probably just want to
> use the non-DMA driver.  My question is then: if the device tree
> describes it as
> 
>         compatible = "qcom,msm-hsuart", "qcom,msm-uart";
> 
> and one driver matches qcom,msm-hsuart and another matches
> qcom,msm-uart, which driver will get used.  Ideally, it would use the
> earliest one in the list.
> 
> If that's the case, I'll get rid of the -lite suffix and just make the
> non-DMA driver compatible with the plain "qcom,msm-uart".

I believe that unfortunately the answer is that the first driver that
matches anything will get used. There are two possible ways that I can
see to make it do what you want anyway:

1. In the probe function for the slow driver, you return an error
when the device you get passed matches "qcom,msm-hsuart", possibly
dependent on whether the other driver also got built.

2. You register one platform driver that handles both names and
gives the device to just one of the two drivers. This would probably
require linking the two drivers into the same module, or having
the non-DMA speed driver just act as a library.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] msm_serial: Add devicetree support
  2011-08-13 21:34       ` Arnd Bergmann
@ 2011-08-16 17:57         ` David Brown
  2011-08-16 19:07           ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: David Brown @ 2011-08-16 17:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Brown, linux-arm-kernel, Randy Dunlap, Daniel Walker,
	Bryan Huntsman, Alan Cox, Grant Likely, linux-doc, linux-arm-msm,
	devicetree-discuss, linux-kernel, linux-serial

On Sat, Aug 13, 2011 at 11:34:15PM +0200, Arnd Bergmann wrote:
> On Saturday 13 August 2011 12:46:45 David Brown wrote:
> > 
> > I'm not sure actually what is best to use here.  I'm thinking that the
> > 'lite' identifier should perhaps go away.  MSM's have two UARTS on
> > them, an older "simple" PIO type of UART, and a newer one that can do
> > DMA (called the hsuart for high-speed).  The hsuart can also be used
> > in a non-DMA driver in a mostly compatible way with the old UART.
> > 
> > For non-high-speed applications, the user will probably just want to
> > use the non-DMA driver.  My question is then: if the device tree
> > describes it as
> > 
> >         compatible = "qcom,msm-hsuart", "qcom,msm-uart";
> > 
> > and one driver matches qcom,msm-hsuart and another matches
> > qcom,msm-uart, which driver will get used.  Ideally, it would use the
> > earliest one in the list.
> > 
> > If that's the case, I'll get rid of the -lite suffix and just make the
> > non-DMA driver compatible with the plain "qcom,msm-uart".
> 
> I believe that unfortunately the answer is that the first driver that
> matches anything will get used. There are two possible ways that I can
> see to make it do what you want anyway:
> 
> 1. In the probe function for the slow driver, you return an error
> when the device you get passed matches "qcom,msm-hsuart", possibly
> dependent on whether the other driver also got built.
> 
> 2. You register one platform driver that handles both names and
> gives the device to just one of the two drivers. This would probably
> require linking the two drivers into the same module, or having
> the non-DMA speed driver just act as a library.

How about if I just keep it simple for now.  Since there isn't
actually a driver for the DMA version, this driver will handle both
UART blocks, so I'll just do the plain thing in the DT.

In the future, when a DMA-capable driver exists, we can figure out how
to determine which driver should be used.  At this point, I'm not even
sure what the correct answer will be, since a given configuration may
want to use non-DMA for one msm-hsuart device, and the DMA driver for
another.  It's kind of board/use specific, but beyond just describing
what the hardware is.

I've just sent new patches with this fixed up.

David

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] msm_serial: Add devicetree support
  2011-08-16 17:57         ` David Brown
@ 2011-08-16 19:07           ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2011-08-16 19:07 UTC (permalink / raw)
  To: David Brown
  Cc: linux-arm-kernel, Randy Dunlap, Daniel Walker, Bryan Huntsman,
	Alan Cox, Grant Likely, linux-doc, linux-arm-msm,
	devicetree-discuss, linux-kernel, linux-serial

On Tuesday 16 August 2011 10:57:02 David Brown wrote:
> How about if I just keep it simple for now.  Since there isn't
> actually a driver for the DMA version, this driver will handle both
> UART blocks, so I'll just do the plain thing in the DT.

Sounds good to me.

> In the future, when a DMA-capable driver exists, we can figure out how
> to determine which driver should be used.  At this point, I'm not even
> sure what the correct answer will be, since a given configuration may
> want to use non-DMA for one msm-hsuart device, and the DMA driver for
> another.  It's kind of board/use specific, but beyond just describing
> what the hardware is.

In order to be absolutely future-proof, you could mandate that you always
list two "compatible" values, one for the generic version and one for
the specific implementation (high-speed or low-speed). It's a simple
change from what you have now and it allows to change the drivers to
bind to just the specific name in case you want to handle them separately
in the future, without having to change the device tree files.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-08-16 19:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-12 23:00 [PATCH 0/4] Initial DT support for MSM8660 David Brown
2011-08-12 23:00 ` [PATCH 1/4] msm_serial: Use relative resources for iomem David Brown
2011-08-12 23:00 ` [PATCH 2/4] msm_serial: Add devicetree support David Brown
2011-08-13  8:29   ` Arnd Bergmann
2011-08-13 19:46     ` David Brown
2011-08-13 21:34       ` Arnd Bergmann
2011-08-16 17:57         ` David Brown
2011-08-16 19:07           ` Arnd Bergmann
2011-08-12 23:00 ` [PATCH 3/4] ARM: msm: Add devicetree support for msm8660-surf David Brown
2011-08-12 23:00 ` [PATCH 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT David Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).