From: Clark Wang <xiaoning.wang@nxp.com> To: Miquel Raynal <miquel.raynal@bootlin.com> Cc: "conor.culhane@silvaco.com" <conor.culhane@silvaco.com>, "alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>, "linux-i3c@lists.infradead.org" <linux-i3c@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: RE: [PATCH V2 5/5] i3c: master: svc: add runtime pm support Date: Fri, 16 Jul 2021 08:43:09 +0000 [thread overview] Message-ID: <AM6PR04MB56232E15368E80791C48C58AF3119@AM6PR04MB5623.eurprd04.prod.outlook.com> (raw) In-Reply-To: <20210716095958.4e27d574@xps13> [-- Attachment #1: Type: text/plain, Size: 3889 bytes --] > -----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 [-- Attachment #2: smime.p7s --] [-- Type: application/pkcs7-signature, Size: 9583 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Clark Wang <xiaoning.wang@nxp.com> To: Miquel Raynal <miquel.raynal@bootlin.com> Cc: "conor.culhane@silvaco.com" <conor.culhane@silvaco.com>, "alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>, "linux-i3c@lists.infradead.org" <linux-i3c@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: RE: [PATCH V2 5/5] i3c: master: svc: add runtime pm support Date: Fri, 16 Jul 2021 08:43:09 +0000 [thread overview] Message-ID: <AM6PR04MB56232E15368E80791C48C58AF3119@AM6PR04MB5623.eurprd04.prod.outlook.com> (raw) In-Reply-To: <20210716095958.4e27d574@xps13> [-- Attachment #1.1: Type: text/plain, Size: 3889 bytes --] > -----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 [-- Attachment #1.2: smime.p7s --] [-- Type: application/pkcs7-signature, Size: 9583 bytes --] [-- Attachment #2: Type: text/plain, Size: 111 bytes --] -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c
next prev parent reply other threads:[~2021-07-16 8:43 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-16 7:37 [PATCH V2 0/5] i3c: master: svc: some bug fixes and add runtime pm support Clark Wang 2021-07-16 7:37 ` Clark Wang 2021-07-16 7:37 ` [PATCH V2 1/5] i3c: master: svc: move module reset behind clk enable Clark Wang 2021-07-16 7:37 ` Clark Wang 2021-07-16 7:37 ` [PATCH V2 2/5] i3c: master: svc: fix atomic issue Clark Wang 2021-07-16 7:37 ` Clark Wang 2021-07-16 7:37 ` [PATCH V2 3/5] i3c: master: svc: add support for slave to stop returning data Clark Wang 2021-07-16 7:37 ` Clark Wang 2021-07-16 7:37 ` [PATCH V2 4/5] i3c: master: svc: set ODSTOP to let I2C device see the STOP signal Clark Wang 2021-07-16 7:37 ` Clark Wang 2021-07-16 7:37 ` [PATCH V2 5/5] i3c: master: svc: add runtime pm support Clark Wang 2021-07-16 7:37 ` Clark Wang 2021-07-16 7:59 ` Miquel Raynal 2021-07-16 7:59 ` Miquel Raynal 2021-07-16 8:43 ` Clark Wang [this message] 2021-07-16 8:43 ` Clark Wang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=AM6PR04MB56232E15368E80791C48C58AF3119@AM6PR04MB5623.eurprd04.prod.outlook.com \ --to=xiaoning.wang@nxp.com \ --cc=alexandre.belloni@bootlin.com \ --cc=conor.culhane@silvaco.com \ --cc=linux-i3c@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=miquel.raynal@bootlin.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.