2017-03-31 16:54:49

by Helmut Klein

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

To be able to use the three none AO uarts of the meson gx SoCs (uart_A,
uart_B & uart_C), the core clock has to be enabled (see chapter 22.3 of
the public s905 data sheet).
At least the u-boot of my s905 based media player (netxeon minimx-g)
doesn't do this. so the driver must enable the clock.

This patch set does:
- exposes the UART clock ids to the dtb
- adds documentation for the dt-bindings of meson_uart
- adds the core clock handling to the driver
- adds the core clock handling to meson-gxbb.dtsi and meson-gxl.dtsi

The patchset is based on the branch "master" of the repository in [1]

Changes since v2
- mail subjects reworked
- add clocks/clock-names to the documentation
- add core clock handling to meson-gxbb.dtsi & meson-gxl.dtsi

Changes since v1
- use git to produce the patch set
- added the clock ids for uart_B and uart_C

[1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git

Helmut Klein (4):
clk: meson: gxbb: expose CLKID_UARTx
dt-bindings: meson_uart: add documentation for meson UARTs
tty/serial: meson_uart: add the core clock handling to the driver
ARM64: dts: meson-gx: add core clock support for uart_A, uart_B and
uart_C

.../bindings/serial/amlogic,meson_uart.txt | 30 ++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 15 +++++++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 15 +++++++++++
drivers/clk/meson/gxbb.h | 6 ++---
drivers/tty/serial/meson_uart.c | 10 ++++++++
include/dt-bindings/clock/gxbb-clkc.h | 3 +++
6 files changed, 76 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/amlogic,meson_uart.txt

--
2.11.0


2017-03-31 16:54:52

by Helmut Klein

[permalink] [raw]
Subject: [PATCH v3 2/4] dt-binding: meson_uart: add documentation for the UARTs of amlogic

Add the documentation for the device tree binding of meson_uart

Signed-off-by: Helmut Klein <[email protected]>
---
.../bindings/serial/amlogic,meson_uart.txt | 30 ++++++++++++++++++++++
1 file changed, 30 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 000000000000..8250a191c2fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson_uart.txt
@@ -0,0 +1,30 @@
+* Amlogic Meson UART, used in multiple SoCs (e.g. S905, s905X, ...)
+
+Required properties:
+- compatible : "amlogic,meson-uart"
+- reg : offset and length of the register set for the device.
+- interrupts : device interrupt
+- clocks : a list of phandle + clock-specifier pairs, one for each
+ entry in clock names.
+- clocks-names : contains:
+ * "xtal" for the baud rate clock
+ * "core" for the core clock of none AO UARTs (optional)
+
+e.g.
+uart_A: serial@84c0 {
+ compatible = "amlogic,meson-uart";
+ reg = <0x0 0x84c0 0x0 0x14>;
+ pinctrl-0 = <&uart_a_pins &uart_a_cts_rts_pins>;
+ interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xtal>, <&clkc CLKID_UART0>;
+ clock-names = "xtal", "core";
+ status = "ok";
+};
+
+Note: Each port should have an alias correctly numbered in "aliases" node.
+
+e.g.
+aliases {
+ serial0 = &uart_AO;
+ serial1 = &uart_A;
+};
--
2.11.0

2017-03-31 16:54:55

by Helmut Klein

[permalink] [raw]
Subject: [PATCH v3 4/4] ARM64: dts: meson-gx: add core clock support for uart_A, uart_B and uart_C

This patch adds the core clock support for the three none AO UARTs
of the meson gxbb & meson gxl SoCs

Signed-off-by: Helmut Klein <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 15 +++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 15 +++++++++++++++
2 files changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 04b3324bc132..aa2f75d5cd37 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -524,3 +524,18 @@
&vpu {
compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
};
+
+&uart_A {
+ clocks = <&xtal>, <&clkc CLKID_UART0>;
+ clock-names = "xtal", "core";
+};
+
+&uart_B {
+ clocks = <&xtal>, <&clkc CLKID_UART1>;
+ clock-names = "xtal", "core";
+};
+
+&uart_C {
+ clocks = <&xtal>, <&clkc CLKID_UART2>;
+ clock-names = "xtal", "core";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index fe11b5fc61f7..f3cc18fd624a 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -381,3 +381,18 @@
&vpu {
compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
};
+
+&uart_A {
+ clocks = <&xtal>, <&clkc CLKID_UART0>;
+ clock-names = "xtal", "core";
+};
+
+&uart_B {
+ clocks = <&xtal>, <&clkc CLKID_UART1>;
+ clock-names = "xtal", "core";
+};
+
+&uart_C {
+ clocks = <&xtal>, <&clkc CLKID_UART2>;
+ clock-names = "xtal", "core";
+};
--
2.11.0

2017-03-31 16:55:09

by Helmut Klein

[permalink] [raw]
Subject: [PATCH v3 3/4] tty/serial: meson_uart: add the core clock handling to the driver

This patch gets the core clock as provided by the DT and enables it.
The code was taken from Amlogic's serial driver, and was tested on my
board.

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

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 60f16795d16b..cb99112288eb 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -600,6 +600,7 @@ static int meson_uart_probe(struct platform_device *pdev)
struct resource *res_mem, *res_irq;
struct uart_port *port;
struct clk *clk;
+ struct clk *core_clk;
int ret = 0;

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

+ core_clk = devm_clk_get(&pdev->dev, "core");
+ if (!IS_ERR(core_clk)) {
+ ret = clk_prepare_enable(core_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't enable clkc\n");
+ return ret;
+ }
+ }
+
clk = clk_get(&pdev->dev, NULL);
if (IS_ERR(clk))
return PTR_ERR(clk);
--
2.11.0

2017-03-31 16:55:34

by Helmut Klein

[permalink] [raw]
Subject: [PATCH v3 1/4] dt-bindings: clock: gxbb: expose UART clocks

Expose the clock ids of the three none AO uarts to the dt-bindings

Signed-off-by: Helmut Klein <[email protected]>
---
drivers/clk/meson/gxbb.h | 6 +++---
include/dt-bindings/clock/gxbb-clkc.h | 3 +++
2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/meson/gxbb.h b/drivers/clk/meson/gxbb.h
index 8ee2022ce5d5..1edfaa5fe307 100644
--- a/drivers/clk/meson/gxbb.h
+++ b/drivers/clk/meson/gxbb.h
@@ -194,7 +194,7 @@
/* #define CLKID_SAR_ADC */
#define CLKID_SMART_CARD 24
#define CLKID_RNG0 25
-#define CLKID_UART0 26
+/* CLKID_UART0 */
#define CLKID_SDHC 27
#define CLKID_STREAM 28
#define CLKID_ASYNC_FIFO 29
@@ -216,7 +216,7 @@
#define CLKID_ADC 45
#define CLKID_BLKMV 46
#define CLKID_AIU 47
-#define CLKID_UART1 48
+/* CLKID_UART1 */
#define CLKID_G2D 49
/* CLKID_USB0 */
/* CLKID_USB1 */
@@ -236,7 +236,7 @@
/* CLKID_USB0_DDR_BRIDGE */
#define CLKID_MMC_PCLK 66
#define CLKID_DVIN 67
-#define CLKID_UART2 68
+/* CLKID_UART2 */
/* #define CLKID_SANA */
#define CLKID_VPU_INTR 70
#define CLKID_SEC_AHB_AHB3_BRIDGE 71
diff --git a/include/dt-bindings/clock/gxbb-clkc.h b/include/dt-bindings/clock/gxbb-clkc.h
index 692846c7941b..7b329df47752 100644
--- a/include/dt-bindings/clock/gxbb-clkc.h
+++ b/include/dt-bindings/clock/gxbb-clkc.h
@@ -15,13 +15,16 @@
#define CLKID_SPI 34
#define CLKID_I2C 22
#define CLKID_SAR_ADC 23
+#define CLKID_UART0 26
#define CLKID_ETH 36
+#define CLKID_UART1 48
#define CLKID_USB0 50
#define CLKID_USB1 51
#define CLKID_USB 55
#define CLKID_HDMI_PCLK 63
#define CLKID_USB1_DDR_BRIDGE 64
#define CLKID_USB0_DDR_BRIDGE 65
+#define CLKID_UART2 68
#define CLKID_SANA 69
#define CLKID_GCLK_VENCI_INT0 77
#define CLKID_AO_I2C 93
--
2.11.0

2017-04-03 14:58:05

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] tty/serial: meson_uart: add the core clock handling to the driver

On Fri, 2017-03-31 at 18:54 +0200, Helmut Klein wrote:
> This patch gets the core clock as provided by the DT and enables it.
> The code was taken from Amlogic's serial driver, and was tested on my
> board.
>
> Signed-off-by: Helmut Klein <[email protected]>
> ---
>  drivers/tty/serial/meson_uart.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 60f16795d16b..cb99112288eb 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -600,6 +600,7 @@ static int meson_uart_probe(struct platform_device *pdev)
>   struct resource *res_mem, *res_irq;
>   struct uart_port *port;
>   struct clk *clk;
> + struct clk *core_clk;
>   int ret = 0;
>
>   if (pdev->dev.of_node)
> @@ -625,6 +626,15 @@ static int meson_uart_probe(struct platform_device *pdev)
>   if (!port)
>   return -ENOMEM;
>
> + core_clk = devm_clk_get(&pdev->dev, "core");
> + if (!IS_ERR(core_clk)) {
> + ret = clk_prepare_enable(core_clk);

This needs to be balanced with a clk_disable_unprepare() in remove.

You could try play with devm_add_action_or_reset, maybe like this:

devm_add_action_or_reset(dev,
(void(*)(void *))clk_disable_unprepare,
core_clk);

Sorry I did not notice it on the v2.


> + if (ret) {
> + dev_err(&pdev->dev, "couldn't enable clkc\n");
> + return ret;
> + }
> + }
> +
>   clk = clk_get(&pdev->dev, NULL);

I still think you should name this one. Otherwise, what the non AO UART will get
here will depends on the order it was declared in DT.

To answer your question from the v2, yes I think it is ok to add clock-names to
the AO-UART. You are doing it for non AO ones so, why not ?


>   if (IS_ERR(clk))
>   return PTR_ERR(clk);
> --
> 2.11.0
>

2017-04-03 15:31:59

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] tty/serial: meson_uart: add the core clock handling to the driver

On Mon, Apr 3, 2017 at 7:57 AM, Jerome Brunet <[email protected]> wrote:
> On Fri, 2017-03-31 at 18:54 +0200, Helmut Klein wrote:
>> This patch gets the core clock as provided by the DT and enables it.
>> The code was taken from Amlogic's serial driver, and was tested on my
>> board.
>>
>> Signed-off-by: Helmut Klein <[email protected]>
>> ---
>> drivers/tty/serial/meson_uart.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>> index 60f16795d16b..cb99112288eb 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -600,6 +600,7 @@ static int meson_uart_probe(struct platform_device *pdev)
>> struct resource *res_mem, *res_irq;
>> struct uart_port *port;
>> struct clk *clk;
>> + struct clk *core_clk;
>> int ret = 0;
>>
>> if (pdev->dev.of_node)
>> @@ -625,6 +626,15 @@ static int meson_uart_probe(struct platform_device *pdev)
>> if (!port)
>> return -ENOMEM;
>>
>> + core_clk = devm_clk_get(&pdev->dev, "core");
>> + if (!IS_ERR(core_clk)) {
>> + ret = clk_prepare_enable(core_clk);
>
> This needs to be balanced with a clk_disable_unprepare() in remove.
>
> You could try play with devm_add_action_or_reset, maybe like this:
>
> devm_add_action_or_reset(dev,
> (void(*)(void *))clk_disable_unprepare,
> core_clk);
>
> Sorry I did not notice it on the v2.
>
>
>> + if (ret) {
>> + dev_err(&pdev->dev, "couldn't enable clkc\n");
>> + return ret;
>> + }
>> + }
>> +
>> clk = clk_get(&pdev->dev, NULL);
>
> I still think you should name this one. Otherwise, what the non AO UART will get
> here will depends on the order it was declared in DT.

Unfortunately, it has to be even a little more complicated.

This driver will need to work with current DT (no clock-names) as well
as newer DT using clock-names for "core" and "xtal". So, you'll have
to first try for "xtal" here, and if it fails, then try NULL.

> To answer your question from the v2, yes I think it is ok to add clock-names to
> the AO-UART. You are doing it for non AO ones so, why not ?

Agreed. And another good reason the driver needs to handle with and
without clock-names.

Kevin

2017-04-03 15:45:37

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] tty/serial: meson_uart: add support for core clock handling

Helmut Klein <[email protected]> writes:

> To be able to use the three none AO uarts of the meson gx SoCs (uart_A,

s/none/non/

> uart_B & uart_C), the core clock has to be enabled (see chapter 22.3 of
> the public s905 data sheet).
> At least the u-boot of my s905 based media player (netxeon minimx-g)
> doesn't do this. so the driver must enable the clock.

FYI: even if a booloader enablesq clocks, it's important that the kernel
enable clocks that it uses so the kernel can be independent of any
bootloader.

> This patch set does:
> - exposes the UART clock ids to the dtb
> - adds documentation for the dt-bindings of meson_uart
> - adds the core clock handling to the driver
> - adds the core clock handling to meson-gxbb.dtsi and meson-gxl.dtsi
>
> The patchset is based on the branch "master" of the repository in [1]

The master branch in my tree just tracks mainline master branch.

To avoid conflicts with other on-going DT changes, the DT patches should
probably be based on top of my v4.12/dt64 branch.

Kevin

2017-04-03 16:52:43

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] dt-binding: meson_uart: add documentation for the UARTs of amlogic

On Fri, Mar 31, 2017 at 06:54:35PM +0200, Helmut Klein wrote:
> Add the documentation for the device tree binding of meson_uart
>
> Signed-off-by: Helmut Klein <[email protected]>
> ---
> .../bindings/serial/amlogic,meson_uart.txt | 30 ++++++++++++++++++++++
> 1 file changed, 30 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 000000000000..8250a191c2fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson_uart.txt
> @@ -0,0 +1,30 @@
> +* Amlogic Meson UART, used in multiple SoCs (e.g. S905, s905X, ...)
> +
> +Required properties:
> +- compatible : "amlogic,meson-uart"

Should have SoC specific compatible strings in addition.

> +- reg : offset and length of the register set for the device.
> +- interrupts : device interrupt
> +- clocks : a list of phandle + clock-specifier pairs, one for each
> + entry in clock names.
> +- clocks-names : contains:
> + * "xtal" for the baud rate clock
> + * "core" for the core clock of none AO UARTs (optional)
> +
> +e.g.
> +uart_A: serial@84c0 {
> + compatible = "amlogic,meson-uart";
> + reg = <0x0 0x84c0 0x0 0x14>;
> + pinctrl-0 = <&uart_a_pins &uart_a_cts_rts_pins>;
> + interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&xtal>, <&clkc CLKID_UART0>;
> + clock-names = "xtal", "core";
> + status = "ok";

Drop status from examples.

> +};
> +
> +Note: Each port should have an alias correctly numbered in "aliases" node.

True for pretty much any uart, so you can drop this.

> +
> +e.g.
> +aliases {
> + serial0 = &uart_AO;
> + serial1 = &uart_A;
> +};
> --
> 2.11.0
>

2017-04-10 07:43:09

by Michael Turquette

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] dt-bindings: clock: gxbb: expose UART clocks

Quoting Helmut Klein (2017-03-31 18:54:34)
> Expose the clock ids of the three none AO uarts to the dt-bindings
>
> Signed-off-by: Helmut Klein <[email protected]>

Acked-by: Michael Turquette <[email protected]>

> ---
> drivers/clk/meson/gxbb.h | 6 +++---
> include/dt-bindings/clock/gxbb-clkc.h | 3 +++
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/meson/gxbb.h b/drivers/clk/meson/gxbb.h
> index 8ee2022ce5d5..1edfaa5fe307 100644
> --- a/drivers/clk/meson/gxbb.h
> +++ b/drivers/clk/meson/gxbb.h
> @@ -194,7 +194,7 @@
> /* #define CLKID_SAR_ADC */
> #define CLKID_SMART_CARD 24
> #define CLKID_RNG0 25
> -#define CLKID_UART0 26
> +/* CLKID_UART0 */
> #define CLKID_SDHC 27
> #define CLKID_STREAM 28
> #define CLKID_ASYNC_FIFO 29
> @@ -216,7 +216,7 @@
> #define CLKID_ADC 45
> #define CLKID_BLKMV 46
> #define CLKID_AIU 47
> -#define CLKID_UART1 48
> +/* CLKID_UART1 */
> #define CLKID_G2D 49
> /* CLKID_USB0 */
> /* CLKID_USB1 */
> @@ -236,7 +236,7 @@
> /* CLKID_USB0_DDR_BRIDGE */
> #define CLKID_MMC_PCLK 66
> #define CLKID_DVIN 67
> -#define CLKID_UART2 68
> +/* CLKID_UART2 */
> /* #define CLKID_SANA */
> #define CLKID_VPU_INTR 70
> #define CLKID_SEC_AHB_AHB3_BRIDGE 71
> diff --git a/include/dt-bindings/clock/gxbb-clkc.h b/include/dt-bindings/clock/gxbb-clkc.h
> index 692846c7941b..7b329df47752 100644
> --- a/include/dt-bindings/clock/gxbb-clkc.h
> +++ b/include/dt-bindings/clock/gxbb-clkc.h
> @@ -15,13 +15,16 @@
> #define CLKID_SPI 34
> #define CLKID_I2C 22
> #define CLKID_SAR_ADC 23
> +#define CLKID_UART0 26
> #define CLKID_ETH 36
> +#define CLKID_UART1 48
> #define CLKID_USB0 50
> #define CLKID_USB1 51
> #define CLKID_USB 55
> #define CLKID_HDMI_PCLK 63
> #define CLKID_USB1_DDR_BRIDGE 64
> #define CLKID_USB0_DDR_BRIDGE 65
> +#define CLKID_UART2 68
> #define CLKID_SANA 69
> #define CLKID_GCLK_VENCI_INT0 77
> #define CLKID_AO_I2C 93
> --
> 2.11.0
>