[v2,6/7] dt-bindings: serial: Add bindings for GENI based UART Controller

Message ID 1515805547-22816-7-git-send-email-kramasub@codeaurora.org
State Not Applicable
Headers show
Series
  • Introduce GENI SE Controller Driver
Related show

Commit Message

Karthik Ramasubramanian Jan. 13, 2018, 1:05 a.m.
Add device tree binding support for GENI based UART Controller in the
QUP Wrapper.

Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org>
Signed-off-by: Girish Mahadevan <girishm@codeaurora.org>
---
 .../devicetree/bindings/serial/qcom,geni-uart.txt  | 29 ++++++++++++++++++++++
 .../devicetree/bindings/soc/qcom/qcom,geni-se.txt  | 13 ++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/qcom,geni-uart.txt

Comments

Bjorn Andersson Jan. 17, 2018, 6:35 a.m. | #1
On Fri 12 Jan 17:05 PST 2018, Karthikeyan Ramasubramanian wrote:

> Add device tree binding support for GENI based UART Controller in the
> QUP Wrapper.
> 
> Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org>
> Signed-off-by: Girish Mahadevan <girishm@codeaurora.org>
> ---
>  .../devicetree/bindings/serial/qcom,geni-uart.txt  | 29 ++++++++++++++++++++++
>  .../devicetree/bindings/soc/qcom/qcom,geni-se.txt  | 13 ++++++++++
>  2 files changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt b/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
> new file mode 100644
> index 0000000..e7b9e24
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
> @@ -0,0 +1,29 @@
> +Qualcomm Technologies Inc. GENI Serial Engine based UART Controller
> +
> +The Generic Interface (GENI) Serial Engine based UART controller supports
> +console use-cases and is supported only by GENI based Qualcomm Universal
> +Peripheral (QUP) cores.
> +
> +Required properties:
> +- compatible: should contain "qcom,geni-debug-uart".

Why is this uart a _debug_ uart? Is there a separate binding for the
geni-uart?

I like that your naming here matches my suggestion with qcom,geni-i2c.

> +- reg: Should contain UART register location and length.
> +- reg-names: Should contain "se-phys".

No need to specify reg-names for a single reg.

> +- interrupts: Should contain UART core interrupts.
> +- clock-names: Should contain "se-clk".

Omit the "clk"

> +- clocks: clocks needed for UART, includes the core clock.

Be more specific.

> +- pinctrl-names/pinctrl-0/1: The GPIOs assigned to this core. The names
> +  Should be "active" and "sleep" for the pin confuguration when core is active
> +  or when entering sleep state.

Omit pinctrl information.

> +
> +Example:
> +uart0: qcom,serial@a88000 {

Don't use qcom, in node name. This should be named "serial".

> +	compatible = "qcom,geni-debug-uart";
> +	reg = <0xa88000 0x7000>;
> +	reg-names = "se-phys";
> +	interrupts = <0 355 0>;

<GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>

> +	clock-names = "se-clk";
> +	clocks = <&clock_gcc GCC_QUPV3_WRAP0_S0_CLK>;
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&qup_1_uart_3_active>;
> +	pinctrl-1 = <&qup_1_uart_3_sleep>;
> +};

Regards,
Bjorn
Karthik Ramasubramanian Feb. 27, 2018, 1:25 p.m. | #2
On 1/16/2018 11:35 PM, Bjorn Andersson wrote:
> On Fri 12 Jan 17:05 PST 2018, Karthikeyan Ramasubramanian wrote:
> 
>> Add device tree binding support for GENI based UART Controller in the
>> QUP Wrapper.
>>
>> Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org>
>> Signed-off-by: Girish Mahadevan <girishm@codeaurora.org>
>> ---
>>   .../devicetree/bindings/serial/qcom,geni-uart.txt  | 29 ++++++++++++++++++++++
>>   .../devicetree/bindings/soc/qcom/qcom,geni-se.txt  | 13 ++++++++++
>>   2 files changed, 42 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
>>
>> diff --git a/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt b/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
>> new file mode 100644
>> index 0000000..e7b9e24
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
>> @@ -0,0 +1,29 @@
>> +Qualcomm Technologies Inc. GENI Serial Engine based UART Controller
>> +
>> +The Generic Interface (GENI) Serial Engine based UART controller supports
>> +console use-cases and is supported only by GENI based Qualcomm Universal
>> +Peripheral (QUP) cores.
>> +
>> +Required properties:
>> +- compatible: should contain "qcom,geni-debug-uart".
> 
> Why is this uart a _debug_ uart? Is there a separate binding for the
> geni-uart?
Yes. It will be uploaded at a later point in time.
> 
> I like that your naming here matches my suggestion with qcom,geni-i2c.
> 
>> +- reg: Should contain UART register location and length.
>> +- reg-names: Should contain "se-phys".
> 
> No need to specify reg-names for a single reg.
Ok.
> 
>> +- interrupts: Should contain UART core interrupts.
>> +- clock-names: Should contain "se-clk".
> 
> Omit the "clk"
Ok.
> 
>> +- clocks: clocks needed for UART, includes the core clock.
> 
> Be more specific.
Ok.
> 
>> +- pinctrl-names/pinctrl-0/1: The GPIOs assigned to this core. The names
>> +  Should be "active" and "sleep" for the pin confuguration when core is active
>> +  or when entering sleep state.
> 
> Omit pinctrl information.
Ok.
> 
>> +
>> +Example:
>> +uart0: qcom,serial@a88000 {
> 
> Don't use qcom, in node name. This should be named "serial".
Ok.
> 
>> +	compatible = "qcom,geni-debug-uart";
>> +	reg = <0xa88000 0x7000>;
>> +	reg-names = "se-phys";
>> +	interrupts = <0 355 0>;
> 
> <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>
Ok.
> 
>> +	clock-names = "se-clk";
>> +	clocks = <&clock_gcc GCC_QUPV3_WRAP0_S0_CLK>;
>> +	pinctrl-names = "default", "sleep";
>> +	pinctrl-0 = <&qup_1_uart_3_active>;
>> +	pinctrl-1 = <&qup_1_uart_3_sleep>;
>> +};
> 
> Regards,
> Bjorn
> 
Regards,
Karthik.

Patch

diff --git a/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt b/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
new file mode 100644
index 0000000..e7b9e24
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
@@ -0,0 +1,29 @@ 
+Qualcomm Technologies Inc. GENI Serial Engine based UART Controller
+
+The Generic Interface (GENI) Serial Engine based UART controller supports
+console use-cases and is supported only by GENI based Qualcomm Universal
+Peripheral (QUP) cores.
+
+Required properties:
+- compatible: should contain "qcom,geni-debug-uart".
+- reg: Should contain UART register location and length.
+- reg-names: Should contain "se-phys".
+- interrupts: Should contain UART core interrupts.
+- clock-names: Should contain "se-clk".
+- clocks: clocks needed for UART, includes the core clock.
+- pinctrl-names/pinctrl-0/1: The GPIOs assigned to this core. The names
+  Should be "active" and "sleep" for the pin confuguration when core is active
+  or when entering sleep state.
+
+Example:
+uart0: qcom,serial@a88000 {
+	compatible = "qcom,geni-debug-uart";
+	reg = <0xa88000 0x7000>;
+	reg-names = "se-phys";
+	interrupts = <0 355 0>;
+	clock-names = "se-clk";
+	clocks = <&clock_gcc GCC_QUPV3_WRAP0_S0_CLK>;
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&qup_1_uart_3_active>;
+	pinctrl-1 = <&qup_1_uart_3_sleep>;
+};
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
index 2ffbb3e..c307788 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
@@ -26,6 +26,7 @@  controller, spi controller, or some combination of aforementioned devices.
 
 See the following documentation for child node definitions:
 Documentation/devicetree/bindings/i2c/i2c-qcom-geni.txt
+Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
 
 Example:
 	qup0: qcom,geniqup0@8c0000 {
@@ -50,4 +51,16 @@  Example:
 			#address-cells = <1>;
 			#size-cells = <0>;
 		};
+
+		uart0: qcom,serial@a88000 {
+			compatible = "qcom,geni-debug-uart";
+			reg = <0xa88000 0x7000>;
+			reg-names = "se-phys";
+			interrupts = <0 355 0>;
+			clock-names = "se-clk";
+			clocks = <&clock_gcc GCC_QUPV3_WRAP0_S0_CLK>;
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&qup_1_uart_3_active>;
+			pinctrl-1 = <&qup_1_uart_3_sleep>;
+		};
 	}