* [PATCH 0/3] serial: atmel: cleanup code
@ 2022-05-25 13:37 Claudiu Beznea
2022-05-25 13:37 ` [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops Claudiu Beznea
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Claudiu Beznea @ 2022-05-25 13:37 UTC (permalink / raw)
To: richard.genoud, gregkh, jirislaby, nicolas.ferre,
alexandre.belloni, patrice.chotard
Cc: linux-serial, linux-arm-kernel, linux-kernel, Claudiu Beznea
Hi,
The following patches does some cleanup for atmel_serial driver by
switching to dev_pm_ops and improving clock management code. Along with
it I took the chance and introduced a patch for st-asc which removes the
include of pm_runtime.h.
Thank you,
Claudiu Beznea
Claudiu Beznea (3):
tty: serial: atmel: stop using legacy pm ops
tty: serial: atmel: improve clock management
serial: st-asc: remove include of pm_runtime.h
drivers/tty/serial/atmel_serial.c | 93 +++++++++----------------------
drivers/tty/serial/st-asc.c | 1 -
2 files changed, 26 insertions(+), 68 deletions(-)
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops 2022-05-25 13:37 [PATCH 0/3] serial: atmel: cleanup code Claudiu Beznea @ 2022-05-25 13:37 ` Claudiu Beznea 2022-06-02 9:14 ` Richard Genoud 2022-05-25 13:37 ` [PATCH 2/3] tty: serial: atmel: improve clock management Claudiu Beznea 2022-05-25 13:37 ` [PATCH 3/3] serial: st-asc: remove include of pm_runtime.h Claudiu Beznea 2 siblings, 1 reply; 9+ messages in thread From: Claudiu Beznea @ 2022-05-25 13:37 UTC (permalink / raw) To: richard.genoud, gregkh, jirislaby, nicolas.ferre, alexandre.belloni, patrice.chotard Cc: linux-serial, linux-arm-kernel, linux-kernel, Claudiu Beznea Stop using legacy PM ops and switch using dev_pm_ops. Along with it #ifdef CONFIG_PM are removed and __maybe_unused and pm_ptr() used instead. Coding style recommends (at chapter Conditional Compilation) to avoid using preprocessor conditional and use __maybe_unused instead. Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- drivers/tty/serial/atmel_serial.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index dd1c7e4bd1c9..5c793a23dc54 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -166,7 +166,6 @@ struct atmel_uart_port { unsigned int fidi_min; unsigned int fidi_max; -#ifdef CONFIG_PM struct { u32 cr; u32 mr; @@ -177,7 +176,6 @@ struct atmel_uart_port { u32 fmr; u32 fimr; } cache; -#endif int (*prepare_rx)(struct uart_port *port); int (*prepare_tx)(struct uart_port *port); @@ -2711,7 +2709,6 @@ static struct uart_driver atmel_uart = { .cons = ATMEL_CONSOLE_DEVICE, }; -#ifdef CONFIG_PM static bool atmel_serial_clk_will_stop(void) { #ifdef CONFIG_ARCH_AT91 @@ -2721,10 +2718,9 @@ static bool atmel_serial_clk_will_stop(void) #endif } -static int atmel_serial_suspend(struct platform_device *pdev, - pm_message_t state) +static int __maybe_unused atmel_serial_suspend(struct device *dev) { - struct uart_port *port = platform_get_drvdata(pdev); + struct uart_port *port = dev_get_drvdata(dev); struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); if (uart_console(port) && console_suspend_enabled) { @@ -2749,14 +2745,14 @@ static int atmel_serial_suspend(struct platform_device *pdev, } /* we can not wake up if we're running on slow clock */ - atmel_port->may_wakeup = device_may_wakeup(&pdev->dev); + atmel_port->may_wakeup = device_may_wakeup(dev); if (atmel_serial_clk_will_stop()) { unsigned long flags; spin_lock_irqsave(&atmel_port->lock_suspended, flags); atmel_port->suspended = true; spin_unlock_irqrestore(&atmel_port->lock_suspended, flags); - device_set_wakeup_enable(&pdev->dev, 0); + device_set_wakeup_enable(dev, 0); } uart_suspend_port(&atmel_uart, port); @@ -2764,9 +2760,9 @@ static int atmel_serial_suspend(struct platform_device *pdev, return 0; } -static int atmel_serial_resume(struct platform_device *pdev) +static int __maybe_unused atmel_serial_resume(struct device *dev) { - struct uart_port *port = platform_get_drvdata(pdev); + struct uart_port *port = dev_get_drvdata(dev); struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); unsigned long flags; @@ -2801,14 +2797,10 @@ static int atmel_serial_resume(struct platform_device *pdev) spin_unlock_irqrestore(&atmel_port->lock_suspended, flags); uart_resume_port(&atmel_uart, port); - device_set_wakeup_enable(&pdev->dev, atmel_port->may_wakeup); + device_set_wakeup_enable(dev, atmel_port->may_wakeup); return 0; } -#else -#define atmel_serial_suspend NULL -#define atmel_serial_resume NULL -#endif static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port, struct platform_device *pdev) @@ -3012,14 +3004,16 @@ static int atmel_serial_remove(struct platform_device *pdev) return ret; } +static SIMPLE_DEV_PM_OPS(atmel_serial_pm_ops, atmel_serial_suspend, + atmel_serial_resume); + static struct platform_driver atmel_serial_driver = { .probe = atmel_serial_probe, .remove = atmel_serial_remove, - .suspend = atmel_serial_suspend, - .resume = atmel_serial_resume, .driver = { .name = "atmel_usart_serial", .of_match_table = of_match_ptr(atmel_serial_dt_ids), + .pm = pm_ptr(&atmel_serial_pm_ops), }, }; -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops 2022-05-25 13:37 ` [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops Claudiu Beznea @ 2022-06-02 9:14 ` Richard Genoud 0 siblings, 0 replies; 9+ messages in thread From: Richard Genoud @ 2022-06-02 9:14 UTC (permalink / raw) To: Claudiu Beznea, gregkh, jirislaby, nicolas.ferre, alexandre.belloni, patrice.chotard Cc: linux-serial, linux-arm-kernel, linux-kernel Hi, Le 25/05/2022 à 15:37, Claudiu Beznea a écrit : > Stop using legacy PM ops and switch using dev_pm_ops. Along with > it #ifdef CONFIG_PM are removed and __maybe_unused and pm_ptr() used > instead. Coding style recommends (at chapter Conditional Compilation) > to avoid using preprocessor conditional and use __maybe_unused > instead. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> Acked-by: Richard Genoud <richard.genoud@gmail.com> Thanks ! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] tty: serial: atmel: improve clock management 2022-05-25 13:37 [PATCH 0/3] serial: atmel: cleanup code Claudiu Beznea 2022-05-25 13:37 ` [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops Claudiu Beznea @ 2022-05-25 13:37 ` Claudiu Beznea 2022-06-02 9:29 ` Richard Genoud 2022-05-25 13:37 ` [PATCH 3/3] serial: st-asc: remove include of pm_runtime.h Claudiu Beznea 2 siblings, 1 reply; 9+ messages in thread From: Claudiu Beznea @ 2022-05-25 13:37 UTC (permalink / raw) To: richard.genoud, gregkh, jirislaby, nicolas.ferre, alexandre.belloni, patrice.chotard Cc: linux-serial, linux-arm-kernel, linux-kernel, Claudiu Beznea atmel_port->clk was requested in atmel_init_port() (which is called only on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is NULL on probing path. Thus don't check this. Along with this the clock is now requested with devm_clk_get() and the clock request has been moved in atmel_serial_probe() function to avoid disabling/re-enabling it on probe path for multiple times. All the checks of atmel_port->clk were removed along with clk_put() and atmel_port->clk = NULL. Now, on probing time the clock is enabled at the beginning and disabled at the end of probe. As atmel_console_setup() is called in the middle of probe and clock is already enabled at that time the clk_prepare_enable() in atmel_console_setup() has also been removed. Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------ 1 file changed, 15 insertions(+), 50 deletions(-) diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index 5c793a23dc54..2955b1012014 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port, if (ret) return ret; - /* for console, the clock could already be configured */ - if (!atmel_port->clk) { - atmel_port->clk = clk_get(&mpdev->dev, "usart"); - if (IS_ERR(atmel_port->clk)) { - ret = PTR_ERR(atmel_port->clk); - atmel_port->clk = NULL; - return ret; - } - ret = clk_prepare_enable(atmel_port->clk); - if (ret) { - clk_put(atmel_port->clk); - atmel_port->clk = NULL; - return ret; - } - port->uartclk = clk_get_rate(atmel_port->clk); - clk_disable_unprepare(atmel_port->clk); - /* only enable clock when USART is in use */ - } + port->uartclk = clk_get_rate(atmel_port->clk); /* * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct console *co, char *options) return -ENODEV; } - ret = clk_prepare_enable(atmel_ports[co->index].clk); - if (ret) - return ret; - atmel_uart_writel(port, ATMEL_US_IDR, -1); atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX); atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN); @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct platform_device *pdev) atomic_set(&atmel_port->tasklet_shutdown, 0); spin_lock_init(&atmel_port->lock_suspended); + atmel_port->clk = devm_clk_get(&pdev->dev, "usart"); + if (IS_ERR(atmel_port->clk)) { + ret = PTR_ERR(atmel_port->clk); + goto err; + } + ret = clk_prepare_enable(atmel_port->clk); + if (ret) + goto err; + ret = atmel_init_port(atmel_port, pdev); if (ret) - goto err_clear_bit; + goto err_clk_disable_unprepare; atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0); if (IS_ERR(atmel_port->gpios)) { ret = PTR_ERR(atmel_port->gpios); - goto err_clear_bit; + goto err_clk_disable_unprepare; } if (!atmel_use_pdc_rx(&atmel_port->uart)) { @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct platform_device *pdev) sizeof(struct atmel_uart_char), GFP_KERNEL); if (!data) - goto err_alloc_ring; + goto err_clk_disable_unprepare; atmel_port->rx_ring.buf = data; } @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct platform_device *pdev) if (ret) goto err_add_port; -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE - if (uart_console(&atmel_port->uart) - && ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) { - /* - * The serial core enabled the clock for us, so undo - * the clk_prepare_enable() in atmel_console_setup() - */ - clk_disable_unprepare(atmel_port->clk); - } -#endif - device_init_wakeup(&pdev->dev, 1); platform_set_drvdata(pdev, atmel_port); - /* - * The peripheral clock has been disabled by atmel_init_port(): - * enable it before accessing I/O registers - */ - clk_prepare_enable(atmel_port->clk); - if (rs485_enabled) { atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR, ATMEL_US_USMODE_NORMAL); @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct platform_device *pdev) err_add_port: kfree(atmel_port->rx_ring.buf); atmel_port->rx_ring.buf = NULL; -err_alloc_ring: - if (!uart_console(&atmel_port->uart)) { - clk_put(atmel_port->clk); - atmel_port->clk = NULL; - } -err_clear_bit: +err_clk_disable_unprepare: + clk_disable_unprepare(atmel_port->clk); clear_bit(atmel_port->uart.line, atmel_ports_in_use); err: return ret; @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct platform_device *pdev) clear_bit(port->line, atmel_ports_in_use); - clk_put(atmel_port->clk); - atmel_port->clk = NULL; pdev->dev.of_node = NULL; return ret; -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] tty: serial: atmel: improve clock management 2022-05-25 13:37 ` [PATCH 2/3] tty: serial: atmel: improve clock management Claudiu Beznea @ 2022-06-02 9:29 ` Richard Genoud 2022-06-08 8:18 ` Claudiu.Beznea 0 siblings, 1 reply; 9+ messages in thread From: Richard Genoud @ 2022-06-02 9:29 UTC (permalink / raw) To: Claudiu Beznea, gregkh, jirislaby, nicolas.ferre, alexandre.belloni, patrice.chotard Cc: linux-serial, linux-arm-kernel, linux-kernel Hi, Le 25/05/2022 à 15:37, Claudiu Beznea a écrit : > atmel_port->clk was requested in atmel_init_port() (which is called only > on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is > NULL on probing path. Thus don't check this. Along with this the clock is > now requested with devm_clk_get() and the clock request has been moved in > atmel_serial_probe() function to avoid disabling/re-enabling it on probe > path for multiple times. All the checks of atmel_port->clk were removed > along with clk_put() and atmel_port->clk = NULL. Now, on probing time the > clock is enabled at the beginning and disabled at the end of probe. As > atmel_console_setup() is called in the middle of probe and clock is > already enabled at that time the clk_prepare_enable() in > atmel_console_setup() has also been removed. Could you split this patch into smaller steps ? I think it will be easier to read and review. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------ > 1 file changed, 15 insertions(+), 50 deletions(-) > > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c > index 5c793a23dc54..2955b1012014 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port, > if (ret) > return ret; > > - /* for console, the clock could already be configured */ > - if (!atmel_port->clk) { > - atmel_port->clk = clk_get(&mpdev->dev, "usart"); > - if (IS_ERR(atmel_port->clk)) { > - ret = PTR_ERR(atmel_port->clk); > - atmel_port->clk = NULL; > - return ret; > - } > - ret = clk_prepare_enable(atmel_port->clk); > - if (ret) { > - clk_put(atmel_port->clk); > - atmel_port->clk = NULL; > - return ret; > - } > - port->uartclk = clk_get_rate(atmel_port->clk); > - clk_disable_unprepare(atmel_port->clk); > - /* only enable clock when USART is in use */ > - } > + port->uartclk = clk_get_rate(atmel_port->clk); > > /* > * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or > @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct console *co, char *options) > return -ENODEV; > } > > - ret = clk_prepare_enable(atmel_ports[co->index].clk); > - if (ret) > - return ret; > - Now, "int ret;" is unused, you can remove it. > atmel_uart_writel(port, ATMEL_US_IDR, -1); > atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX); > atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN); > @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct platform_device *pdev) > atomic_set(&atmel_port->tasklet_shutdown, 0); > spin_lock_init(&atmel_port->lock_suspended); > > + atmel_port->clk = devm_clk_get(&pdev->dev, "usart"); > + if (IS_ERR(atmel_port->clk)) { > + ret = PTR_ERR(atmel_port->clk); > + goto err; > + } > + ret = clk_prepare_enable(atmel_port->clk); > + if (ret) > + goto err; > + > ret = atmel_init_port(atmel_port, pdev); > if (ret) > - goto err_clear_bit; > + goto err_clk_disable_unprepare; > > atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0); > if (IS_ERR(atmel_port->gpios)) { > ret = PTR_ERR(atmel_port->gpios); > - goto err_clear_bit; > + goto err_clk_disable_unprepare; > } > > if (!atmel_use_pdc_rx(&atmel_port->uart)) { > @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct platform_device *pdev) > sizeof(struct atmel_uart_char), > GFP_KERNEL); > if (!data) > - goto err_alloc_ring; > + goto err_clk_disable_unprepare; > atmel_port->rx_ring.buf = data; > } > > @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct platform_device *pdev) > if (ret) > goto err_add_port; > > -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE > - if (uart_console(&atmel_port->uart) > - && ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) { > - /* > - * The serial core enabled the clock for us, so undo > - * the clk_prepare_enable() in atmel_console_setup() > - */ > - clk_disable_unprepare(atmel_port->clk); > - } > -#endif > - > device_init_wakeup(&pdev->dev, 1); > platform_set_drvdata(pdev, atmel_port); > > - /* > - * The peripheral clock has been disabled by atmel_init_port(): > - * enable it before accessing I/O registers > - */ > - clk_prepare_enable(atmel_port->clk); > - > if (rs485_enabled) { > atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR, > ATMEL_US_USMODE_NORMAL); > @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct platform_device *pdev) > err_add_port: > kfree(atmel_port->rx_ring.buf); > atmel_port->rx_ring.buf = NULL; > -err_alloc_ring: > - if (!uart_console(&atmel_port->uart)) { > - clk_put(atmel_port->clk); > - atmel_port->clk = NULL; > - } > -err_clear_bit: > +err_clk_disable_unprepare: > + clk_disable_unprepare(atmel_port->clk); > clear_bit(atmel_port->uart.line, atmel_ports_in_use); > err: > return ret; > @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct platform_device *pdev) > > clear_bit(port->line, atmel_ports_in_use); > > - clk_put(atmel_port->clk); > - atmel_port->clk = NULL; > pdev->dev.of_node = NULL; > > return ret; Thanks ! Regards, Richard _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] tty: serial: atmel: improve clock management 2022-06-02 9:29 ` Richard Genoud @ 2022-06-08 8:18 ` Claudiu.Beznea 2022-06-10 13:43 ` Richard Genoud 0 siblings, 1 reply; 9+ messages in thread From: Claudiu.Beznea @ 2022-06-08 8:18 UTC (permalink / raw) To: richard.genoud, gregkh, jirislaby, Nicolas.Ferre, alexandre.belloni, patrice.chotard Cc: linux-serial, linux-arm-kernel, linux-kernel Hi, On 02.06.2022 12:29, Richard Genoud wrote: > > Hi, > > Le 25/05/2022 à 15:37, Claudiu Beznea a écrit : >> atmel_port->clk was requested in atmel_init_port() (which is called only >> on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is >> NULL on probing path. Thus don't check this. Along with this the clock is >> now requested with devm_clk_get() and the clock request has been moved in >> atmel_serial_probe() function to avoid disabling/re-enabling it on probe >> path for multiple times. All the checks of atmel_port->clk were removed >> along with clk_put() and atmel_port->clk = NULL. Now, on probing time the >> clock is enabled at the beginning and disabled at the end of probe. As >> atmel_console_setup() is called in the middle of probe and clock is >> already enabled at that time the clk_prepare_enable() in >> atmel_console_setup() has also been removed. > Could you split this patch into smaller steps ? > I think it will be easier to read and review. I kept it as a single patch as it is all related to clock management. Having the clock enabled only at the beginning of probe and disabled at the end of probe lead to removing the code in atmel_init_port(), also removing the code under CONFIG_SERIAL_ATMEL_CONSOLE in probe. Same for the rest of the removed code. With this, would you still want to split it in multiple patches? Thank you, Claudiu Beznea >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> --- >> drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------ >> 1 file changed, 15 insertions(+), 50 deletions(-) >> >> diff --git a/drivers/tty/serial/atmel_serial.c >> b/drivers/tty/serial/atmel_serial.c >> index 5c793a23dc54..2955b1012014 100644 >> --- a/drivers/tty/serial/atmel_serial.c >> +++ b/drivers/tty/serial/atmel_serial.c >> @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port >> *atmel_port, >> if (ret) >> return ret; >> >> - /* for console, the clock could already be configured */ >> - if (!atmel_port->clk) { >> - atmel_port->clk = clk_get(&mpdev->dev, "usart"); >> - if (IS_ERR(atmel_port->clk)) { >> - ret = PTR_ERR(atmel_port->clk); >> - atmel_port->clk = NULL; >> - return ret; >> - } >> - ret = clk_prepare_enable(atmel_port->clk); >> - if (ret) { >> - clk_put(atmel_port->clk); >> - atmel_port->clk = NULL; >> - return ret; >> - } >> - port->uartclk = clk_get_rate(atmel_port->clk); >> - clk_disable_unprepare(atmel_port->clk); >> - /* only enable clock when USART is in use */ >> - } >> + port->uartclk = clk_get_rate(atmel_port->clk); >> >> /* >> * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or >> @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct >> console *co, char *options) >> return -ENODEV; >> } >> >> - ret = clk_prepare_enable(atmel_ports[co->index].clk); >> - if (ret) >> - return ret; >> - > Now, "int ret;" is unused, you can remove it. > >> atmel_uart_writel(port, ATMEL_US_IDR, -1); >> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | >> ATMEL_US_RSTRX); >> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN); >> @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct >> platform_device *pdev) >> atomic_set(&atmel_port->tasklet_shutdown, 0); >> spin_lock_init(&atmel_port->lock_suspended); >> >> + atmel_port->clk = devm_clk_get(&pdev->dev, "usart"); >> + if (IS_ERR(atmel_port->clk)) { >> + ret = PTR_ERR(atmel_port->clk); >> + goto err; >> + } >> + ret = clk_prepare_enable(atmel_port->clk); >> + if (ret) >> + goto err; >> + >> ret = atmel_init_port(atmel_port, pdev); >> if (ret) >> - goto err_clear_bit; >> + goto err_clk_disable_unprepare; >> >> atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0); >> if (IS_ERR(atmel_port->gpios)) { >> ret = PTR_ERR(atmel_port->gpios); >> - goto err_clear_bit; >> + goto err_clk_disable_unprepare; >> } >> >> if (!atmel_use_pdc_rx(&atmel_port->uart)) { >> @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct >> platform_device *pdev) >> sizeof(struct atmel_uart_char), >> GFP_KERNEL); >> if (!data) >> - goto err_alloc_ring; >> + goto err_clk_disable_unprepare; >> atmel_port->rx_ring.buf = data; >> } >> >> @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct >> platform_device *pdev) >> if (ret) >> goto err_add_port; >> >> -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE >> - if (uart_console(&atmel_port->uart) >> - && ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) { >> - /* >> - * The serial core enabled the clock for us, so undo >> - * the clk_prepare_enable() in atmel_console_setup() >> - */ >> - clk_disable_unprepare(atmel_port->clk); >> - } >> -#endif >> - >> device_init_wakeup(&pdev->dev, 1); >> platform_set_drvdata(pdev, atmel_port); >> >> - /* >> - * The peripheral clock has been disabled by atmel_init_port(): >> - * enable it before accessing I/O registers >> - */ >> - clk_prepare_enable(atmel_port->clk); >> - >> if (rs485_enabled) { >> atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR, >> ATMEL_US_USMODE_NORMAL); >> @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct >> platform_device *pdev) >> err_add_port: >> kfree(atmel_port->rx_ring.buf); >> atmel_port->rx_ring.buf = NULL; >> -err_alloc_ring: >> - if (!uart_console(&atmel_port->uart)) { >> - clk_put(atmel_port->clk); >> - atmel_port->clk = NULL; >> - } >> -err_clear_bit: >> +err_clk_disable_unprepare: >> + clk_disable_unprepare(atmel_port->clk); >> clear_bit(atmel_port->uart.line, atmel_ports_in_use); >> err: >> return ret; >> @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct >> platform_device *pdev) >> >> clear_bit(port->line, atmel_ports_in_use); >> >> - clk_put(atmel_port->clk); >> - atmel_port->clk = NULL; >> pdev->dev.of_node = NULL; >> >> return ret; > Thanks ! > > Regards, > Richard _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] tty: serial: atmel: improve clock management 2022-06-08 8:18 ` Claudiu.Beznea @ 2022-06-10 13:43 ` Richard Genoud 0 siblings, 0 replies; 9+ messages in thread From: Richard Genoud @ 2022-06-10 13:43 UTC (permalink / raw) To: Claudiu.Beznea, gregkh, jirislaby, Nicolas.Ferre, alexandre.belloni, patrice.chotard Cc: linux-serial, linux-arm-kernel, linux-kernel Hi, Le 08/06/2022 à 10:18, Claudiu.Beznea@microchip.com a écrit : > Hi, > > On 02.06.2022 12:29, Richard Genoud wrote: >> >> Hi, >> >> Le 25/05/2022 à 15:37, Claudiu Beznea a écrit : >>> atmel_port->clk was requested in atmel_init_port() (which is called only >>> on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is >>> NULL on probing path. Thus don't check this. Along with this the clock is >>> now requested with devm_clk_get() and the clock request has been moved in >>> atmel_serial_probe() function to avoid disabling/re-enabling it on probe >>> path for multiple times. All the checks of atmel_port->clk were removed >>> along with clk_put() and atmel_port->clk = NULL. Now, on probing time the >>> clock is enabled at the beginning and disabled at the end of probe. As >>> atmel_console_setup() is called in the middle of probe and clock is >>> already enabled at that time the clk_prepare_enable() in >>> atmel_console_setup() has also been removed. >> Could you split this patch into smaller steps ? >> I think it will be easier to read and review. > > I kept it as a single patch as it is all related to clock management. > > Having the clock enabled only at the beginning of probe and disabled at the > end of probe lead to removing the code in atmel_init_port(), also removing > the code under CONFIG_SERIAL_ATMEL_CONSOLE in probe. Same for the rest of > the removed code. > > With this, would you still want to split it in multiple patches? I think that, at least, the switch from clk_get() to devm_clk_get() can be in a separate patch. But in general, I prefer when patches are self-explaining rather than a bigger patch with a longer explanation of what it does in the commit message. Regards, Richard > > Thank you, > Claudiu Beznea >>> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >>> --- >>> drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------ >>> 1 file changed, 15 insertions(+), 50 deletions(-) >>> >>> diff --git a/drivers/tty/serial/atmel_serial.c >>> b/drivers/tty/serial/atmel_serial.c >>> index 5c793a23dc54..2955b1012014 100644 >>> --- a/drivers/tty/serial/atmel_serial.c >>> +++ b/drivers/tty/serial/atmel_serial.c >>> @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port >>> *atmel_port, >>> if (ret) >>> return ret; >>> >>> - /* for console, the clock could already be configured */ >>> - if (!atmel_port->clk) { >>> - atmel_port->clk = clk_get(&mpdev->dev, "usart"); >>> - if (IS_ERR(atmel_port->clk)) { >>> - ret = PTR_ERR(atmel_port->clk); >>> - atmel_port->clk = NULL; >>> - return ret; >>> - } >>> - ret = clk_prepare_enable(atmel_port->clk); >>> - if (ret) { >>> - clk_put(atmel_port->clk); >>> - atmel_port->clk = NULL; >>> - return ret; >>> - } >>> - port->uartclk = clk_get_rate(atmel_port->clk); >>> - clk_disable_unprepare(atmel_port->clk); >>> - /* only enable clock when USART is in use */ >>> - } >>> + port->uartclk = clk_get_rate(atmel_port->clk); >>> >>> /* >>> * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or >>> @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct >>> console *co, char *options) >>> return -ENODEV; >>> } >>> >>> - ret = clk_prepare_enable(atmel_ports[co->index].clk); >>> - if (ret) >>> - return ret; >>> - >> Now, "int ret;" is unused, you can remove it. >> >>> atmel_uart_writel(port, ATMEL_US_IDR, -1); >>> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | >>> ATMEL_US_RSTRX); >>> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN); >>> @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct >>> platform_device *pdev) >>> atomic_set(&atmel_port->tasklet_shutdown, 0); >>> spin_lock_init(&atmel_port->lock_suspended); >>> >>> + atmel_port->clk = devm_clk_get(&pdev->dev, "usart"); >>> + if (IS_ERR(atmel_port->clk)) { >>> + ret = PTR_ERR(atmel_port->clk); >>> + goto err; >>> + } >>> + ret = clk_prepare_enable(atmel_port->clk); >>> + if (ret) >>> + goto err; >>> + >>> ret = atmel_init_port(atmel_port, pdev); >>> if (ret) >>> - goto err_clear_bit; >>> + goto err_clk_disable_unprepare; >>> >>> atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0); >>> if (IS_ERR(atmel_port->gpios)) { >>> ret = PTR_ERR(atmel_port->gpios); >>> - goto err_clear_bit; >>> + goto err_clk_disable_unprepare; >>> } >>> >>> if (!atmel_use_pdc_rx(&atmel_port->uart)) { >>> @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct >>> platform_device *pdev) >>> sizeof(struct atmel_uart_char), >>> GFP_KERNEL); >>> if (!data) >>> - goto err_alloc_ring; >>> + goto err_clk_disable_unprepare; >>> atmel_port->rx_ring.buf = data; >>> } >>> >>> @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct >>> platform_device *pdev) >>> if (ret) >>> goto err_add_port; >>> >>> -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE >>> - if (uart_console(&atmel_port->uart) >>> - && ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) { >>> - /* >>> - * The serial core enabled the clock for us, so undo >>> - * the clk_prepare_enable() in atmel_console_setup() >>> - */ >>> - clk_disable_unprepare(atmel_port->clk); >>> - } >>> -#endif >>> - >>> device_init_wakeup(&pdev->dev, 1); >>> platform_set_drvdata(pdev, atmel_port); >>> >>> - /* >>> - * The peripheral clock has been disabled by atmel_init_port(): >>> - * enable it before accessing I/O registers >>> - */ >>> - clk_prepare_enable(atmel_port->clk); >>> - >>> if (rs485_enabled) { >>> atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR, >>> ATMEL_US_USMODE_NORMAL); >>> @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct >>> platform_device *pdev) >>> err_add_port: >>> kfree(atmel_port->rx_ring.buf); >>> atmel_port->rx_ring.buf = NULL; >>> -err_alloc_ring: >>> - if (!uart_console(&atmel_port->uart)) { >>> - clk_put(atmel_port->clk); >>> - atmel_port->clk = NULL; >>> - } >>> -err_clear_bit: >>> +err_clk_disable_unprepare: >>> + clk_disable_unprepare(atmel_port->clk); >>> clear_bit(atmel_port->uart.line, atmel_ports_in_use); >>> err: >>> return ret; >>> @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct >>> platform_device *pdev) >>> >>> clear_bit(port->line, atmel_ports_in_use); >>> >>> - clk_put(atmel_port->clk); >>> - atmel_port->clk = NULL; >>> pdev->dev.of_node = NULL; >>> >>> return ret; >> Thanks ! >> >> Regards, >> Richard > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] serial: st-asc: remove include of pm_runtime.h 2022-05-25 13:37 [PATCH 0/3] serial: atmel: cleanup code Claudiu Beznea 2022-05-25 13:37 ` [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops Claudiu Beznea 2022-05-25 13:37 ` [PATCH 2/3] tty: serial: atmel: improve clock management Claudiu Beznea @ 2022-05-25 13:37 ` Claudiu Beznea 2022-05-30 7:47 ` Patrice CHOTARD 2 siblings, 1 reply; 9+ messages in thread From: Claudiu Beznea @ 2022-05-25 13:37 UTC (permalink / raw) To: richard.genoud, gregkh, jirislaby, nicolas.ferre, alexandre.belloni, patrice.chotard Cc: linux-serial, linux-arm-kernel, linux-kernel, Claudiu Beznea st-asc driver doesn't use helpers from pm_runtime.h thus remove its include. Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- drivers/tty/serial/st-asc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c index 1b0da603ab54..cce42f4c9bc2 100644 --- a/drivers/tty/serial/st-asc.c +++ b/drivers/tty/serial/st-asc.c @@ -17,7 +17,6 @@ #include <linux/tty_flip.h> #include <linux/delay.h> #include <linux/spinlock.h> -#include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/of_platform.h> #include <linux/serial_core.h> -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] serial: st-asc: remove include of pm_runtime.h 2022-05-25 13:37 ` [PATCH 3/3] serial: st-asc: remove include of pm_runtime.h Claudiu Beznea @ 2022-05-30 7:47 ` Patrice CHOTARD 0 siblings, 0 replies; 9+ messages in thread From: Patrice CHOTARD @ 2022-05-30 7:47 UTC (permalink / raw) To: Claudiu Beznea, richard.genoud, gregkh, jirislaby, nicolas.ferre, alexandre.belloni Cc: linux-serial, linux-arm-kernel, linux-kernel Hi Claudiu On 5/25/22 15:37, Claudiu Beznea wrote: > st-asc driver doesn't use helpers from pm_runtime.h thus remove its > include. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > drivers/tty/serial/st-asc.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c > index 1b0da603ab54..cce42f4c9bc2 100644 > --- a/drivers/tty/serial/st-asc.c > +++ b/drivers/tty/serial/st-asc.c > @@ -17,7 +17,6 @@ > #include <linux/tty_flip.h> > #include <linux/delay.h> > #include <linux/spinlock.h> > -#include <linux/pm_runtime.h> > #include <linux/of.h> > #include <linux/of_platform.h> > #include <linux/serial_core.h> Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com> Thanks _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-06-10 13:44 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-05-25 13:37 [PATCH 0/3] serial: atmel: cleanup code Claudiu Beznea 2022-05-25 13:37 ` [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops Claudiu Beznea 2022-06-02 9:14 ` Richard Genoud 2022-05-25 13:37 ` [PATCH 2/3] tty: serial: atmel: improve clock management Claudiu Beznea 2022-06-02 9:29 ` Richard Genoud 2022-06-08 8:18 ` Claudiu.Beznea 2022-06-10 13:43 ` Richard Genoud 2022-05-25 13:37 ` [PATCH 3/3] serial: st-asc: remove include of pm_runtime.h Claudiu Beznea 2022-05-30 7:47 ` Patrice CHOTARD
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).