On Wed, Feb 26, 2020 at 4:54 PM Changqi Hu <changqi.hu@xxxxxxxxxxxx> wrote:
>
> MTK uart design no need to control uart clock,
> so we just control bus clock in runtime function.
> Add uart clock used count to avoid repeatedly switching the clock.
This patch does a lot more than that:
- Adds a busy loop in mtk8250_runtime_suspend
- Changes how you do pm_runtime stuff.
These probably need to be split to different patches, and can you
please describe why you are making those changes in the commit
message?
> Signed-off-by: Changqi Hu <changqi.hu@xxxxxxxxxxxx>
> ---
>
> Changes in v4:
> Modify commit-message
>
> Changes in v3:
> Merge patch v1 and v2 together.
>
> Changes in v2:
> Enable uart bus clock when probe and resume base on v1 patch,
> but miss v1 patch itself.
>
> drivers/tty/serial/8250/8250_mtk.c | 50 ++++++++++++++++++++++++--------------
> 1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 4d067f5..f839380 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -32,6 +32,7 @@
> #define MTK_UART_RXTRI_AD 0x14 /* RX Trigger address */
> #define MTK_UART_FRACDIV_L 0x15 /* Fractional divider LSB address */
> #define MTK_UART_FRACDIV_M 0x16 /* Fractional divider MSB address */
> +#define MTK_UART_DEBUG0 0x18
> #define MTK_UART_IER_XOFFI 0x20 /* Enable XOFF character interrupt */
> #define MTK_UART_IER_RTSI 0x40 /* Enable RTS Modem status interrupt */
> #define MTK_UART_IER_CTSI 0x80 /* Enable CTS Modem status interrupt */
> @@ -388,9 +389,18 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios,
> static int __maybe_unused mtk8250_runtime_suspend(struct device *dev)
> {
> struct mtk8250_data *data = dev_get_drvdata(dev);
> + struct uart_8250_port *up = serial8250_get_port(data->line);
>
> - clk_disable_unprepare(data->uart_clk);
> - clk_disable_unprepare(data->bus_clk);
> + /* wait until UART in idle status */
> + while
> + (serial_in(up, MTK_UART_DEBUG0));
No timeout?
> +
> + if (data->clk_count == 0U) {
> + dev_dbg(dev, "%s clock count is 0\n", __func__);
> + } else {
> + clk_disable_unprepare(data->bus_clk);
> + data->clk_count--;
> + }
The clock core already does reference counting for you, so I don't
think you need this.
https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk.c#L1004
>
> return 0;
> }
> @@ -400,16 +410,16 @@ static int __maybe_unused mtk8250_runtime_resume(struct device *dev)
> struct mtk8250_data *data = dev_get_drvdata(dev);
> int err;
>
> - err = clk_prepare_enable(data->uart_clk);
> - if (err) {
> - dev_warn(dev, "Can't enable clock\n");
> - return err;
> - }
> -
> - err = clk_prepare_enable(data->bus_clk);
> - if (err) {
> - dev_warn(dev, "Can't enable bus clock\n");
> - return err;
> + if (data->clk_count > 0U) {
> + dev_dbg(dev, "%s clock count is %d\n", __func__,
> + data->clk_count);
> + } else {
> + err = clk_prepare_enable(data->bus_clk);
> + if (err) {
> + dev_warn(dev, "Can't enable bus clock\n");
> + return err;
> + }
> + data->clk_count++;
> }
>
> return 0;
> @@ -419,12 +429,14 @@ static void
> mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
> {
> if (!state)
> - pm_runtime_get_sync(port->dev);
> + if (!mtk8250_runtime_resume(port->dev))
> + pm_runtime_get_sync(port->dev);
>
> serial8250_do_pm(port, state, old);
>
> if (state)
> - pm_runtime_put_sync_suspend(port->dev);
> + if (!pm_runtime_put_sync_suspend(port->dev))
> + mtk8250_runtime_suspend(port->dev);
> }
>
> #ifdef CONFIG_SERIAL_8250_DMA
> @@ -501,6 +513,8 @@ static int mtk8250_probe(struct platform_device *pdev)
> if (!data)
> return -ENOMEM;
>
> + data->clk_count = 0;
> +
> if (pdev->dev.of_node) {
> err = mtk8250_probe_of(pdev, &uart.port, data);
> if (err)
> @@ -533,6 +547,7 @@ static int mtk8250_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, data);
>
> + pm_runtime_enable(&pdev->dev);
> err = mtk8250_runtime_resume(&pdev->dev);
> if (err)
> return err;
> @@ -541,9 +556,6 @@ static int mtk8250_probe(struct platform_device *pdev)
> if (data->line < 0)
> return data->line;
>
> - pm_runtime_set_active(&pdev->dev);
> - pm_runtime_enable(&pdev->dev);
> -
> data->rx_wakeup_irq = platform_get_irq_optional(pdev, 1);
>
> return 0;
> @@ -556,11 +568,13 @@ static int mtk8250_remove(struct platform_device *pdev)
> pm_runtime_get_sync(&pdev->dev);
>
> serial8250_unregister_port(data->line);
> - mtk8250_runtime_suspend(&pdev->dev);
>
> pm_runtime_disable(&pdev->dev);
> pm_runtime_put_noidle(&pdev->dev);
>
> + if (!pm_runtime_status_suspended(&pdev->dev))
> + mtk8250_runtime_suspend(&pdev->dev);
> +
> return 0;
> }
>
> --
> 2.6.4
