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

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

[Helmut Klein]

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

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

[Helmut Klein]

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 <hgkr.klein@gmail.com>
---
 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

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

[Jerome Brunet]

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 <hgkr.klein@gmail.com>
> ---
>  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
> 

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

[Kevin Hilman]

On Mon, Apr 3, 2017 at 7:57 AM, Jerome Brunet <jbrunet@baylibre.com> 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 <hgkr.klein@gmail.com>
>> ---
>>  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

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

[Kevin Hilman]

Helmut Klein <hgkr.klein@gmail.com> 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

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

[Neil Armstrong]

Hi Helmut,


On 03/31/2017 06:54 PM, Helmut Klein wrote:
> 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

I'll take the first patch then I will need to rework your patchset (keeping your
autorship, don't worry) since we need to handle the old bindings.

I'll post it for review when ready.

Anyway, thanks for the patchset.

Neil

> 
> 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
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>