diff mbox series

[07/31] serial: qcom_geni_serial: convert to use devm_pm_opp_* API

Message ID 20210101165507.19486-8-tiny.windzz@gmail.com
State New
Headers show
Series Introduce devm_pm_opp_* API | expand

Commit Message

Yangtao Li Jan. 1, 2021, 4:54 p.m. UTC
Use devm_pm_opp_* API to simplify code, and we don't need
to make opp_table glabal.

Let's remove opp_table from geni_se later.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

Comments

Viresh Kumar Jan. 4, 2021, 7:19 a.m. UTC | #1
On 01-01-21, 16:54, Yangtao Li wrote:
> Use devm_pm_opp_* API to simplify code, and we don't need
> to make opp_table glabal.
> 
> Let's remove opp_table from geni_se later.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 5aada7ebae35..36a92df8ec11 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1352,6 +1352,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	int irq;
>  	bool console = false;
>  	struct uart_driver *drv;
> +	struct opp_table *opp_table;
>  
>  	if (of_device_is_compatible(pdev->dev.of_node, "qcom,geni-debug-uart"))
>  		console = true;
> @@ -1433,13 +1434,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap"))
>  		port->cts_rts_swap = true;
>  
> -	port->se.opp_table = dev_pm_opp_set_clkname(&pdev->dev, "se");
> -	if (IS_ERR(port->se.opp_table))
> -		return PTR_ERR(port->se.opp_table);
> +	opp_table = devm_pm_opp_set_clkname(&pdev->dev, "se");
> +	if (IS_ERR(opp_table))
> +		return PTR_ERR(opp_table);
>  	/* OPP table is optional */
> -	ret = dev_pm_opp_of_add_table(&pdev->dev);
> +	ret = devm_pm_opp_of_add_table(&pdev->dev);
>  	if (ret) {
> -		dev_pm_opp_put_clkname(port->se.opp_table);
> +		devm_pm_opp_put_clkname(&pdev->dev, opp_table);

We shouldn't be doing this here, i.e. put_clkname. Even when the OPP
table isn't present, this driver calls dev_pm_opp_set_rate() which
behaves like clk_set_rate() in this case and so the clk name is still
required by the OPP core.

>  		if (ret != -ENODEV) {
>  			dev_err(&pdev->dev, "invalid OPP table in device tree\n");
>  			return ret;
> @@ -1453,7 +1454,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  
>  	ret = uart_add_one_port(drv, uport);
>  	if (ret)
> -		goto err;
> +		return ret;
>  
>  	irq_set_status_flags(uport->irq, IRQ_NOAUTOEN);
>  	ret = devm_request_irq(uport->dev, uport->irq, qcom_geni_serial_isr,
> @@ -1461,7 +1462,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	if (ret) {
>  		dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret);
>  		uart_remove_one_port(drv, uport);
> -		goto err;
> +		return ret;
>  	}
>  
>  	/*
> @@ -1478,15 +1479,11 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  		if (ret) {
>  			device_init_wakeup(&pdev->dev, false);
>  			uart_remove_one_port(drv, uport);
> -			goto err;
> +			return ret;
>  		}
>  	}
>  
>  	return 0;
> -err:
> -	dev_pm_opp_of_remove_table(&pdev->dev);
> -	dev_pm_opp_put_clkname(port->se.opp_table);
> -	return ret;
>  }
>  
>  static int qcom_geni_serial_remove(struct platform_device *pdev)
> @@ -1494,8 +1491,6 @@ static int qcom_geni_serial_remove(struct platform_device *pdev)
>  	struct qcom_geni_serial_port *port = platform_get_drvdata(pdev);
>  	struct uart_driver *drv = port->private_data.drv;
>  
> -	dev_pm_opp_of_remove_table(&pdev->dev);
> -	dev_pm_opp_put_clkname(port->se.opp_table);
>  	dev_pm_clear_wake_irq(&pdev->dev);
>  	device_init_wakeup(&pdev->dev, false);
>  	uart_remove_one_port(drv, &port->uport);
> -- 
> 2.25.1
Viresh Kumar Jan. 4, 2021, 7:27 a.m. UTC | #2
Dropped lots of people from cc list

On 04-01-21, 12:49, Viresh Kumar wrote:
> On 01-01-21, 16:54, Yangtao Li wrote:
> > Use devm_pm_opp_* API to simplify code, and we don't need
> > to make opp_table glabal.
> > 
> > Let's remove opp_table from geni_se later.
> > 
> > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > ---
> >  drivers/tty/serial/qcom_geni_serial.c | 23 +++++++++--------------
> >  1 file changed, 9 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> > index 5aada7ebae35..36a92df8ec11 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -1352,6 +1352,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> >  	int irq;
> >  	bool console = false;
> >  	struct uart_driver *drv;
> > +	struct opp_table *opp_table;
> >  
> >  	if (of_device_is_compatible(pdev->dev.of_node, "qcom,geni-debug-uart"))
> >  		console = true;
> > @@ -1433,13 +1434,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> >  	if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap"))
> >  		port->cts_rts_swap = true;
> >  
> > -	port->se.opp_table = dev_pm_opp_set_clkname(&pdev->dev, "se");
> > -	if (IS_ERR(port->se.opp_table))
> > -		return PTR_ERR(port->se.opp_table);
> > +	opp_table = devm_pm_opp_set_clkname(&pdev->dev, "se");
> > +	if (IS_ERR(opp_table))
> > +		return PTR_ERR(opp_table);
> >  	/* OPP table is optional */
> > -	ret = dev_pm_opp_of_add_table(&pdev->dev);
> > +	ret = devm_pm_opp_of_add_table(&pdev->dev);
> >  	if (ret) {
> > -		dev_pm_opp_put_clkname(port->se.opp_table);
> > +		devm_pm_opp_put_clkname(&pdev->dev, opp_table);
> 
> We shouldn't be doing this here, i.e. put_clkname. Even when the OPP
> table isn't present, this driver calls dev_pm_opp_set_rate() which
> behaves like clk_set_rate() in this case and so the clk name is still
> required by the OPP core.

The same problem is there with multiple patches, fix them all please.
diff mbox series

Patch

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 5aada7ebae35..36a92df8ec11 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1352,6 +1352,7 @@  static int qcom_geni_serial_probe(struct platform_device *pdev)
 	int irq;
 	bool console = false;
 	struct uart_driver *drv;
+	struct opp_table *opp_table;
 
 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,geni-debug-uart"))
 		console = true;
@@ -1433,13 +1434,13 @@  static int qcom_geni_serial_probe(struct platform_device *pdev)
 	if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap"))
 		port->cts_rts_swap = true;
 
-	port->se.opp_table = dev_pm_opp_set_clkname(&pdev->dev, "se");
-	if (IS_ERR(port->se.opp_table))
-		return PTR_ERR(port->se.opp_table);
+	opp_table = devm_pm_opp_set_clkname(&pdev->dev, "se");
+	if (IS_ERR(opp_table))
+		return PTR_ERR(opp_table);
 	/* OPP table is optional */
-	ret = dev_pm_opp_of_add_table(&pdev->dev);
+	ret = devm_pm_opp_of_add_table(&pdev->dev);
 	if (ret) {
-		dev_pm_opp_put_clkname(port->se.opp_table);
+		devm_pm_opp_put_clkname(&pdev->dev, opp_table);
 		if (ret != -ENODEV) {
 			dev_err(&pdev->dev, "invalid OPP table in device tree\n");
 			return ret;
@@ -1453,7 +1454,7 @@  static int qcom_geni_serial_probe(struct platform_device *pdev)
 
 	ret = uart_add_one_port(drv, uport);
 	if (ret)
-		goto err;
+		return ret;
 
 	irq_set_status_flags(uport->irq, IRQ_NOAUTOEN);
 	ret = devm_request_irq(uport->dev, uport->irq, qcom_geni_serial_isr,
@@ -1461,7 +1462,7 @@  static int qcom_geni_serial_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret);
 		uart_remove_one_port(drv, uport);
-		goto err;
+		return ret;
 	}
 
 	/*
@@ -1478,15 +1479,11 @@  static int qcom_geni_serial_probe(struct platform_device *pdev)
 		if (ret) {
 			device_init_wakeup(&pdev->dev, false);
 			uart_remove_one_port(drv, uport);
-			goto err;
+			return ret;
 		}
 	}
 
 	return 0;
-err:
-	dev_pm_opp_of_remove_table(&pdev->dev);
-	dev_pm_opp_put_clkname(port->se.opp_table);
-	return ret;
 }
 
 static int qcom_geni_serial_remove(struct platform_device *pdev)
@@ -1494,8 +1491,6 @@  static int qcom_geni_serial_remove(struct platform_device *pdev)
 	struct qcom_geni_serial_port *port = platform_get_drvdata(pdev);
 	struct uart_driver *drv = port->private_data.drv;
 
-	dev_pm_opp_of_remove_table(&pdev->dev);
-	dev_pm_opp_put_clkname(port->se.opp_table);
 	dev_pm_clear_wake_irq(&pdev->dev);
 	device_init_wakeup(&pdev->dev, false);
 	uart_remove_one_port(drv, &port->uport);