2017-06-09 09:49:31

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v4 0/7] tty/serial: meson_uart: add support for core clock handling

This patchset is a re-spin of Helmut Klein's v3 patchset at [0].

Initially, the original patchset was made to enable usage on the non-AO UARTS
not enabled by the Bootloader (uart_B and uart_C), but the patchset needed
an overall change to have clean and stable DT bindings.

The Amlogic Meson UART Driver did not have stable DT bindings and mismatched
clock handling on non-AO UARTs since these "EE" UARTs needs a clock gate to
be ungated to works correctly.
In the same way, the AO UARTs does not need gating and can be used as
Early Consoles.

In the same time, the UART Interfaces can take clock input for the baudrate
generate from either the external Xtal or the internal Bus Clock (clk81).

So new bindings was necessary to meet these requirements and the DT
maintainers requirements.

The "legacy" binding actually used in the driver is left until all the DT
files are switched to the new bindings.

The GX DT has been tested, but the last 4 Meson6/Meson8/b are only
compile-tested, and testing is welcome.
Thus only the first 3 patches can be merged until the Meson6/Meson8/b are
formally tested.

It must be noted that the meson6 cannot work today except using an early
console since the UART driver could not probe without a clocks property.

[0] [email protected]

Helmut Klein (3):
dt-bindings: serial: Add bindings for the Amlogic Meson UARTs
tty/serial: meson_uart: update to stable bindings
ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

Neil Armstrong (4):
ARM: dts: meson: use meson6 UART compatible like other nodes
ARM: dts: meson6: switch to new bindings for UART nodes
ARM: dts: meson8: switch to new bindings for UART nodes
ARM: dts: meson8b: switch to new bindings for UART nodes

.../bindings/serial/amlogic,meson-uart.txt | 38 +++++++
arch/arm/boot/dts/meson.dtsi | 8 +-
arch/arm/boot/dts/meson6.dtsi | 28 ++++++
arch/arm/boot/dts/meson8.dtsi | 23 ++++-
arch/arm/boot/dts/meson8b.dtsi | 23 ++++-
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 12 +--
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 25 +++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 25 +++++
drivers/tty/serial/meson_uart.c | 109 +++++++++++++++++++--
9 files changed, 266 insertions(+), 25 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt

--
1.9.1


2017-06-09 09:49:37

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v4 3/7] ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

From: Helmut Klein <[email protected]>

This patch switches to the stable UART bindings but also add the correct
gate clock to the non-AO UART nodes for GXBB and GXL SoCs.

Signed-off-by: Helmut Klein <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 12 +++++-------
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 25 +++++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 25 +++++++++++++++++++++++++
3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 436b875..d2b8d52 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -225,7 +225,7 @@
};

uart_A: serial@84c0 {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson-gx-uart";
reg = <0x0 0x84c0 0x0 0x14>;
interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
clocks = <&xtal>;
@@ -233,7 +233,7 @@
};

uart_B: serial@84dc {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson-gx-uart";
reg = <0x0 0x84dc 0x0 0x14>;
interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>;
clocks = <&xtal>;
@@ -279,7 +279,7 @@
};

uart_C: serial@8700 {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson-gx-uart";
reg = <0x0 0x8700 0x0 0x14>;
interrupts = <GIC_SPI 93 IRQ_TYPE_EDGE_RISING>;
clocks = <&xtal>;
@@ -366,18 +366,16 @@
};

uart_AO: serial@4c0 {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart";
reg = <0x0 0x004c0 0x0 0x14>;
interrupts = <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>;
- clocks = <&xtal>;
status = "disabled";
};

uart_AO_B: serial@4e0 {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart";
reg = <0x0 0x004e0 0x0 0x14>;
interrupts = <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>;
- clocks = <&xtal>;
status = "disabled";
};

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index dbd300f..1ae8da7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -675,6 +675,31 @@
clocks = <&clkc CLKID_SPI>;
};

+&uart_A {
+ clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO {
+ clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO_B {
+ clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+ clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+ clock-names = "xtal", "core", "baud";
+};
+
+&uart_C {
+ clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
+ clock-names = "xtal", "core", "baud";
+};
+
&vpu {
compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 4dfc22b..0c601f8 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -616,6 +616,31 @@
clocks = <&clkc CLKID_SPI>;
};

+&uart_A {
+ clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+ clock-names = "xtal", "core", "baud";
+};
+
+&uart_AO {
+ clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO_B {
+ clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+ clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+ clock-names = "xtal", "core", "baud";
+};
+
+&uart_C {
+ clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
+ clock-names = "xtal", "core", "baud";
+};
+
&vpu {
compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
};
--
1.9.1

2017-06-09 09:50:09

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v4 4/7] ARM: dts: meson: use meson6 UART compatible like other nodes

The UART bindings needs specifying a SoC family, use the meson6 family
for the UART nodes like the other nodes.

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm/boot/dts/meson.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
index 8d9c369..ecc9330 100644
--- a/arch/arm/boot/dts/meson.dtsi
+++ b/arch/arm/boot/dts/meson.dtsi
@@ -79,14 +79,14 @@
ranges = <0x0 0xc1100000 0x200000>;

uart_A: serial@84c0 {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson6-uart";
reg = <0x84c0 0x18>;
interrupts = <0 26 1>;
status = "disabled";
};

uart_B: serial@84dc {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson6-uart";
reg = <0x84dc 0x18>;
interrupts = <0 75 1>;
status = "disabled";
@@ -102,7 +102,7 @@
};

uart_C: serial@8700 {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson6-uart";
reg = <0x8700 0x18>;
interrupts = <0 93 1>;
status = "disabled";
@@ -153,7 +153,7 @@
};

uart_AO: serial@4c0 {
- compatible = "amlogic,meson-uart";
+ compatible = "amlogic,meson6-uart", "amlogic,meson-ao-uart";
reg = <0x4c0 0x18>;
interrupts = <0 90 1>;
status = "disabled";
--
1.9.1

2017-06-09 09:50:08

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v4 5/7] ARM: dts: meson6: switch to new bindings for UART nodes

Switch to the stable UART bindings by adding a XTAL node and using the
proper compatible strings.

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm/boot/dts/meson6.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/meson6.dtsi b/arch/arm/boot/dts/meson6.dtsi
index b0fc91f..a334fbe 100644
--- a/arch/arm/boot/dts/meson6.dtsi
+++ b/arch/arm/boot/dts/meson6.dtsi
@@ -70,9 +70,37 @@
};
};

+ xtal: xtal-clk {
+ compatible = "fixed-clock";
+ clock-frequency = <24000000>;
+ clock-output-names = "xtal";
+ #clock-cells = <0>;
+ };
+
clk81: clk@0 {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <200000000>;
};
}; /* end of / */
+
+
+&uart_AO {
+ clocks = <&xtal>, <&clk81>, <&clk81>;
+ clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_A {
+ clocks = <&xtal>, <&clk81>, <&clk81>;
+ clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+ clocks = <&xtal>, <&clk81>, <&clk81>;
+ clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_C {
+ clocks = <&xtal>, <&clk81>, <&clk81>;
+ clock-names = "xtal", "pclk", "baud";
+};
--
1.9.1

2017-06-09 09:50:06

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v4 6/7] ARM: dts: meson8: switch to new bindings for UART nodes

Switch to the stable UART bindings by adding a XTAL node and using the
proper compatible strings.

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm/boot/dts/meson8.dtsi | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index 6993077..a2ea112 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -83,6 +83,13 @@
};
};

+ xtal: xtal-clk {
+ compatible = "fixed-clock";
+ clock-frequency = <24000000>;
+ clock-output-names = "xtal";
+ #clock-cells = <0>;
+ };
+
clk81: clk@0 {
#clock-cells = <0>;
compatible = "fixed-clock";
@@ -199,17 +206,25 @@
};

&uart_AO {
- clocks = <&clk81>;
+ compatible = "amlogic,meson8-uart", "amlogic,meson-ao-uart";
+ clocks = <&xtal>, <&clk81>, <&clk81>;
+ clock-names = "xtal", "pclk", "baud";
};

&uart_A {
- clocks = <&clk81>;
+ compatible = "amlogic,meson8-uart";
+ clocks = <&xtal>, <&clk81>, <&clk81>;
+ clock-names = "xtal", "pclk", "baud";
};

&uart_B {
- clocks = <&clk81>;
+ compatible = "amlogic,meson8-uart";
+ clocks = <&xtal>, <&clk81>, <&clk81>;
+ clock-names = "xtal", "pclk", "baud";
};

&uart_C {
- clocks = <&clk81>;
+ compatible = "amlogic,meson8-uart";
+ clocks = <&xtal>, <&clk81>, <&clk81>;
+ clock-names = "xtal", "pclk", "baud";
};
--
1.9.1

2017-06-09 09:50:04

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v4 7/7] ARM: dts: meson8b: switch to new bindings for UART nodes

Switch to the stable UART bindings by adding a XTAL node and using the
proper compatible strings.

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm/boot/dts/meson8b.dtsi | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index d9f116a..651ad4a 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -82,6 +82,13 @@
reg = <0x203>;
};
};
+
+ xtal: xtal-clk {
+ compatible = "fixed-clock";
+ clock-frequency = <24000000>;
+ clock-output-names = "xtal";
+ #clock-cells = <0>;
+ };
}; /* end of / */

&aobus {
@@ -178,17 +185,25 @@
};

&uart_AO {
- clocks = <&clkc CLKID_CLK81>;
+ compatible = "amlogic,meson8b-uart", "amlogic,meson-ao-uart";
+ clocks = <&xtal>, <&clkc CLKID_CLK81>, <&clkc CLKID_CLK81>;
+ clock-names = "xtal", "pclk", "baud";
};

&uart_A {
- clocks = <&clkc CLKID_CLK81>;
+ compatible = "amlogic,meson8b-uart";
+ clocks = <&xtal>, <&clkc CLKID_CLK81>, <&clkc CLKID_CLK81>;
+ clock-names = "xtal", "pclk", "baud";
};

&uart_B {
- clocks = <&clkc CLKID_CLK81>;
+ compatible = "amlogic,meson8b-uart";
+ clocks = <&xtal>, <&clkc CLKID_CLK81>, <&clkc CLKID_CLK81>;
+ clock-names = "xtal", "pclk", "baud";
};

&uart_C {
- clocks = <&clkc CLKID_CLK81>;
+ compatible = "amlogic,meson8b-uart";
+ clocks = <&xtal>, <&clkc CLKID_CLK81>, <&clkc CLKID_CLK81>;
+ clock-names = "xtal", "pclk", "baud";
};
--
1.9.1

2017-06-09 09:51:04

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings

From: Helmut Klein <[email protected]>

This patch handle the stable UART bindings but also keeps compatibility
with the legacy non-stable bindings until all boards uses them.

Signed-off-by: Helmut Klein <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/tty/serial/meson_uart.c | 109 +++++++++++++++++++++++++++++++++++++---
1 file changed, 103 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 60f1679..d2c8136 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -579,8 +579,12 @@ static void meson_serial_early_console_write(struct console *co,
device->con->write = meson_serial_early_console_write;
return 0;
}
+/* Legacy bindings, should be removed when no more used */
OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
meson_serial_early_console_setup);
+/* Stable bindings */
+OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
+ meson_serial_early_console_setup);

#define MESON_SERIAL_CONSOLE (&meson_serial_console)
#else
@@ -595,11 +599,95 @@ static void meson_serial_early_console_write(struct console *co,
.cons = MESON_SERIAL_CONSOLE,
};

+/*
+ * This function gets clocks in the legacy non-stable DT bindings.
+ * This code will be remove once all the platforms switch to the
+ * new DT bindings.
+ */
+static int meson_uart_probe_clocks_legacy(struct platform_device *pdev,
+ struct uart_port *port)
+{
+ struct clk *clk = NULL;
+ int ret;
+
+ clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ ret = clk_prepare_enable(clk);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't enable clk\n");
+ return ret;
+ }
+
+ devm_add_action_or_reset(&pdev->dev,
+ (void(*)(void *))clk_disable_unprepare,
+ clk);
+
+ port->uartclk = clk_get_rate(clk);
+
+ return 0;
+}
+
+static int meson_uart_probe_clocks(struct platform_device *pdev,
+ struct uart_port *port)
+{
+ struct clk *clk_xtal = NULL;
+ struct clk *clk_pclk = NULL;
+ struct clk *clk_baud = NULL;
+ int ret;
+
+ clk_pclk = devm_clk_get(&pdev->dev, "pclk");
+ if (IS_ERR(clk_pclk))
+ return PTR_ERR(clk_pclk);
+
+ clk_xtal = devm_clk_get(&pdev->dev, "xtal");
+ if (IS_ERR(clk_xtal))
+ return PTR_ERR(clk_xtal);
+
+ clk_baud = devm_clk_get(&pdev->dev, "baud");
+ if (IS_ERR(clk_xtal))
+ return PTR_ERR(clk_baud);
+
+ ret = clk_prepare_enable(clk_pclk);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't enable pclk\n");
+ return ret;
+ }
+
+ devm_add_action_or_reset(&pdev->dev,
+ (void(*)(void *))clk_disable_unprepare,
+ clk_pclk);
+
+ ret = clk_prepare_enable(clk_xtal);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't enable xtal\n");
+ return ret;
+ }
+
+ devm_add_action_or_reset(&pdev->dev,
+ (void(*)(void *))clk_disable_unprepare,
+ clk_xtal);
+
+ ret = clk_prepare_enable(clk_baud);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't enable baud clk\n");
+ return ret;
+ }
+
+ devm_add_action_or_reset(&pdev->dev,
+ (void(*)(void *))clk_disable_unprepare,
+ clk_baud);
+
+ port->uartclk = clk_get_rate(clk_baud);
+
+ return 0;
+}
+
static int meson_uart_probe(struct platform_device *pdev)
{
struct resource *res_mem, *res_irq;
struct uart_port *port;
- struct clk *clk;
int ret = 0;

if (pdev->dev.of_node)
@@ -625,11 +713,15 @@ static int meson_uart_probe(struct platform_device *pdev)
if (!port)
return -ENOMEM;

- clk = clk_get(&pdev->dev, NULL);
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ /* Use legacy way until all platforms switch to new bindings */
+ if (of_device_is_compatible(pdev->dev.of_node, "amlogic,meson-uart"))
+ ret = meson_uart_probe_clocks_legacy(pdev, port);
+ else
+ ret = meson_uart_probe_clocks(pdev, port);
+
+ if (ret)
+ return ret;

- port->uartclk = clk_get_rate(clk);
port->iotype = UPIO_MEM;
port->mapbase = res_mem->start;
port->irq = res_irq->start;
@@ -668,9 +760,14 @@ static int meson_uart_remove(struct platform_device *pdev)
return 0;
}

-
static const struct of_device_id meson_uart_dt_match[] = {
+ /* Legacy bindings, should be removed when no more used */
{ .compatible = "amlogic,meson-uart" },
+ /* Stable bindings */
+ { .compatible = "amlogic,meson6-uart" },
+ { .compatible = "amlogic,meson8-uart" },
+ { .compatible = "amlogic,meson8b-uart" },
+ { .compatible = "amlogic,meson-gx-uart" },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
--
1.9.1

2017-06-09 09:52:06

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v4 1/7] dt-bindings: serial: Add bindings for the Amlogic Meson UARTs

From: Helmut Klein <[email protected]>

Add the documentation for the device tree binding of Amlogic Meson Serial UART.

Signed-off-by: Helmut Klein <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
.../bindings/serial/amlogic,meson-uart.txt | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt

diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt
new file mode 100644
index 0000000..8ff65fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt
@@ -0,0 +1,38 @@
+Amlogic Meson SoC UART Serial Interface
+=======================================
+
+The Amlogic Meson SoC UART Serial Interface is present on a large range
+of SoCs, and can be present either in the "Always-On" power domain or the
+"Everything-Else" power domain.
+
+The particularity of the "Always-On" Serial Interface is that the hardware
+is active since power-on and does not need any clock gating and is usable
+as very early serial console.
+
+Required properties:
+- compatible : compatible: value should be different for each SoC family as :
+ - Meson6 : "amlogic,meson6-uart"
+ - Meson8 : "amlogic,meson8-uart"
+ - Meson8b : "amlogic,meson8b-uart"
+ - GX (GXBB, GXL, GXM) : "amlogic,meson-gx-uart"
+ eventually followed by : "amlogic,meson-ao-uart" if this UART interface
+ is in the "Always-On" power domain.
+- reg : offset and length of the register set for the device.
+- interrupts : identifier to the device interrupt
+- clocks : a list of phandle + clock-specifier pairs, one for each
+ entry in clock names.
+- clocks-names :
+ * "xtal" for external xtal clock identifier
+ * "pclk" for the bus core clock, either the clk81 clock or the gate clock
+ * "baud" for the source of the baudrate generator, can be either the xtal
+ or the pclk.
+
+e.g.
+uart_A: serial@84c0 {
+ compatible = "amlogic,meson-gx-uart";
+ reg = <0x0 0x84c0 0x0 0x14>;
+ interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
+ /* Use xtal as baud rate clock source */
+ clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+};
--
1.9.1

2017-06-09 22:37:34

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v4 6/7] ARM: dts: meson8: switch to new bindings for UART nodes

Hi Neil,

On Fri, Jun 9, 2017 at 11:49 AM, Neil Armstrong <[email protected]> wrote:
> Switch to the stable UART bindings by adding a XTAL node and using the
> proper compatible strings.
unfortunately this won't apply now that Kevin has merged my "ARM: dts:
meson8: add and use the real clock controller"
on the other hand this will make the patch easier as you can now do
the same changes as in meson8b.dtsi

> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> arch/arm/boot/dts/meson8.dtsi | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
> index 6993077..a2ea112 100644
> --- a/arch/arm/boot/dts/meson8.dtsi
> +++ b/arch/arm/boot/dts/meson8.dtsi
> @@ -83,6 +83,13 @@
> };
> };
>
> + xtal: xtal-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <24000000>;
> + clock-output-names = "xtal";
> + #clock-cells = <0>;
> + };
> +
> clk81: clk@0 {
> #clock-cells = <0>;
> compatible = "fixed-clock";
> @@ -199,17 +206,25 @@
> };
>
> &uart_AO {
> - clocks = <&clk81>;
> + compatible = "amlogic,meson8-uart", "amlogic,meson-ao-uart";
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
> };
>
> &uart_A {
> - clocks = <&clk81>;
> + compatible = "amlogic,meson8-uart";
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
> };
>
> &uart_B {
> - clocks = <&clk81>;
> + compatible = "amlogic,meson8-uart";
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
> };
>
> &uart_C {
> - clocks = <&clk81>;
> + compatible = "amlogic,meson8-uart";
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
> };
> --
> 1.9.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

2017-06-11 20:19:20

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v4 6/7] ARM: dts: meson8: switch to new bindings for UART nodes

On Sat, Jun 10, 2017 at 12:37 AM, Martin Blumenstingl
<[email protected]> wrote:
> Hi Neil,
>
> On Fri, Jun 9, 2017 at 11:49 AM, Neil Armstrong <[email protected]> wrote:
>> Switch to the stable UART bindings by adding a XTAL node and using the
>> proper compatible strings.
> unfortunately this won't apply now that Kevin has merged my "ARM: dts:
> meson8: add and use the real clock controller"
> on the other hand this will make the patch easier as you can now do
> the same changes as in meson8b.dtsi
Neil, if you want you could also drop this from your series and let me
handle this (just let me know). I would even go one step further and
export the CLKID_UART0, CLKID_UART1 and CLKID_UART2 gates and pass
them as "pclk" for uart_{A,B,C}
I'll even *try* to test if this works on real hardware (my Meson8m2
board has a RTL8723BS SDIO wifi + bluetooth chip - wifi driver support
has been pretty bad so far - but I'll try to see if I can get and
messages out of the bluetooth part)
just let me know if you want me to handle this patch for you

>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> arch/arm/boot/dts/meson8.dtsi | 23 +++++++++++++++++++----
>> 1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
>> index 6993077..a2ea112 100644
>> --- a/arch/arm/boot/dts/meson8.dtsi
>> +++ b/arch/arm/boot/dts/meson8.dtsi
>> @@ -83,6 +83,13 @@
>> };
>> };
>>
>> + xtal: xtal-clk {
>> + compatible = "fixed-clock";
>> + clock-frequency = <24000000>;
>> + clock-output-names = "xtal";
>> + #clock-cells = <0>;
>> + };
>> +
>> clk81: clk@0 {
>> #clock-cells = <0>;
>> compatible = "fixed-clock";
>> @@ -199,17 +206,25 @@
>> };
>>
>> &uart_AO {
>> - clocks = <&clk81>;
>> + compatible = "amlogic,meson8-uart", "amlogic,meson-ao-uart";
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> };
>>
>> &uart_A {
>> - clocks = <&clk81>;
>> + compatible = "amlogic,meson8-uart";
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> };
>>
>> &uart_B {
>> - clocks = <&clk81>;
>> + compatible = "amlogic,meson8-uart";
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> };
>>
>> &uart_C {
>> - clocks = <&clk81>;
>> + compatible = "amlogic,meson8-uart";
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> };
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic

2017-06-12 07:27:29

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v4 6/7] ARM: dts: meson8: switch to new bindings for UART nodes

On 06/11/2017 10:18 PM, Martin Blumenstingl wrote:
> On Sat, Jun 10, 2017 at 12:37 AM, Martin Blumenstingl
> <[email protected]> wrote:
>> Hi Neil,
>>
>> On Fri, Jun 9, 2017 at 11:49 AM, Neil Armstrong <[email protected]> wrote:
>>> Switch to the stable UART bindings by adding a XTAL node and using the
>>> proper compatible strings.
>> unfortunately this won't apply now that Kevin has merged my "ARM: dts:
>> meson8: add and use the real clock controller"
>> on the other hand this will make the patch easier as you can now do
>> the same changes as in meson8b.dtsi
> Neil, if you want you could also drop this from your series and let me
> handle this (just let me know). I would even go one step further and
> export the CLKID_UART0, CLKID_UART1 and CLKID_UART2 gates and pass
> them as "pclk" for uart_{A,B,C}
> I'll even *try* to test if this works on real hardware (my Meson8m2
> board has a RTL8723BS SDIO wifi + bluetooth chip - wifi driver support
> has been pretty bad so far - but I'll try to see if I can get and
> messages out of the bluetooth part)
> just let me know if you want me to handle this patch for you


No in fact it was just a "PoC" to show how to handle it on non-gx DTS.

I'm ok to leave it to you when the UART code is merged.

Neil

>
>>> Signed-off-by: Neil Armstrong <[email protected]>
>>> ---
>>> arch/arm/boot/dts/meson8.dtsi | 23 +++++++++++++++++++----
>>> 1 file changed, 19 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
>>> index 6993077..a2ea112 100644
>>> --- a/arch/arm/boot/dts/meson8.dtsi
>>> +++ b/arch/arm/boot/dts/meson8.dtsi
>>> @@ -83,6 +83,13 @@
>>> };
>>> };
>>>
>>> + xtal: xtal-clk {
>>> + compatible = "fixed-clock";
>>> + clock-frequency = <24000000>;
>>> + clock-output-names = "xtal";
>>> + #clock-cells = <0>;
>>> + };
>>> +
>>> clk81: clk@0 {
>>> #clock-cells = <0>;
>>> compatible = "fixed-clock";
>>> @@ -199,17 +206,25 @@
>>> };
>>>
>>> &uart_AO {
>>> - clocks = <&clk81>;
>>> + compatible = "amlogic,meson8-uart", "amlogic,meson-ao-uart";
>>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>>> + clock-names = "xtal", "pclk", "baud";
>>> };
>>>
>>> &uart_A {
>>> - clocks = <&clk81>;
>>> + compatible = "amlogic,meson8-uart";
>>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>>> + clock-names = "xtal", "pclk", "baud";
>>> };
>>>
>>> &uart_B {
>>> - clocks = <&clk81>;
>>> + compatible = "amlogic,meson8-uart";
>>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>>> + clock-names = "xtal", "pclk", "baud";
>>> };
>>>
>>> &uart_C {
>>> - clocks = <&clk81>;
>>> + compatible = "amlogic,meson8-uart";
>>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>>> + clock-names = "xtal", "pclk", "baud";
>>> };
>>> --
>>> 1.9.1
>>>
>>>
>>> _______________________________________________
>>> linux-amlogic mailing list
>>> [email protected]
>>> http://lists.infradead.org/mailman/listinfo/linux-amlogic

2017-06-12 09:13:49

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v4 6/7] ARM: dts: meson8: switch to new bindings for UART nodes

On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
> Switch to the stable UART bindings by adding a XTAL node and using the
> proper compatible strings.
>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
>  arch/arm/boot/dts/meson8.dtsi | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)

I think this is clashing with the recent change from Martin on meson8 clock
driver. Kevin just applied it : 

https://lkml.kernel.org/r/[email protected]

<&clk81> no longer exists ...

>
> diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
> index 6993077..a2ea112 100644
> --- a/arch/arm/boot/dts/meson8.dtsi
> +++ b/arch/arm/boot/dts/meson8.dtsi
> @@ -83,6 +83,13 @@
>   };
>   };
>  
> + xtal: xtal-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <24000000>;
> + clock-output-names = "xtal";
> + #clock-cells = <0>;
> + };
> +
>   clk81: clk@0 {
>   #clock-cells = <0>;
>   compatible = "fixed-clock";
> @@ -199,17 +206,25 @@
>  };
>  
>  &uart_AO {
> - clocks = <&clk81>;
> + compatible = "amlogic,meson8-uart", "amlogic,meson-ao-uart";
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
>  };
>  
>  &uart_A {
> - clocks = <&clk81>;
> + compatible = "amlogic,meson8-uart";
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
>  };
>  
>  &uart_B {
> - clocks = <&clk81>;
> + compatible = "amlogic,meson8-uart";
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
>  };
>  
>  &uart_C {
> - clocks = <&clk81>;
> + compatible = "amlogic,meson8-uart";
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
>  };

2017-06-12 09:39:10

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings

On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
> From: Helmut Klein <[email protected]>
>
> This patch handle the stable UART bindings but also keeps compatibility
> with the legacy non-stable bindings until all boards uses them.
>
> Signed-off-by: Helmut Klein <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
>  drivers/tty/serial/meson_uart.c | 109 +++++++++++++++++++++++++++++++++++++
> ---
>  1 file changed, 103 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 60f1679..d2c8136 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -579,8 +579,12 @@ static void meson_serial_early_console_write(struct
> console *co,
>   device->con->write = meson_serial_early_console_write;
>   return 0;
>  }
> +/* Legacy bindings, should be removed when no more used */
>  OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
>       meson_serial_early_console_setup);
> +/* Stable bindings */
> +OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
> +     meson_serial_early_console_setup);
>  
>  #define MESON_SERIAL_CONSOLE (&meson_serial_console)
>  #else
> @@ -595,11 +599,95 @@ static void meson_serial_early_console_write(struct
> console *co,
>   .cons = MESON_SERIAL_CONSOLE,
>  };
>  
> +/*
> + * This function gets clocks in the legacy non-stable DT bindings.
> + * This code will be remove once all the platforms switch to the
> + * new DT bindings.
> + */
> +static int meson_uart_probe_clocks_legacy(struct platform_device *pdev,
> +   struct uart_port *port)
> +{
> + struct clk *clk = NULL;
> + int ret;
> +
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(&pdev->dev, "couldn't enable clk\n");
> + return ret;
> + }
> +
> + devm_add_action_or_reset(&pdev->dev,
> + (void(*)(void *))clk_disable_unprepare,
> + clk);
> +
> + port->uartclk = clk_get_rate(clk);
> +
> + return 0;
> +}
> +
> +static int meson_uart_probe_clocks(struct platform_device *pdev,
> +    struct uart_port *port)
> +{
> + struct clk *clk_xtal = NULL;
> + struct clk *clk_pclk = NULL;
> + struct clk *clk_baud = NULL;
> + int ret;
> +
> + clk_pclk = devm_clk_get(&pdev->dev, "pclk");
> + if (IS_ERR(clk_pclk))
> + return PTR_ERR(clk_pclk);
> +
> + clk_xtal = devm_clk_get(&pdev->dev, "xtal");
> + if (IS_ERR(clk_xtal))
> + return PTR_ERR(clk_xtal);
> +
> + clk_baud = devm_clk_get(&pdev->dev, "baud");
> + if (IS_ERR(clk_xtal))
> + return PTR_ERR(clk_baud);
> +
> + ret = clk_prepare_enable(clk_pclk);
> + if (ret) {
> + dev_err(&pdev->dev, "couldn't enable pclk\n");
> + return ret;
> + }
> +
> + devm_add_action_or_reset(&pdev->dev,
> + (void(*)(void *))clk_disable_unprepare,
> + clk_pclk);
> +
> + ret = clk_prepare_enable(clk_xtal);
> + if (ret) {
> + dev_err(&pdev->dev, "couldn't enable xtal\n");
> + return ret;
> + }
> +
> + devm_add_action_or_reset(&pdev->dev,
> + (void(*)(void *))clk_disable_unprepare,
> + clk_xtal);
> +
> + ret = clk_prepare_enable(clk_baud);
> + if (ret) {
> + dev_err(&pdev->dev, "couldn't enable baud clk\n");
> + return ret;
> + }
> +
> + devm_add_action_or_reset(&pdev->dev,
> + (void(*)(void *))clk_disable_unprepare,
> + clk_baud);

It's not critical but there is a lot of duplication here. Should we add an
helper function doing "get, prepare_enable, add_reset_action" with the clock
name as argument ?

Apart from this:

Reviewed-by: Jerome Brunet <[email protected]>

> +
> + port->uartclk = clk_get_rate(clk_baud);

This was already like this, but I wonder if we should store the *clk instead of
caching the rate. Then call get_rate when appropriate

Could be done in separate patch.


> +
> + return 0;
> +}
> +
>  static int meson_uart_probe(struct platform_device *pdev)
>  {
>   struct resource *res_mem, *res_irq;
>   struct uart_port *port;
> - struct clk *clk;
>   int ret = 0;
>  
>   if (pdev->dev.of_node)
> @@ -625,11 +713,15 @@ static int meson_uart_probe(struct platform_device
> *pdev)
>   if (!port)
>   return -ENOMEM;
>  
> - clk = clk_get(&pdev->dev, NULL);
> - if (IS_ERR(clk))
> - return PTR_ERR(clk);
> + /* Use legacy way until all platforms switch to new bindings */
> + if (of_device_is_compatible(pdev->dev.of_node, "amlogic,meson-uart"))
> + ret = meson_uart_probe_clocks_legacy(pdev, port);
> + else
> + ret = meson_uart_probe_clocks(pdev, port);
> +
> + if (ret)
> + return ret;
>  
> - port->uartclk = clk_get_rate(clk);
>   port->iotype = UPIO_MEM;
>   port->mapbase = res_mem->start;
>   port->irq = res_irq->start;
> @@ -668,9 +760,14 @@ static int meson_uart_remove(struct platform_device
> *pdev)
>   return 0;
>  }
>  
> -
>  static const struct of_device_id meson_uart_dt_match[] = {
> + /* Legacy bindings, should be removed when no more used */
>   { .compatible = "amlogic,meson-uart" },
> + /* Stable bindings */
> + { .compatible = "amlogic,meson6-uart" },
> + { .compatible = "amlogic,meson8-uart" },
> + { .compatible = "amlogic,meson8b-uart" },
> + { .compatible = "amlogic,meson-gx-uart" },
>   { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, meson_uart_dt_match);

2017-06-12 09:40:21

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v4 3/7] ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
> From: Helmut Klein <[email protected]>
>
> This patch switches to the stable UART bindings but also add the correct
> gate clock to the non-AO UART nodes for GXBB and GXL SoCs.
>
> Signed-off-by: Helmut Klein <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>

Looks Good

Acked-by: Jerome Brunet <[email protected]>

> ---
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi   | 12 +++++-------
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 25 +++++++++++++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 25 +++++++++++++++++++++++++
>  3 files changed, 55 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> index 436b875..d2b8d52 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -225,7 +225,7 @@
>   };
>  
>   uart_A: serial@84c0 {
> - compatible = "amlogic,meson-uart";
> + compatible = "amlogic,meson-gx-uart";
>   reg = <0x0 0x84c0 0x0 0x14>;
>   interrupts = <GIC_SPI 26
> IRQ_TYPE_EDGE_RISING>;
>   clocks = <&xtal>;
> @@ -233,7 +233,7 @@
>   };
>  
>   uart_B: serial@84dc {
> - compatible = "amlogic,meson-uart";
> + compatible = "amlogic,meson-gx-uart";
>   reg = <0x0 0x84dc 0x0 0x14>;
>   interrupts = <GIC_SPI 75
> IRQ_TYPE_EDGE_RISING>;
>   clocks = <&xtal>;
> @@ -279,7 +279,7 @@
>   };
>  
>   uart_C: serial@8700 {
> - compatible = "amlogic,meson-uart";
> + compatible = "amlogic,meson-gx-uart";
>   reg = <0x0 0x8700 0x0 0x14>;
>   interrupts = <GIC_SPI 93
> IRQ_TYPE_EDGE_RISING>;
>   clocks = <&xtal>;
> @@ -366,18 +366,16 @@
>   };
>  
>   uart_AO: serial@4c0 {
> - compatible = "amlogic,meson-uart";
> + compatible = "amlogic,meson-gx-uart",
> "amlogic,meson-ao-uart";
>   reg = <0x0 0x004c0 0x0 0x14>;
>   interrupts = <GIC_SPI 193
> IRQ_TYPE_EDGE_RISING>;
> - clocks = <&xtal>;
>   status = "disabled";
>   };
>  
>   uart_AO_B: serial@4e0 {
> - compatible = "amlogic,meson-uart";
> + compatible = "amlogic,meson-gx-uart",
> "amlogic,meson-ao-uart";
>   reg = <0x0 0x004e0 0x0 0x14>;
>   interrupts = <GIC_SPI 197
> IRQ_TYPE_EDGE_RISING>;
> - clocks = <&xtal>;
>   status = "disabled";
>   };
>  
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> index dbd300f..1ae8da7 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> @@ -675,6 +675,31 @@
>   clocks = <&clkc CLKID_SPI>;
>  };
>  
> +&uart_A {
> + clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_AO {
> + clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_AO_B {
> + clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_B {
> + clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
> + clock-names = "xtal", "core", "baud";
> +};
> +
> +&uart_C {
> + clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
> + clock-names = "xtal", "core", "baud";
> +};
> +
>  &vpu {
>   compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
>  };
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> index 4dfc22b..0c601f8 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> @@ -616,6 +616,31 @@
>   clocks = <&clkc CLKID_SPI>;
>  };
>  
> +&uart_A {
> + clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
> + clock-names = "xtal", "core", "baud";
> +};
> +
> +&uart_AO {
> + clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_AO_B {
> + clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_B {
> + clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
> + clock-names = "xtal", "core", "baud";
> +};
> +
> +&uart_C {
> + clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
> + clock-names = "xtal", "core", "baud";
> +};
> +
>  &vpu {
>   compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
>  };

2017-06-12 09:43:02

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v4 5/7] ARM: dts: meson6: switch to new bindings for UART nodes

On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
> Switch to the stable UART bindings by adding a XTAL node and using the
> proper compatible strings.
>
> Signed-off-by: Neil Armstrong <[email protected]>

Shouldn't this patch be squashed with patch 4 ?


> ---
>  arch/arm/boot/dts/meson6.dtsi | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/arch/arm/boot/dts/meson6.dtsi b/arch/arm/boot/dts/meson6.dtsi
> index b0fc91f..a334fbe 100644
> --- a/arch/arm/boot/dts/meson6.dtsi
> +++ b/arch/arm/boot/dts/meson6.dtsi
> @@ -70,9 +70,37 @@
>   };
>   };
>  
> + xtal: xtal-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <24000000>;
> + clock-output-names = "xtal";
> + #clock-cells = <0>;
> + };
> +
>   clk81: clk@0 {
>   #clock-cells = <0>;
>   compatible = "fixed-clock";
>   clock-frequency = <200000000>;
>   };
>  }; /* end of / */
> +
> +
> +&uart_AO {
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_A {
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_B {
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_C {
> + clocks = <&xtal>, <&clk81>, <&clk81>;
> + clock-names = "xtal", "pclk", "baud";
> +};

2017-06-12 09:48:04

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v4 7/7] ARM: dts: meson8b: switch to new bindings for UART nodes

On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
> Switch to the stable UART bindings by adding a XTAL node and using the
> proper compatible strings.
>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
>  arch/arm/boot/dts/meson8b.dtsi | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
> index d9f116a..651ad4a 100644
> --- a/arch/arm/boot/dts/meson8b.dtsi
> +++ b/arch/arm/boot/dts/meson8b.dtsi
> @@ -82,6 +82,13 @@
>   reg = <0x203>;
>   };
>   };
> +
> + xtal: xtal-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <24000000>;
> + clock-output-names = "xtal";
> + #clock-cells = <0>;
> + };

On meson8b, I think xtal is already provided by <&clkc CLKID_XTAL>.
We have to choose which one we want.

With this handled:

Acked-by: Jerome Brunet <[email protected]>

>  }; /* end of / */
>  
>  &aobus {
> @@ -178,17 +185,25 @@
>  };
>  
>  &uart_AO {
> - clocks = <&clkc CLKID_CLK81>;
> + compatible = "amlogic,meson8b-uart", "amlogic,meson-ao-uart";
> + clocks = <&xtal>, <&clkc CLKID_CLK81>, <&clkc CLKID_CLK81>;
> + clock-names = "xtal", "pclk", "baud";
>  };
>  
>  &uart_A {
> - clocks = <&clkc CLKID_CLK81>;
> + compatible = "amlogic,meson8b-uart";
> + clocks = <&xtal>, <&clkc CLKID_CLK81>, <&clkc CLKID_CLK81>;
> + clock-names = "xtal", "pclk", "baud";
>  };
>  
>  &uart_B {
> - clocks = <&clkc CLKID_CLK81>;
> + compatible = "amlogic,meson8b-uart";
> + clocks = <&xtal>, <&clkc CLKID_CLK81>, <&clkc CLKID_CLK81>;
> + clock-names = "xtal", "pclk", "baud";
>  };
>  
>  &uart_C {
> - clocks = <&clkc CLKID_CLK81>;
> + compatible = "amlogic,meson8b-uart";
> + clocks = <&xtal>, <&clkc CLKID_CLK81>, <&clkc CLKID_CLK81>;
> + clock-names = "xtal", "pclk", "baud";
>  };

2017-06-12 12:45:12

by Chris Moore

[permalink] [raw]
Subject: Re: [PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings

Le 09/06/2017 à 11:49, Neil Armstrong a écrit :
> From: Helmut Klein <[email protected]>
>
> This patch handle the stable UART bindings but also keeps compatibility
> with the legacy non-stable bindings until all boards uses them.
>
> Signed-off-by: Helmut Klein <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> drivers/tty/serial/meson_uart.c | 109 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 103 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 60f1679..d2c8136 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c

[snip]

> +static int meson_uart_probe_clocks(struct platform_device *pdev,
> + struct uart_port *port)
> +{
> + struct clk *clk_xtal = NULL;
> + struct clk *clk_pclk = NULL;
> + struct clk *clk_baud = NULL;
> + int ret;
> +
> + clk_pclk = devm_clk_get(&pdev->dev, "pclk");
> + if (IS_ERR(clk_pclk))
> + return PTR_ERR(clk_pclk);
> +
> + clk_xtal = devm_clk_get(&pdev->dev, "xtal");
> + if (IS_ERR(clk_xtal))
> + return PTR_ERR(clk_xtal);
> +
> + clk_baud = devm_clk_get(&pdev->dev, "baud");
> + if (IS_ERR(clk_xtal))

Copy/paste error: s/clk_xtal/clk_baud/

> + return PTR_ERR(clk_baud);

2017-06-12 12:48:08

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings

On 06/12/2017 11:39 AM, Jerome Brunet wrote:
> On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
>> From: Helmut Klein <[email protected]>
>>
>> This patch handle the stable UART bindings but also keeps compatibility
>> with the legacy non-stable bindings until all boards uses them.
>>
>> Signed-off-by: Helmut Klein <[email protected]>
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> drivers/tty/serial/meson_uart.c | 109 +++++++++++++++++++++++++++++++++++++
>> ---
>> 1 file changed, 103 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>> index 60f1679..d2c8136 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -579,8 +579,12 @@ static void meson_serial_early_console_write(struct
>> console *co,
>> device->con->write = meson_serial_early_console_write;
>> return 0;
>> }
>> +/* Legacy bindings, should be removed when no more used */
>> OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
>> meson_serial_early_console_setup);
>> +/* Stable bindings */
>> +OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
>> + meson_serial_early_console_setup);
>>
>> #define MESON_SERIAL_CONSOLE (&meson_serial_console)
>> #else
>> @@ -595,11 +599,95 @@ static void meson_serial_early_console_write(struct
>> console *co,
>> .cons = MESON_SERIAL_CONSOLE,
>> };
>>
>> +/*
>> + * This function gets clocks in the legacy non-stable DT bindings.
>> + * This code will be remove once all the platforms switch to the
>> + * new DT bindings.
>> + */
>> +static int meson_uart_probe_clocks_legacy(struct platform_device *pdev,
>> + struct uart_port *port)
>> +{
>> + struct clk *clk = NULL;
>> + int ret;
>> +
>> + clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(clk))
>> + return PTR_ERR(clk);
>> +
>> + ret = clk_prepare_enable(clk);
>> + if (ret) {
>> + dev_err(&pdev->dev, "couldn't enable clk\n");
>> + return ret;
>> + }
>> +
>> + devm_add_action_or_reset(&pdev->dev,
>> + (void(*)(void *))clk_disable_unprepare,
>> + clk);
>> +
>> + port->uartclk = clk_get_rate(clk);
>> +
>> + return 0;
>> +}
>> +
>> +static int meson_uart_probe_clocks(struct platform_device *pdev,
>> + struct uart_port *port)
>> +{
>> + struct clk *clk_xtal = NULL;
>> + struct clk *clk_pclk = NULL;
>> + struct clk *clk_baud = NULL;
>> + int ret;
>> +
>> + clk_pclk = devm_clk_get(&pdev->dev, "pclk");
>> + if (IS_ERR(clk_pclk))
>> + return PTR_ERR(clk_pclk);
>> +
>> + clk_xtal = devm_clk_get(&pdev->dev, "xtal");
>> + if (IS_ERR(clk_xtal))
>> + return PTR_ERR(clk_xtal);
>> +
>> + clk_baud = devm_clk_get(&pdev->dev, "baud");
>> + if (IS_ERR(clk_xtal))
>> + return PTR_ERR(clk_baud);
>> +
>> + ret = clk_prepare_enable(clk_pclk);
>> + if (ret) {
>> + dev_err(&pdev->dev, "couldn't enable pclk\n");
>> + return ret;
>> + }
>> +
>> + devm_add_action_or_reset(&pdev->dev,
>> + (void(*)(void *))clk_disable_unprepare,
>> + clk_pclk);
>> +
>> + ret = clk_prepare_enable(clk_xtal);
>> + if (ret) {
>> + dev_err(&pdev->dev, "couldn't enable xtal\n");
>> + return ret;
>> + }
>> +
>> + devm_add_action_or_reset(&pdev->dev,
>> + (void(*)(void *))clk_disable_unprepare,
>> + clk_xtal);
>> +
>> + ret = clk_prepare_enable(clk_baud);
>> + if (ret) {
>> + dev_err(&pdev->dev, "couldn't enable baud clk\n");
>> + return ret;
>> + }
>> +
>> + devm_add_action_or_reset(&pdev->dev,
>> + (void(*)(void *))clk_disable_unprepare,
>> + clk_baud);
>
> It's not critical but there is a lot of duplication here. Should we add an
> helper function doing "get, prepare_enable, add_reset_action" with the clock
> name as argument ?

Yep, will do.

>
> Apart from this:
>
> Reviewed-by: Jerome Brunet <[email protected]>
>
>> +
>> + port->uartclk = clk_get_rate(clk_baud);
>
> This was already like this, but I wonder if we should store the *clk instead of
> caching the rate. Then call get_rate when appropriate
>
> Could be done in separate patch.

Well, this is the limit of the TTY framework, I'll leave it like this until we find
a good way to pass a context structure with the proper baud clock pointer and
don't rely anymore on the tty structure for this.

>
>> +
>> + return 0;
>> +}
>> +
>> static int meson_uart_probe(struct platform_device *pdev)
>> {
>> struct resource *res_mem, *res_irq;
>> struct uart_port *port;
>> - struct clk *clk;
>> int ret = 0;
>>
>> if (pdev->dev.of_node)
>> @@ -625,11 +713,15 @@ static int meson_uart_probe(struct platform_device
>> *pdev)
>> if (!port)
>> return -ENOMEM;
>>
>> - clk = clk_get(&pdev->dev, NULL);
>> - if (IS_ERR(clk))
>> - return PTR_ERR(clk);
>> + /* Use legacy way until all platforms switch to new bindings */
>> + if (of_device_is_compatible(pdev->dev.of_node, "amlogic,meson-uart"))
>> + ret = meson_uart_probe_clocks_legacy(pdev, port);
>> + else
>> + ret = meson_uart_probe_clocks(pdev, port);
>> +
>> + if (ret)
>> + return ret;
>>
>> - port->uartclk = clk_get_rate(clk);
>> port->iotype = UPIO_MEM;
>> port->mapbase = res_mem->start;
>> port->irq = res_irq->start;
>> @@ -668,9 +760,14 @@ static int meson_uart_remove(struct platform_device
>> *pdev)
>> return 0;
>> }
>>
>> -
>> static const struct of_device_id meson_uart_dt_match[] = {
>> + /* Legacy bindings, should be removed when no more used */
>> { .compatible = "amlogic,meson-uart" },
>> + /* Stable bindings */
>> + { .compatible = "amlogic,meson6-uart" },
>> + { .compatible = "amlogic,meson8-uart" },
>> + { .compatible = "amlogic,meson8b-uart" },
>> + { .compatible = "amlogic,meson-gx-uart" },
>> { /* sentinel */ },
>> };
>> MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
>

Thanks,
Neil

2017-06-12 12:48:24

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings

On 06/12/2017 02:45 PM, Chris Moore wrote:
> Le 09/06/2017 à 11:49, Neil Armstrong a écrit :
>> From: Helmut Klein <[email protected]>
>>
>> This patch handle the stable UART bindings but also keeps compatibility
>> with the legacy non-stable bindings until all boards uses them.
>>
>> Signed-off-by: Helmut Klein <[email protected]>
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> drivers/tty/serial/meson_uart.c | 109 +++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 103 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>> index 60f1679..d2c8136 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>
> [snip]
>
>> +static int meson_uart_probe_clocks(struct platform_device *pdev,
>> + struct uart_port *port)
>> +{
>> + struct clk *clk_xtal = NULL;
>> + struct clk *clk_pclk = NULL;
>> + struct clk *clk_baud = NULL;
>> + int ret;
>> +
>> + clk_pclk = devm_clk_get(&pdev->dev, "pclk");
>> + if (IS_ERR(clk_pclk))
>> + return PTR_ERR(clk_pclk);
>> +
>> + clk_xtal = devm_clk_get(&pdev->dev, "xtal");
>> + if (IS_ERR(clk_xtal))
>> + return PTR_ERR(clk_xtal);
>> +
>> + clk_baud = devm_clk_get(&pdev->dev, "baud");
>> + if (IS_ERR(clk_xtal))
>
> Copy/paste error: s/clk_xtal/clk_baud/
>
>> + return PTR_ERR(clk_baud);
>

Good catch,
Thanks !
Neil

2017-06-12 12:49:19

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v4 5/7] ARM: dts: meson6: switch to new bindings for UART nodes

On 06/12/2017 11:42 AM, Jerome Brunet wrote:
> On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
>> Switch to the stable UART bindings by adding a XTAL node and using the
>> proper compatible strings.
>>
>> Signed-off-by: Neil Armstrong <[email protected]>
>
> Shouldn't this patch be squashed with patch 4 ?
>

Yep, so it doesn't break bisect.

Neil

>
>> ---
>> arch/arm/boot/dts/meson6.dtsi | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/meson6.dtsi b/arch/arm/boot/dts/meson6.dtsi
>> index b0fc91f..a334fbe 100644
>> --- a/arch/arm/boot/dts/meson6.dtsi
>> +++ b/arch/arm/boot/dts/meson6.dtsi
>> @@ -70,9 +70,37 @@
>> };
>> };
>>
>> + xtal: xtal-clk {
>> + compatible = "fixed-clock";
>> + clock-frequency = <24000000>;
>> + clock-output-names = "xtal";
>> + #clock-cells = <0>;
>> + };
>> +
>> clk81: clk@0 {
>> #clock-cells = <0>;
>> compatible = "fixed-clock";
>> clock-frequency = <200000000>;
>> };
>> }; /* end of / */
>> +
>> +
>> +&uart_AO {
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> +};
>> +
>> +&uart_A {
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> +};
>> +
>> +&uart_B {
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> +};
>> +
>> +&uart_C {
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> +};
>

2017-06-12 12:49:50

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v4 6/7] ARM: dts: meson8: switch to new bindings for UART nodes

On 06/12/2017 11:13 AM, Jerome Brunet wrote:
> On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
>> Switch to the stable UART bindings by adding a XTAL node and using the
>> proper compatible strings.
>>
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> arch/arm/boot/dts/meson8.dtsi | 23 +++++++++++++++++++----
>> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> I think this is clashing with the recent change from Martin on meson8 clock
> driver. Kevin just applied it :
>
> https://lkml.kernel.org/r/[email protected]
>
> <&clk81> no longer exists ...
>
>>
>> diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
>> index 6993077..a2ea112 100644
>> --- a/arch/arm/boot/dts/meson8.dtsi
>> +++ b/arch/arm/boot/dts/meson8.dtsi
>> @@ -83,6 +83,13 @@
>> };
>> };
>>
>> + xtal: xtal-clk {
>> + compatible = "fixed-clock";
>> + clock-frequency = <24000000>;
>> + clock-output-names = "xtal";
>> + #clock-cells = <0>;
>> + };
>> +
>> clk81: clk@0 {
>> #clock-cells = <0>;
>> compatible = "fixed-clock";
>> @@ -199,17 +206,25 @@
>> };
>>
>> &uart_AO {
>> - clocks = <&clk81>;
>> + compatible = "amlogic,meson8-uart", "amlogic,meson-ao-uart";
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> };
>>
>> &uart_A {
>> - clocks = <&clk81>;
>> + compatible = "amlogic,meson8-uart";
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> };
>>
>> &uart_B {
>> - clocks = <&clk81>;
>> + compatible = "amlogic,meson8-uart";
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> };
>>
>> &uart_C {
>> - clocks = <&clk81>;
>> + compatible = "amlogic,meson8-uart";
>> + clocks = <&xtal>, <&clk81>, <&clk81>;
>> + clock-names = "xtal", "pclk", "baud";
>> };
>

I will drop this until martin has a better version, and tested !

Neil