[PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings
From: jbrunet@baylibre.com (Jerome Brunet)
Date: 2017-06-12 09:39:10
Also in:
linux-amlogic, linux-serial, lkml
On Fri, 2017-06-09 at 11:49 +0200, Neil Armstrong wrote:
quoted hunk ↗ jump to hunk
From: Helmut Klein <redacted> 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 <redacted> Signed-off-by: Neil Armstrong <redacted> --- ?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(structconsole *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(structconsole *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 <jbrunet@baylibre.com>
+ + 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.
quoted hunk ↗ jump to hunk
+ + 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);