diff mbox series

[V2,5/5] i3c: master: svc: add runtime pm support

Message ID 20210716073723.3490180-6-xiaoning.wang@nxp.com (mailing list archive)
State Superseded
Headers show
Series i3c: master: svc: some bug fixes and add runtime pm support | expand

Commit Message

Clark Wang July 16, 2021, 7:37 a.m. UTC
Add runtime pm support to dynamically manage the clock.

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
V2: New patch in V2.
---
 drivers/i3c/master/svc-i3c-master.c | 169 +++++++++++++++++++++++-----
 1 file changed, 142 insertions(+), 27 deletions(-)

Comments

Miquel Raynal July 16, 2021, 7:59 a.m. UTC | #1
Hi Clark,


> @@ -1431,7 +1502,7 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  					 GFP_KERNEL);
>  	if (!master->ibi.slots) {
>  		ret = -ENOMEM;
> -		goto err_disable_sclk;
> +		goto rpm_disable;
>  	}
>  
>  	platform_set_drvdata(pdev, master);
> @@ -1442,18 +1513,17 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  	ret = i3c_master_register(&master->base, &pdev->dev,
>  				  &svc_i3c_master_ops, false);
>  	if (ret)
> -		goto err_disable_sclk;
> +		goto rpm_disable;
>  
> -	return 0;
> -
> -err_disable_sclk:
> -	clk_disable_unprepare(master->sclk);
> +	pm_runtime_mark_last_busy(&pdev->dev);
> +	pm_runtime_put_autosuspend(&pdev->dev);
>  
> -err_disable_fclk:
> -	clk_disable_unprepare(master->fclk);
> +	return 0;
>  
> -err_disable_pclk:
> -	clk_disable_unprepare(master->pclk);

It's not clear to me why you drop the disable_*clk labels to move them
back in place? I would rather prefer to keep a clean error path.

> +rpm_disable:
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
>  
>  	return ret;
>  }
> @@ -1467,13 +1537,57 @@ static int svc_i3c_master_remove(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused svc_i3c_runtime_suspend(struct device *dev)
> +{
> +	struct svc_i3c_master *master = dev_get_drvdata(dev);
> +
>  	clk_disable_unprepare(master->pclk);
>  	clk_disable_unprepare(master->fclk);
>  	clk_disable_unprepare(master->sclk);
> +	pinctrl_pm_select_sleep_state(dev);
>  
>  	return 0;
>  }
>  
> +static int __maybe_unused svc_i3c_runtime_resume(struct device *dev)
> +{
> +	struct svc_i3c_master *master = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	pinctrl_pm_select_default_state(dev);
> +	ret = clk_prepare_enable(master->pclk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(master->fclk);
> +	if (ret) {
> +		clk_disable_unprepare(master->pclk);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(master->sclk);
> +	if (ret) {
> +		clk_disable_unprepare(master->pclk);
> +		clk_disable_unprepare(master->fclk);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops svc_i3c_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				      pm_runtime_force_resume)
> +	SET_RUNTIME_PM_OPS(svc_i3c_runtime_suspend,
> +			   svc_i3c_runtime_resume, NULL)
> +};
> +
>  static const struct of_device_id svc_i3c_master_of_match_tbl[] = {
>  	{ .compatible = "silvaco,i3c-master" },
>  	{ /* sentinel */ },
> @@ -1485,6 +1599,7 @@ static struct platform_driver svc_i3c_master = {
>  	.driver = {
>  		.name = "silvaco-i3c-master",
>  		.of_match_table = svc_i3c_master_of_match_tbl,
> +		.pm = &svc_i3c_pm_ops,
>  	},
>  };
>  module_platform_driver(svc_i3c_master);

Thanks,
Miquèl
Clark Wang July 16, 2021, 8:43 a.m. UTC | #2
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Friday, July 16, 2021 16:00
> To: Clark Wang <xiaoning.wang@nxp.com>
> Cc: conor.culhane@silvaco.com; alexandre.belloni@bootlin.com; linux-
> i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V2 5/5] i3c: master: svc: add runtime pm support
> 
> Hi Clark,
> 
> 
> > @@ -1431,7 +1502,7 @@ static int svc_i3c_master_probe(struct
> platform_device *pdev)
> >  					 GFP_KERNEL);
> >  	if (!master->ibi.slots) {
> >  		ret = -ENOMEM;
> > -		goto err_disable_sclk;
> > +		goto rpm_disable;
> >  	}
> >
> >  	platform_set_drvdata(pdev, master);
> > @@ -1442,18 +1513,17 @@ static int svc_i3c_master_probe(struct
> platform_device *pdev)
> >  	ret = i3c_master_register(&master->base, &pdev->dev,
> >  				  &svc_i3c_master_ops, false);
> >  	if (ret)
> > -		goto err_disable_sclk;
> > +		goto rpm_disable;
> >
> > -	return 0;
> > -
> > -err_disable_sclk:
> > -	clk_disable_unprepare(master->sclk);
> > +	pm_runtime_mark_last_busy(&pdev->dev);
> > +	pm_runtime_put_autosuspend(&pdev->dev);
> >
> > -err_disable_fclk:
> > -	clk_disable_unprepare(master->fclk);
> > +	return 0;
> >
> > -err_disable_pclk:
> > -	clk_disable_unprepare(master->pclk);
> 
> It's not clear to me why you drop the disable_*clk labels to move them back
> in place? I would rather prefer to keep a clean error path.

Hi Miquel,

Yes it is. This change is indeed not very clear.
I have restored the error path.
And I found that it is not necessary to enable runtime pm so early.

Please check V3.

Thanks.

Best Regards,
Clark Wang

> 
> > +rpm_disable:
> > +	pm_runtime_dont_use_autosuspend(&pdev->dev);
> > +	pm_runtime_put_sync(&pdev->dev);
> > +	pm_runtime_disable(&pdev->dev);
> >
> >  	return ret;
> >  }
> > @@ -1467,13 +1537,57 @@ static int svc_i3c_master_remove(struct
> platform_device *pdev)
> >  	if (ret)
> >  		return ret;
> >
> > +	pm_runtime_dont_use_autosuspend(&pdev->dev);
> > +	pm_runtime_disable(&pdev->dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused svc_i3c_runtime_suspend(struct device *dev)
> > +{
> > +	struct svc_i3c_master *master = dev_get_drvdata(dev);
> > +
> >  	clk_disable_unprepare(master->pclk);
> >  	clk_disable_unprepare(master->fclk);
> >  	clk_disable_unprepare(master->sclk);
> > +	pinctrl_pm_select_sleep_state(dev);
> >
> >  	return 0;
> >  }
> >
> > +static int __maybe_unused svc_i3c_runtime_resume(struct device *dev)
> > +{
> > +	struct svc_i3c_master *master = dev_get_drvdata(dev);
> > +	int ret = 0;
> > +
> > +	pinctrl_pm_select_default_state(dev);
> > +	ret = clk_prepare_enable(master->pclk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = clk_prepare_enable(master->fclk);
> > +	if (ret) {
> > +		clk_disable_unprepare(master->pclk);
> > +		return ret;
> > +	}
> > +
> > +	ret = clk_prepare_enable(master->sclk);
> > +	if (ret) {
> > +		clk_disable_unprepare(master->pclk);
> > +		clk_disable_unprepare(master->fclk);
> > +		return ret;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct dev_pm_ops svc_i3c_pm_ops = {
> > +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > +				      pm_runtime_force_resume)
> > +	SET_RUNTIME_PM_OPS(svc_i3c_runtime_suspend,
> > +			   svc_i3c_runtime_resume, NULL)
> > +};
> > +
> >  static const struct of_device_id svc_i3c_master_of_match_tbl[] = {
> >  	{ .compatible = "silvaco,i3c-master" },
> >  	{ /* sentinel */ },
> > @@ -1485,6 +1599,7 @@ static struct platform_driver svc_i3c_master = {
> >  	.driver = {
> >  		.name = "silvaco-i3c-master",
> >  		.of_match_table = svc_i3c_master_of_match_tbl,
> > +		.pm = &svc_i3c_pm_ops,
> >  	},
> >  };
> >  module_platform_driver(svc_i3c_master);
> 
> Thanks,
> Miquèl
diff mbox series

Patch

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 5677b5b31a86..4c264750cfc6 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -17,7 +17,9 @@ 
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 
 /* Master Mode Registers */
 #define SVC_I3C_MCONFIG      0x000
@@ -119,6 +121,7 @@ 
 #define   SVC_MDYNADDR_ADDR(x) FIELD_PREP(GENMASK(7, 1), (x))
 
 #define SVC_I3C_MAX_DEVS 32
+#define SVC_I3C_PM_TIMEOUT_MS 1000
 
 /* This parameter depends on the implementation and may be tuned */
 #define SVC_I3C_FIFO_SIZE 16
@@ -452,10 +455,18 @@  static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
 	u32 ppbaud, pplow, odhpp, odbaud, odstop, i2cbaud, reg;
 	int ret;
 
+	ret = pm_runtime_resume_and_get(master->dev);
+	if (ret < 0) {
+		dev_err(master->dev, "<%s> Cannot get runtime PM.\n", __func__);
+		return ret;
+	}
+
 	/* Timings derivation */
 	fclk_rate = clk_get_rate(master->fclk);
-	if (!fclk_rate)
-		return -EINVAL;
+	if (!fclk_rate) {
+		ret = -EINVAL;
+		goto rpm_out;
+	}
 
 	fclk_period_ns = DIV_ROUND_UP(1000000000, fclk_rate);
 
@@ -499,7 +510,7 @@  static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
 		odstop = 1;
 		break;
 	default:
-		return -EINVAL;
+		goto rpm_out;
 	}
 
 	reg = SVC_I3C_MCONFIG_MASTER_EN |
@@ -517,7 +528,7 @@  static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
 	/* Master core's registration */
 	ret = i3c_master_get_free_addr(m, 0);
 	if (ret < 0)
-		return ret;
+		goto rpm_out;
 
 	info.dyn_addr = ret;
 
@@ -526,21 +537,35 @@  static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
 
 	ret = i3c_master_set_info(&master->base, &info);
 	if (ret)
-		return ret;
+		goto rpm_out;
 
 	svc_i3c_master_enable_interrupts(master, SVC_I3C_MINT_SLVSTART);
 
-	return 0;
+rpm_out:
+	pm_runtime_mark_last_busy(master->dev);
+	pm_runtime_put_autosuspend(master->dev);
+
+	return ret;
 }
 
 static void svc_i3c_master_bus_cleanup(struct i3c_master_controller *m)
 {
 	struct svc_i3c_master *master = to_svc_i3c_master(m);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(master->dev);
+	if (ret < 0) {
+		dev_err(master->dev, "<%s> Cannot get runtime PM.\n", __func__);
+		return;
+	}
 
 	svc_i3c_master_disable_interrupts(master);
 
 	/* Disable master */
 	writel(0, master->regs + SVC_I3C_MCONFIG);
+
+	pm_runtime_mark_last_busy(master->dev);
+	pm_runtime_put_autosuspend(master->dev);
 }
 
 static int svc_i3c_master_reserve_slot(struct svc_i3c_master *master)
@@ -839,6 +864,12 @@  static int svc_i3c_master_do_daa(struct i3c_master_controller *m)
 	unsigned int dev_nb;
 	int ret, i;
 
+	ret = pm_runtime_resume_and_get(master->dev);
+	if (ret < 0) {
+		dev_err(master->dev, "<%s> Cannot get runtime PM.\n", __func__);
+		return ret;
+	}
+
 	spin_lock_irqsave(&master->xferqueue.lock, flags);
 	ret = svc_i3c_master_do_daa_locked(master, addrs, &dev_nb);
 	spin_unlock_irqrestore(&master->xferqueue.lock, flags);
@@ -849,22 +880,24 @@  static int svc_i3c_master_do_daa(struct i3c_master_controller *m)
 	for (i = 0; i < dev_nb; i++) {
 		ret = i3c_master_add_i3c_dev_locked(m, addrs[i]);
 		if (ret)
-			return ret;
+			goto rpm_out;
 	}
 
 	/* Configure IBI auto-rules */
 	ret = svc_i3c_update_ibirules(master);
-	if (ret) {
+	if (ret)
 		dev_err(master->dev, "Cannot handle such a list of devices");
-		return ret;
-	}
 
-	return 0;
+	goto rpm_out;
 
 emit_stop:
 	svc_i3c_master_emit_stop(master);
 	svc_i3c_master_clear_merrwarn(master);
 
+rpm_out:
+	pm_runtime_mark_last_busy(master->dev);
+	pm_runtime_put_autosuspend(master->dev);
+
 	return ret;
 }
 
@@ -936,6 +969,12 @@  static int svc_i3c_master_xfer(struct svc_i3c_master *master,
 	u32 reg;
 	int ret;
 
+	ret = pm_runtime_resume_and_get(master->dev);
+	if (ret < 0) {
+		dev_err(master->dev, "<%s> Cannot get runtime PM.\n", __func__);
+		return ret;
+	}
+
 	writel(SVC_I3C_MCTRL_REQUEST_START_ADDR |
 	       xfer_type |
 	       SVC_I3C_MCTRL_IBIRESP_NACK |
@@ -966,12 +1005,18 @@  static int svc_i3c_master_xfer(struct svc_i3c_master *master,
 	if (!continued)
 		svc_i3c_master_emit_stop(master);
 
+	pm_runtime_mark_last_busy(master->dev);
+	pm_runtime_put_autosuspend(master->dev);
+
 	return 0;
 
 emit_stop:
 	svc_i3c_master_emit_stop(master);
 	svc_i3c_master_clear_merrwarn(master);
 
+	pm_runtime_mark_last_busy(master->dev);
+	pm_runtime_put_autosuspend(master->dev);
+
 	return ret;
 }
 
@@ -1310,6 +1355,14 @@  static void svc_i3c_master_free_ibi(struct i3c_dev_desc *dev)
 static int svc_i3c_master_enable_ibi(struct i3c_dev_desc *dev)
 {
 	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct svc_i3c_master *master = to_svc_i3c_master(m);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(master->dev);
+	if (ret < 0) {
+		dev_err(master->dev, "<%s> Cannot get runtime PM.\n", __func__);
+		return ret;
+	}
 
 	return i3c_master_enec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
 }
@@ -1317,8 +1370,15 @@  static int svc_i3c_master_enable_ibi(struct i3c_dev_desc *dev)
 static int svc_i3c_master_disable_ibi(struct i3c_dev_desc *dev)
 {
 	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct svc_i3c_master *master = to_svc_i3c_master(m);
+	int ret;
 
-	return i3c_master_disec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
+	ret = i3c_master_disec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
+
+	pm_runtime_mark_last_busy(master->dev);
+	pm_runtime_put_autosuspend(master->dev);
+
+	return ret;
 }
 
 static void svc_i3c_master_recycle_ibi_slot(struct i3c_dev_desc *dev,
@@ -1405,19 +1465,30 @@  static int svc_i3c_master_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = clk_prepare_enable(master->fclk);
-	if (ret)
-		goto err_disable_pclk;
+	if (ret) {
+		clk_disable_unprepare(master->pclk);
+		return ret;
+	}
 
 	ret = clk_prepare_enable(master->sclk);
-	if (ret)
-		goto err_disable_fclk;
+	if (ret) {
+		clk_disable_unprepare(master->pclk);
+		clk_disable_unprepare(master->fclk);
+		return ret;
+	}
+
+	pm_runtime_set_autosuspend_delay(&pdev->dev, SVC_I3C_PM_TIMEOUT_MS);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
 
 	INIT_WORK(&master->hj_work, svc_i3c_master_hj_work);
 	INIT_WORK(&master->ibi_work, svc_i3c_master_ibi_work);
 	ret = devm_request_irq(dev, master->irq, svc_i3c_master_irq_handler,
 			       IRQF_NO_SUSPEND, "svc-i3c-irq", master);
 	if (ret)
-		goto err_disable_sclk;
+		goto rpm_disable;
 
 	master->free_slots = GENMASK(SVC_I3C_MAX_DEVS - 1, 0);
 
@@ -1431,7 +1502,7 @@  static int svc_i3c_master_probe(struct platform_device *pdev)
 					 GFP_KERNEL);
 	if (!master->ibi.slots) {
 		ret = -ENOMEM;
-		goto err_disable_sclk;
+		goto rpm_disable;
 	}
 
 	platform_set_drvdata(pdev, master);
@@ -1442,18 +1513,17 @@  static int svc_i3c_master_probe(struct platform_device *pdev)
 	ret = i3c_master_register(&master->base, &pdev->dev,
 				  &svc_i3c_master_ops, false);
 	if (ret)
-		goto err_disable_sclk;
+		goto rpm_disable;
 
-	return 0;
-
-err_disable_sclk:
-	clk_disable_unprepare(master->sclk);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
 
-err_disable_fclk:
-	clk_disable_unprepare(master->fclk);
+	return 0;
 
-err_disable_pclk:
-	clk_disable_unprepare(master->pclk);
+rpm_disable:
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
 
 	return ret;
 }
@@ -1467,13 +1537,57 @@  static int svc_i3c_master_remove(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static int __maybe_unused svc_i3c_runtime_suspend(struct device *dev)
+{
+	struct svc_i3c_master *master = dev_get_drvdata(dev);
+
 	clk_disable_unprepare(master->pclk);
 	clk_disable_unprepare(master->fclk);
 	clk_disable_unprepare(master->sclk);
+	pinctrl_pm_select_sleep_state(dev);
 
 	return 0;
 }
 
+static int __maybe_unused svc_i3c_runtime_resume(struct device *dev)
+{
+	struct svc_i3c_master *master = dev_get_drvdata(dev);
+	int ret = 0;
+
+	pinctrl_pm_select_default_state(dev);
+	ret = clk_prepare_enable(master->pclk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(master->fclk);
+	if (ret) {
+		clk_disable_unprepare(master->pclk);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(master->sclk);
+	if (ret) {
+		clk_disable_unprepare(master->pclk);
+		clk_disable_unprepare(master->fclk);
+		return ret;
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops svc_i3c_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				      pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(svc_i3c_runtime_suspend,
+			   svc_i3c_runtime_resume, NULL)
+};
+
 static const struct of_device_id svc_i3c_master_of_match_tbl[] = {
 	{ .compatible = "silvaco,i3c-master" },
 	{ /* sentinel */ },
@@ -1485,6 +1599,7 @@  static struct platform_driver svc_i3c_master = {
 	.driver = {
 		.name = "silvaco-i3c-master",
 		.of_match_table = svc_i3c_master_of_match_tbl,
+		.pm = &svc_i3c_pm_ops,
 	},
 };
 module_platform_driver(svc_i3c_master);