diff mbox series

[RFT,3/6] serial: sh-sci: Move runtime PM enable to sci_probe_single()

Message ID 20241204155806.3781200-4-claudiu.beznea.uj@bp.renesas.com
State New
Headers show
Series serial: sh-sci: Fixes for earlycon and keep_bootcon | expand

Commit Message

Claudiu Beznea Dec. 4, 2024, 3:58 p.m. UTC 
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Relocate the runtime PM enable operation to sci_probe_single(). This change
prepares the codebase for upcoming fixes.

While at it, replace the existing logic with a direct call to
devm_pm_runtime_enable() and remove sci_cleanup_single(). The
devm_pm_runtime_enable() function automatically handles disabling runtime
PM during driver removal.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 drivers/tty/serial/sh-sci.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

Comments

Geert Uytterhoeven Dec. 19, 2024, 10:18 a.m. UTC | #1 
Hi Claudiu,

On Wed, Dec 4, 2024 at 4:58 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Relocate the runtime PM enable operation to sci_probe_single(). This change
> prepares the codebase for upcoming fixes.
>
> While at it, replace the existing logic with a direct call to
> devm_pm_runtime_enable() and remove sci_cleanup_single(). The
> devm_pm_runtime_enable() function automatically handles disabling runtime
> PM during driver removal.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -3440,7 +3434,6 @@ static int sci_probe_single(struct platform_device *dev,
>
>         ret = uart_add_one_port(&sci_uart_driver, &sciport->port);
>         if (ret) {
> -               sci_cleanup_single(sciport);
>                 return ret;
>         }

Next line is:

    return 0;

so please just merge that into

    return uart_add_one_port(&sci_uart_driver, &sciport->port);

Actually [PATCH 5/6] makes that change, but there is no reason not
to do that here.

For the logical changes:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
Claudiu Beznea Dec. 21, 2024, 9:20 a.m. UTC | #2 
On 19.12.2024 12:18, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Wed, Dec 4, 2024 at 4:58 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Relocate the runtime PM enable operation to sci_probe_single(). This change
>> prepares the codebase for upcoming fixes.
>>
>> While at it, replace the existing logic with a direct call to
>> devm_pm_runtime_enable() and remove sci_cleanup_single(). The
>> devm_pm_runtime_enable() function automatically handles disabling runtime
>> PM during driver removal.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/tty/serial/sh-sci.c
>> +++ b/drivers/tty/serial/sh-sci.c
>> @@ -3440,7 +3434,6 @@ static int sci_probe_single(struct platform_device *dev,
>>
>>         ret = uart_add_one_port(&sci_uart_driver, &sciport->port);
>>         if (ret) {
>> -               sci_cleanup_single(sciport);
>>                 return ret;
>>         }
> 
> Next line is:
> 
>     return 0;
> 
> so please just merge that into
> 
>     return uart_add_one_port(&sci_uart_driver, &sciport->port);
> 

You're right with these.

> Actually [PATCH 5/6] makes that change, but there is no reason not
> to do that here.

I remember I chose to keep it like this as I had the impression that if I
format the patches as proposed by you the 5/6 will just revert what I will
have been done in this patch. But I think I was wrong.

Thank you,
Claudiu

> 
> For the logical changes:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>
diff mbox series

Patch

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4f5da3254420..373195995d3b 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3056,10 +3056,6 @@  static int sci_init_single(struct platform_device *dev,
 		ret = sci_init_clocks(sci_port, &dev->dev);
 		if (ret < 0)
 			return ret;
-
-		port->dev = &dev->dev;
-
-		pm_runtime_enable(&dev->dev);
 	}
 
 	port->type		= p->type;
@@ -3086,11 +3082,6 @@  static int sci_init_single(struct platform_device *dev,
 	return 0;
 }
 
-static void sci_cleanup_single(struct sci_port *port)
-{
-	pm_runtime_disable(port->port.dev);
-}
-
 #if defined(CONFIG_SERIAL_SH_SCI_CONSOLE) || \
     defined(CONFIG_SERIAL_SH_SCI_EARLYCON)
 static void serial_console_putchar(struct uart_port *port, unsigned char ch)
@@ -3260,8 +3251,6 @@  static void sci_remove(struct platform_device *dev)
 	sci_ports_in_use &= ~BIT(port->port.line);
 	uart_remove_one_port(&sci_uart_driver, &port->port);
 
-	sci_cleanup_single(port);
-
 	if (port->port.fifosize > 1)
 		device_remove_file(&dev->dev, &dev_attr_rx_fifo_trigger);
 	if (type == PORT_SCIFA || type == PORT_SCIFB || type == PORT_HSCIF)
@@ -3425,6 +3414,11 @@  static int sci_probe_single(struct platform_device *dev,
 	if (ret)
 		return ret;
 
+	sciport->port.dev = &dev->dev;
+	ret = devm_pm_runtime_enable(&dev->dev);
+	if (ret)
+		return ret;
+
 	sciport->gpios = mctrl_gpio_init(&sciport->port, 0);
 	if (IS_ERR(sciport->gpios))
 		return PTR_ERR(sciport->gpios);
@@ -3440,7 +3434,6 @@  static int sci_probe_single(struct platform_device *dev,
 
 	ret = uart_add_one_port(&sci_uart_driver, &sciport->port);
 	if (ret) {
-		sci_cleanup_single(sciport);
 		return ret;
 	}