2013-08-19 21:40:03

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 0/4] Document msm_serial bindings and support newer uartdms

This patchset aligns the msm_serial driver more with downstream usage and
also documents the msm_serial driver's DT binding. Along the way we
update the clock names and add support for future UARTDM hardware that
isn't part of a GSBI.

Stephen Boyd (4):
msm_serial: Switch clock consumer strings and simplify code
devicetree: serial: Document msm_serial bindings
msm_serial: Add support for non-GSBI UARTDM devices
ARM: dts: msm: Update uartdm compatible strings

.../devicetree/bindings/serial/msm_serial.txt | 82 ++++++++++++++++++++++
arch/arm/boot/dts/msm8660-surf.dts | 2 +-
arch/arm/boot/dts/msm8960-cdp.dts | 2 +-
arch/arm/mach-msm/devices-msm7x00.c | 6 +-
arch/arm/mach-msm/devices-msm7x30.c | 2 +-
arch/arm/mach-msm/devices-qsd8x50.c | 6 +-
drivers/tty/serial/msm_serial.c | 29 ++++----
7 files changed, 104 insertions(+), 25 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/msm_serial.txt

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


2013-08-19 21:40:06

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 2/4] devicetree: serial: Document msm_serial bindings

The msm serial device bindings were added to the DTS files but
never documented. Let's document them now and also fix things up
so that it's clearer what hardware is supported. Instead of using
hsuart (for high speed uart), let's use uartdm because that
matches the actual name of the hardware. Also, let's add the
version information in case we need to differentiate between
different versions of the hardware in the future.

Cc: David Brown <[email protected]>
Cc: <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../devicetree/bindings/serial/msm_serial.txt | 82 ++++++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/msm_serial.txt

diff --git a/Documentation/devicetree/bindings/serial/msm_serial.txt b/Documentation/devicetree/bindings/serial/msm_serial.txt
new file mode 100644
index 0000000..a6efac3
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/msm_serial.txt
@@ -0,0 +1,82 @@
+* MSM Serial UART and UARTDM
+
+There are two MSM serial hardware designs. UARTDM is designed for use with a
+dma engine in high-speed use cases and the non-DM design is for lower speed use
+cases. The two designs are mostly compatible from a software perspective except
+the non-DM design can only read and write one character at a time and so the
+register layout differs slightly.
+
+UART
+----
+Required properties:
+- compatible: Should contain "qcom,msm-uart"
+- reg: Should contain UART register location and length. The first
+ register shall specify the main control registers
+- interrupts: Should contain UART interrupt.
+- clocks: Should contain the core clock.
+- clock-names: Should be "core_clk".
+
+Optional properties:
+- dmas: Should contain dma specifiers for transmit and receive
+- dma-names: Should contain "tx" for transmit and "rx" for receive
+
+Example:
+
+A uart device with dma capabilities.
+
+serial@a9c00000 {
+ compatible = "qcom,msm-uart";
+ reg = <0xa9c00000 0x1000>;
+ interrupts = <11>;
+ clocks = <&uart_cxc>;
+ clock-names = "core_clk";
+ dmas = <&dma0 0>, <&dma0 1>;
+ dma-names = "tx", "rx";
+};
+
+UARTDM
+------
+Required properties:
+- compatible: Should contain at least "qcom,msm-uartdm".
+ A more specific property should be specified as follows depending
+ on the version:
+ "qcom,msm-uartdm-v1.1"
+ "qcom,msm-uartdm-v1.2"
+ "qcom,msm-uartdm-v1.3"
+ "qcom,msm-uartdm-v1.4"
+- reg: Should contain UART register locations and lengths. The first
+ register shall specify the main control registers. An optional second
+ register location shall specify the GSBI control region.
+- interrupts: Should contain UART interrupt.
+- clocks: Should contain the core clock and the ahb clock.
+- clock-names: Should be "core_clk" for the core clock and "iface_clk" for the
+ ahb clock.
+
+Optional properties:
+- dmas: Should contain dma specifiers for transmit and receive channels
+- dma-names: Should contain "tx" for transmit and "rx" for receive channels
+
+Examples:
+
+A uartdm v1.4 device with dma capabilities.
+
+serial@f991e000 {
+ compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
+ reg = <0xf991e000 0x1000>;
+ interrupts = <0 108 0x0>;
+ clocks = <&blsp1_uart2_apps_cxc>, <&blsp1_ahb_cxc>;
+ clock-names = "core_clk", "iface_clk";
+ dmas = <&dma0 0>, <&dma0 1>;
+ dma-names = "tx", "rx";
+};
+
+A uartdm v1.3 device without dma capabilities.
+
+serial@19c40000 {
+ compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
+ reg = <0x19c40000 0x1000>,
+ <0x19c00000 0x1000>;
+ interrupts = <0 195 0x0>;
+ clocks = <&gsbi5_uart_cxc>, <&gsbi5_ahb_cxc>;
+ clock-names = "core_clk", "iface_clk";
+};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-08-19 21:40:05

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 1/4] msm_serial: Switch clock consumer strings and simplify code

In downstream kernels we've standardized the clock consumer names
that MSM device drivers use. Replace the uart specific clock
names in this driver with the more standard 'core_clk' and
'iface_clk' names. Also simplify the code by assuming that
clk_prepare_enable() and clk_disable_unprepare() will properly
check for NULL pointers (it will because MSM uses the common
clock framework).

Cc: David Brown <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/mach-msm/devices-msm7x00.c | 6 +++---
arch/arm/mach-msm/devices-msm7x30.c | 2 +-
arch/arm/mach-msm/devices-qsd8x50.c | 6 +++---
drivers/tty/serial/msm_serial.c | 19 +++++--------------
4 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-msm/devices-msm7x00.c b/arch/arm/mach-msm/devices-msm7x00.c
index 6d50fb9..aa2a3c7 100644
--- a/arch/arm/mach-msm/devices-msm7x00.c
+++ b/arch/arm/mach-msm/devices-msm7x00.c
@@ -456,9 +456,9 @@ static struct clk_pcom_desc msm_clocks_7x01a[] = {
CLK_PCOM("tsif_ref_clk", TSIF_REF_CLK, NULL, 0),
CLK_PCOM("tv_dac_clk", TV_DAC_CLK, NULL, 0),
CLK_PCOM("tv_enc_clk", TV_ENC_CLK, NULL, 0),
- CLK_PCOM("uart_clk", UART1_CLK, "msm_serial.0", OFF),
- CLK_PCOM("uart_clk", UART2_CLK, "msm_serial.1", 0),
- CLK_PCOM("uart_clk", UART3_CLK, "msm_serial.2", OFF),
+ CLK_PCOM("core_clk", UART1_CLK, "msm_serial.0", OFF),
+ CLK_PCOM("core_clk", UART2_CLK, "msm_serial.1", 0),
+ CLK_PCOM("core_clk", UART3_CLK, "msm_serial.2", OFF),
CLK_PCOM("uart1dm_clk", UART1DM_CLK, NULL, OFF),
CLK_PCOM("uart2dm_clk", UART2DM_CLK, NULL, 0),
CLK_PCOM("usb_hs_clk", USB_HS_CLK, "msm_hsusb", OFF),
diff --git a/arch/arm/mach-msm/devices-msm7x30.c b/arch/arm/mach-msm/devices-msm7x30.c
index d4db75a..597251e 100644
--- a/arch/arm/mach-msm/devices-msm7x30.c
+++ b/arch/arm/mach-msm/devices-msm7x30.c
@@ -211,7 +211,7 @@ static struct clk_pcom_desc msm_clocks_7x30[] = {
CLK_PCOM("spi_pclk", SPI_P_CLK, NULL, 0),
CLK_PCOM("tv_dac_clk", TV_DAC_CLK, NULL, 0),
CLK_PCOM("tv_enc_clk", TV_ENC_CLK, NULL, 0),
- CLK_PCOM("uart_clk", UART2_CLK, "msm_serial.1", 0),
+ CLK_PCOM("core_clk", UART2_CLK, "msm_serial.1", 0),
CLK_PCOM("usb_phy_clk", USB_PHY_CLK, NULL, 0),
CLK_PCOM("usb_hs_clk", USB_HS_CLK, NULL, OFF),
CLK_PCOM("usb_hs_pclk", USB_HS_P_CLK, NULL, OFF),
diff --git a/arch/arm/mach-msm/devices-qsd8x50.c b/arch/arm/mach-msm/devices-qsd8x50.c
index f551811..b7344a4 100644
--- a/arch/arm/mach-msm/devices-qsd8x50.c
+++ b/arch/arm/mach-msm/devices-qsd8x50.c
@@ -358,9 +358,9 @@ static struct clk_pcom_desc msm_clocks_8x50[] = {
CLK_PCOM("tsif_ref_clk", TSIF_REF_CLK, NULL, 0),
CLK_PCOM("tv_dac_clk", TV_DAC_CLK, NULL, 0),
CLK_PCOM("tv_enc_clk", TV_ENC_CLK, NULL, 0),
- CLK_PCOM("uart_clk", UART1_CLK, NULL, OFF),
- CLK_PCOM("uart_clk", UART2_CLK, NULL, 0),
- CLK_PCOM("uart_clk", UART3_CLK, "msm_serial.2", OFF),
+ CLK_PCOM("core_clk", UART1_CLK, NULL, OFF),
+ CLK_PCOM("core_clk", UART2_CLK, NULL, 0),
+ CLK_PCOM("core_clk", UART3_CLK, "msm_serial.2", OFF),
CLK_PCOM("uartdm_clk", UART1DM_CLK, NULL, OFF),
CLK_PCOM("uartdm_clk", UART2DM_CLK, NULL, 0),
CLK_PCOM("usb_hs_clk", USB_HS_CLK, NULL, OFF),
diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 97642ec..7631313 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -416,8 +416,7 @@ static void msm_init_clock(struct uart_port *port)
struct msm_port *msm_port = UART_TO_MSM(port);

clk_prepare_enable(msm_port->clk);
- if (!IS_ERR(msm_port->pclk))
- clk_prepare_enable(msm_port->pclk);
+ clk_prepare_enable(msm_port->pclk);
msm_serial_set_mnd_regs(port);
}

@@ -693,13 +692,11 @@ static void msm_power(struct uart_port *port, unsigned int state,
switch (state) {
case 0:
clk_prepare_enable(msm_port->clk);
- if (!IS_ERR(msm_port->pclk))
- clk_prepare_enable(msm_port->pclk);
+ clk_prepare_enable(msm_port->pclk);
break;
case 3:
clk_disable_unprepare(msm_port->clk);
- if (!IS_ERR(msm_port->pclk))
- clk_disable_unprepare(msm_port->pclk);
+ clk_disable_unprepare(msm_port->pclk);
break;
default:
printk(KERN_ERR "msm_serial: Unknown PM state %d\n", state);
@@ -887,18 +884,12 @@ static int __init msm_serial_probe(struct platform_device *pdev)
else
msm_port->is_uartdm = 0;

- if (msm_port->is_uartdm) {
- msm_port->clk = devm_clk_get(&pdev->dev, "gsbi_uart_clk");
- msm_port->pclk = devm_clk_get(&pdev->dev, "gsbi_pclk");
- } else {
- msm_port->clk = devm_clk_get(&pdev->dev, "uart_clk");
- msm_port->pclk = ERR_PTR(-ENOENT);
- }
-
+ msm_port->clk = devm_clk_get(&pdev->dev, "core_clk");
if (IS_ERR(msm_port->clk))
return PTR_ERR(msm_port->clk);

if (msm_port->is_uartdm) {
+ msm_port->pclk = devm_clk_get(&pdev->dev, "iface_clk");
if (IS_ERR(msm_port->pclk))
return PTR_ERR(msm_port->pclk);

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-08-19 21:40:59

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 3/4] msm_serial: Add support for non-GSBI UARTDM devices

Not all UARTDM hardware is part of a GSBI complex. Add support
for these devices and fix a bug where we assumed uartdm meant the
hardware was part of a GSBI complex.

Cc: David Brown <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/tty/serial/msm_serial.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 7631313..f42c805 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -670,7 +670,7 @@ static void msm_config_port(struct uart_port *port, int flags)
if (ret)
return;
}
- if (msm_port->is_uartdm)
+ if (msm_port->gsbi_base)
writel_relaxed(GSBI_PROTOCOL_UART,
msm_port->gsbi_base + GSBI_CONTROL);
}
@@ -860,6 +860,11 @@ static struct uart_driver msm_uart_driver = {

static atomic_t msm_uart_next_id = ATOMIC_INIT(0);

+static const struct of_device_id msm_uartdm_table[] = {
+ { .compatible = "qcom,msm-uartdm" },
+ { }
+};
+
static int __init msm_serial_probe(struct platform_device *pdev)
{
struct msm_port *msm_port;
@@ -879,7 +884,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(pdev, IORESOURCE_MEM, 1))
+ if (of_match_device(msm_uartdm_table, &pdev->dev))
msm_port->is_uartdm = 1;
else
msm_port->is_uartdm = 0;
@@ -926,6 +931,7 @@ static int msm_serial_remove(struct platform_device *pdev)

static struct of_device_id msm_match_table[] = {
{ .compatible = "qcom,msm-uart" },
+ { .compatible = "qcom,msm-uartdm" },
{}
};

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-08-19 21:40:58

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 4/4] ARM: dts: msm: Update uartdm compatible strings

Let's follow the ratified DT binding and use uartdm instead of
hsuart. This does break backwards compatibility but this
shouldn't be a problem because the uart driver isn't probing on
these devices without adding clock support (which isn't merged so
far).

Cc: David Brown <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/boot/dts/msm8660-surf.dts | 2 +-
arch/arm/boot/dts/msm8960-cdp.dts | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
index cdc010e..386d428 100644
--- a/arch/arm/boot/dts/msm8660-surf.dts
+++ b/arch/arm/boot/dts/msm8660-surf.dts
@@ -38,7 +38,7 @@
};

serial@19c40000 {
- compatible = "qcom,msm-hsuart", "qcom,msm-uart";
+ compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
reg = <0x19c40000 0x1000>,
<0x19c00000 0x1000>;
interrupts = <0 195 0x0>;
diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
index db2060c..532050b 100644
--- a/arch/arm/boot/dts/msm8960-cdp.dts
+++ b/arch/arm/boot/dts/msm8960-cdp.dts
@@ -38,7 +38,7 @@
};

serial@16440000 {
- compatible = "qcom,msm-hsuart", "qcom,msm-uart";
+ compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
reg = <0x16440000 0x1000>,
<0x16400000 0x1000>;
interrupts = <0 154 0x0>;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-08-20 14:41:21

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH 2/4] devicetree: serial: Document msm_serial bindings


On Aug 19, 2013, at 4:39 PM, Stephen Boyd wrote:

> The msm serial device bindings were added to the DTS files but
> never documented. Let's document them now and also fix things up
> so that it's clearer what hardware is supported. Instead of using
> hsuart (for high speed uart), let's use uartdm because that
> matches the actual name of the hardware. Also, let's add the
> version information in case we need to differentiate between
> different versions of the hardware in the future.
>
> Cc: David Brown <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../devicetree/bindings/serial/msm_serial.txt | 82 ++++++++++++++++++++++
> 1 file changed, 82 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/msm_serial.txt
>
> diff --git a/Documentation/devicetree/bindings/serial/msm_serial.txt b/Documentation/devicetree/bindings/serial/msm_serial.txt
> new file mode 100644
> index 0000000..a6efac3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/msm_serial.txt
> @@ -0,0 +1,82 @@
> +* MSM Serial UART and UARTDM
> +
> +There are two MSM serial hardware designs. UARTDM is designed for use with a
> +dma engine in high-speed use cases and the non-DM design is for lower speed use
> +cases. The two designs are mostly compatible from a software perspective except
> +the non-DM design can only read and write one character at a time and so the
> +register layout differs slightly.

I think you split this into two binding spec docs, one for each type of uart.

> +
> +UART
> +----
> +Required properties:
> +- compatible: Should contain "qcom,msm-uart"
> +- reg: Should contain UART register location and length. The first

first? is there more than one reg region?

> + register shall specify the main control registers
> +- interrupts: Should contain UART interrupt.
> +- clocks: Should contain the core clock.
> +- clock-names: Should be "core_clk".
> +
> +Optional properties:
> +- dmas: Should contain dma specifiers for transmit and receive
> +- dma-names: Should contain "tx" for transmit and "rx" for receive

confused, above you say the non-DM doesn't support DMA so, why the optional props?

> +
> +Example:
> +
> +A uart device with dma capabilities.
> +
> +serial@a9c00000 {
> + compatible = "qcom,msm-uart";
> + reg = <0xa9c00000 0x1000>;
> + interrupts = <11>;
> + clocks = <&uart_cxc>;
> + clock-names = "core_clk";
> + dmas = <&dma0 0>, <&dma0 1>;
> + dma-names = "tx", "rx";
> +};
> +
> +UARTDM
> +------
> +Required properties:
> +- compatible: Should contain at least "qcom,msm-uartdm".
> + A more specific property should be specified as follows depending
> + on the version:
> + "qcom,msm-uartdm-v1.1"
> + "qcom,msm-uartdm-v1.2"
> + "qcom,msm-uartdm-v1.3"
> + "qcom,msm-uartdm-v1.4"
> +- reg: Should contain UART register locations and lengths. The first
> + register shall specify the main control registers. An optional second
> + register location shall specify the GSBI control region.

Is GSBI region existing tied to particular versions (if so can we say that)

reg-names?

> +- interrupts: Should contain UART interrupt.
> +- clocks: Should contain the core clock and the ahb clock.

nit, ahb in caps?

> +- clock-names: Should be "core_clk" for the core clock and "iface_clk" for the
> + ahb clock.
> +
> +Optional properties:
> +- dmas: Should contain dma specifiers for transmit and receive channels
> +- dma-names: Should contain "tx" for transmit and "rx" for receive channels
> +
> +Examples:
> +
> +A uartdm v1.4 device with dma capabilities.
> +
> +serial@f991e000 {
> + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
> + reg = <0xf991e000 0x1000>;
> + interrupts = <0 108 0x0>;
> + clocks = <&blsp1_uart2_apps_cxc>, <&blsp1_ahb_cxc>;
> + clock-names = "core_clk", "iface_clk";
> + dmas = <&dma0 0>, <&dma0 1>;
> + dma-names = "tx", "rx";
> +};
> +
> +A uartdm v1.3 device without dma capabilities.
> +
> +serial@19c40000 {
> + compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
> + reg = <0x19c40000 0x1000>,
> + <0x19c00000 0x1000>;
> + interrupts = <0 195 0x0>;
> + clocks = <&gsbi5_uart_cxc>, <&gsbi5_ahb_cxc>;
> + clock-names = "core_clk", "iface_clk";
> +};
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2013-08-20 17:00:26

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 2/4] devicetree: serial: Document msm_serial bindings

On 08/20/13 07:41, Kumar Gala wrote:
> On Aug 19, 2013, at 4:39 PM, Stephen Boyd wrote:
>
>> diff --git a/Documentation/devicetree/bindings/serial/msm_serial.txt b/Documentation/devicetree/bindings/serial/msm_serial.txt
>> new file mode 100644
>> index 0000000..a6efac3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/msm_serial.txt
>> @@ -0,0 +1,82 @@
>> +* MSM Serial UART and UARTDM
>> +
>> +There are two MSM serial hardware designs. UARTDM is designed for use with a
>> +dma engine in high-speed use cases and the non-DM design is for lower speed use
>> +cases. The two designs are mostly compatible from a software perspective except
>> +the non-DM design can only read and write one character at a time and so the
>> +register layout differs slightly.
> I think you split this into two binding spec docs, one for each type of uart.

Should split into two files? I can do that.

>
>> +
>> +UART
>> +----
>> +Required properties:
>> +- compatible: Should contain "qcom,msm-uart"
>> +- reg: Should contain UART register location and length. The first
> first? is there more than one reg region?
>
>> + register shall specify the main control registers
>> +- interrupts: Should contain UART interrupt.
>> +- clocks: Should contain the core clock.
>> +- clock-names: Should be "core_clk".
>> +
>> +Optional properties:
>> +- dmas: Should contain dma specifiers for transmit and receive
>> +- dma-names: Should contain "tx" for transmit and "rx" for receive
> confused, above you say the non-DM doesn't support DMA so, why the optional props?

Ah sorry, copy pasta.

>
>> +
>> +Example:
>> +
>> +A uart device with dma capabilities.
>> +
>> +serial@a9c00000 {
>> + compatible = "qcom,msm-uart";
>> + reg = <0xa9c00000 0x1000>;
>> + interrupts = <11>;
>> + clocks = <&uart_cxc>;
>> + clock-names = "core_clk";
>> + dmas = <&dma0 0>, <&dma0 1>;
>> + dma-names = "tx", "rx";
>> +};
>> +
>> +UARTDM
>> +------
>> +Required properties:
>> +- compatible: Should contain at least "qcom,msm-uartdm".
>> + A more specific property should be specified as follows depending
>> + on the version:
>> + "qcom,msm-uartdm-v1.1"
>> + "qcom,msm-uartdm-v1.2"
>> + "qcom,msm-uartdm-v1.3"
>> + "qcom,msm-uartdm-v1.4"
>> +- reg: Should contain UART register locations and lengths. The first
>> + register shall specify the main control registers. An optional second
>> + register location shall specify the GSBI control region.
> Is GSBI region existing tied to particular versions (if so can we say that)

Not really. GSBI will always be related to v1.3 but not all v1.3
hardware is part of a GSBI.

>
> reg-names?

Optional should be fine? The driver is already handling this without
reg-names.

>> +- interrupts: Should contain UART interrupt.
>> +- clocks: Should contain the core clock and the ahb clock.
> nit, ahb in caps?

Done.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2013-08-20 18:03:36

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH 2/4] devicetree: serial: Document msm_serial bindings


On Aug 20, 2013, at 12:00 PM, Stephen Boyd wrote:

> On 08/20/13 07:41, Kumar Gala wrote:
>> On Aug 19, 2013, at 4:39 PM, Stephen Boyd wrote:
>>
>>> diff --git a/Documentation/devicetree/bindings/serial/msm_serial.txt b/Documentation/devicetree/bindings/serial/msm_serial.txt
>>> new file mode 100644
>>> index 0000000..a6efac3
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/msm_serial.txt
>>> @@ -0,0 +1,82 @@
>>> +* MSM Serial UART and UARTDM
>>> +
>>> +There are two MSM serial hardware designs. UARTDM is designed for use with a
>>> +dma engine in high-speed use cases and the non-DM design is for lower speed use
>>> +cases. The two designs are mostly compatible from a software perspective except
>>> +the non-DM design can only read and write one character at a time and so the
>>> +register layout differs slightly.
>> I think you split this into two binding spec docs, one for each type of uart.
>
> Should split into two files? I can do that.

yes.


>>> +UART
>>> +----
>>> +Required properties:
>>> +- compatible: Should contain "qcom,msm-uart"
>>> +- reg: Should contain UART register location and length. The first
>> first? is there more than one reg region?

?

>>> + register shall specify the main control registers
>>> +- interrupts: Should contain UART interrupt.
>>> +- clocks: Should contain the core clock.
>>> +- clock-names: Should be "core_clk".
>>> +
>>> +Optional properties:
>>> +- dmas: Should contain dma specifiers for transmit and receive
>>> +- dma-names: Should contain "tx" for transmit and "rx" for receive
>> confused, above you say the non-DM doesn't support DMA so, why the optional props?
>
> Ah sorry, copy pasta.
>
>>
>>> +
>>> +Example:
>>> +
>>> +A uart device with dma capabilities.
>>> +
>>> +serial@a9c00000 {
>>> + compatible = "qcom,msm-uart";
>>> + reg = <0xa9c00000 0x1000>;
>>> + interrupts = <11>;
>>> + clocks = <&uart_cxc>;
>>> + clock-names = "core_clk";
>>> + dmas = <&dma0 0>, <&dma0 1>;
>>> + dma-names = "tx", "rx";
>>> +};
>>> +
>>> +UARTDM
>>> +------
>>> +Required properties:
>>> +- compatible: Should contain at least "qcom,msm-uartdm".
>>> + A more specific property should be specified as follows depending
>>> + on the version:
>>> + "qcom,msm-uartdm-v1.1"
>>> + "qcom,msm-uartdm-v1.2"
>>> + "qcom,msm-uartdm-v1.3"
>>> + "qcom,msm-uartdm-v1.4"
>>> +- reg: Should contain UART register locations and lengths. The first
>>> + register shall specify the main control registers. An optional second
>>> + register location shall specify the GSBI control region.
>> Is GSBI region existing tied to particular versions (if so can we say that)
>
> Not really. GSBI will always be related to v1.3 but not all v1.3
> hardware is part of a GSBI.

How about adding that into the binding.

>> reg-names?
>
> Optional should be fine? The driver is already handling this without
> reg-names.
>
>>> +- interrupts: Should contain UART interrupt.
>>> +- clocks: Should contain the core clock and the ahb clock.
>> nit, ahb in caps?
>
> Done.

- k

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation