| Message ID | 20200515104340.10473-3-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v7,1/5] dt-bindings: Document JZ47xx VPU auxiliary processor | expand |
Hi Paul, On 5/15/20 5:43 AM, Paul Cercueil wrote: > Call pm_runtime_get_sync() before the firmware is loaded, and > pm_runtime_put() after the remote processor has been stopped. > > Even though the remoteproc device has no PM callbacks, this allows the > parent device's PM callbacks to be properly called. I see this patch staged now for 5.8, and the latest -next branch has broken the pm-runtime autosuspend feature we have in the OMAP remoteproc driver. See commit 5f31b232c674 ("remoteproc/omap: Add support for runtime auto-suspend/resume"). What was the original purpose of this patch, because there can be differing backends across different SoCs. regards Suman > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > > Notes: > v2-v4: No change > v5: Move calls to prepare/unprepare to rproc_fw_boot/rproc_shutdown > v6: Instead of prepare/unprepare callbacks, use PM runtime callbacks > v7: Check return value of pm_runtime_get_sync() > > drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index a7f96bc98406..e33d1ef27981 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -29,6 +29,7 @@ > #include <linux/devcoredump.h> > #include <linux/rculist.h> > #include <linux/remoteproc.h> > +#include <linux/pm_runtime.h> > #include <linux/iommu.h> > #include <linux/idr.h> > #include <linux/elf.h> > @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) > if (ret) > return ret; > > + ret = pm_runtime_get_sync(dev); > + if (ret < 0) { > + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret); > + return ret; > + } > + > dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size); > > /* > @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) > ret = rproc_enable_iommu(rproc); > if (ret) { > dev_err(dev, "can't enable iommu: %d\n", ret); > - return ret; > + goto put_pm_runtime; > } > > rproc->bootaddr = rproc_get_boot_addr(rproc, fw); > @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) > rproc->table_ptr = NULL; > disable_iommu: > rproc_disable_iommu(rproc); > +put_pm_runtime: > + pm_runtime_put(dev); > return ret; > } > > @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc) > > rproc_disable_iommu(rproc); > > + pm_runtime_put(dev); > + > /* Free the copy of the resource table */ > kfree(rproc->cached_table); > rproc->cached_table = NULL; > @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, > > rproc->state = RPROC_OFFLINE; > > + pm_runtime_no_callbacks(&rproc->dev); > + pm_runtime_enable(&rproc->dev); > + > return rproc; > } > EXPORT_SYMBOL(rproc_alloc); > @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc); > */ > void rproc_free(struct rproc *rproc) > { > + pm_runtime_disable(&rproc->dev); > put_device(&rproc->dev); > } > EXPORT_SYMBOL(rproc_free); >
Hi Suman, Le ven. 22 mai 2020 à 11:47, Suman Anna <s-anna@ti.com> a écrit : > Hi Paul, > > On 5/15/20 5:43 AM, Paul Cercueil wrote: >> Call pm_runtime_get_sync() before the firmware is loaded, and >> pm_runtime_put() after the remote processor has been stopped. >> >> Even though the remoteproc device has no PM callbacks, this allows >> the >> parent device's PM callbacks to be properly called. > > I see this patch staged now for 5.8, and the latest -next branch has > broken the pm-runtime autosuspend feature we have in the OMAP > remoteproc driver. See commit 5f31b232c674 ("remoteproc/omap: Add > support for runtime auto-suspend/resume"). > > What was the original purpose of this patch, because there can be > differing backends across different SoCs. Did you try pm_suspend_ignore_children()? It looks like it was made for your use-case. Cheers, -Paul > > regards > Suman > >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> >> Notes: >> v2-v4: No change >> v5: Move calls to prepare/unprepare to >> rproc_fw_boot/rproc_shutdown >> v6: Instead of prepare/unprepare callbacks, use PM runtime >> callbacks >> v7: Check return value of pm_runtime_get_sync() >> >> drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++- >> 1 file changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/remoteproc_core.c >> b/drivers/remoteproc/remoteproc_core.c >> index a7f96bc98406..e33d1ef27981 100644 >> --- a/drivers/remoteproc/remoteproc_core.c >> +++ b/drivers/remoteproc/remoteproc_core.c >> @@ -29,6 +29,7 @@ >> #include <linux/devcoredump.h> >> #include <linux/rculist.h> >> #include <linux/remoteproc.h> >> +#include <linux/pm_runtime.h> >> #include <linux/iommu.h> >> #include <linux/idr.h> >> #include <linux/elf.h> >> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc, >> const struct firmware *fw) >> if (ret) >> return ret; >> + ret = pm_runtime_get_sync(dev); >> + if (ret < 0) { >> + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret); >> + return ret; >> + } >> + >> dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size); >> /* >> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc, >> const struct firmware *fw) >> ret = rproc_enable_iommu(rproc); >> if (ret) { >> dev_err(dev, "can't enable iommu: %d\n", ret); >> - return ret; >> + goto put_pm_runtime; >> } >> rproc->bootaddr = rproc_get_boot_addr(rproc, fw); >> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc, >> const struct firmware *fw) >> rproc->table_ptr = NULL; >> disable_iommu: >> rproc_disable_iommu(rproc); >> +put_pm_runtime: >> + pm_runtime_put(dev); >> return ret; >> } >> @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc) >> rproc_disable_iommu(rproc); >> + pm_runtime_put(dev); >> + >> /* Free the copy of the resource table */ >> kfree(rproc->cached_table); >> rproc->cached_table = NULL; >> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev, >> const char *name, >> rproc->state = RPROC_OFFLINE; >> + pm_runtime_no_callbacks(&rproc->dev); >> + pm_runtime_enable(&rproc->dev); >> + >> return rproc; >> } >> EXPORT_SYMBOL(rproc_alloc); >> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc); >> */ >> void rproc_free(struct rproc *rproc) >> { >> + pm_runtime_disable(&rproc->dev); >> put_device(&rproc->dev); >> } >> EXPORT_SYMBOL(rproc_free); >> >
Hi Paul, On 5/22/20 12:11 PM, Paul Cercueil wrote: > Hi Suman, > > Le ven. 22 mai 2020 à 11:47, Suman Anna <s-anna@ti.com> a écrit : >> Hi Paul, >> >> On 5/15/20 5:43 AM, Paul Cercueil wrote: >>> Call pm_runtime_get_sync() before the firmware is loaded, and >>> pm_runtime_put() after the remote processor has been stopped. >>> >>> Even though the remoteproc device has no PM callbacks, this allows the >>> parent device's PM callbacks to be properly called. >> >> I see this patch staged now for 5.8, and the latest -next branch has >> broken the pm-runtime autosuspend feature we have in the OMAP >> remoteproc driver. See commit 5f31b232c674 ("remoteproc/omap: Add >> support for runtime auto-suspend/resume"). >> >> What was the original purpose of this patch, because there can be >> differing backends across different SoCs. > > Did you try pm_suspend_ignore_children()? It looks like it was made for > your use-case. Sorry for the delay in getting back. So, using pm_suspend_ignore_children() does fix my current issue. But I still fail to see the original purpose of this patch in the remoteproc core especially given that the core itself does not have any callbacks. If the sole intention was to call the parent pdev's callbacks, then I feel that state-machine is better managed within that particular platform driver itself, as the sequencing/device management can vary with different platform drivers. regards Suman > > Cheers, > -Paul > >> >> regards >> Suman >> >>> >>> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >>> --- >>> >>> Notes: >>> v2-v4: No change >>> v5: Move calls to prepare/unprepare to rproc_fw_boot/rproc_shutdown >>> v6: Instead of prepare/unprepare callbacks, use PM runtime >>> callbacks >>> v7: Check return value of pm_runtime_get_sync() >>> >>> drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++- >>> 1 file changed, 16 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/remoteproc/remoteproc_core.c >>> b/drivers/remoteproc/remoteproc_core.c >>> index a7f96bc98406..e33d1ef27981 100644 >>> --- a/drivers/remoteproc/remoteproc_core.c >>> +++ b/drivers/remoteproc/remoteproc_core.c >>> @@ -29,6 +29,7 @@ >>> #include <linux/devcoredump.h> >>> #include <linux/rculist.h> >>> #include <linux/remoteproc.h> >>> +#include <linux/pm_runtime.h> >>> #include <linux/iommu.h> >>> #include <linux/idr.h> >>> #include <linux/elf.h> >>> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc, >>> const struct firmware *fw) >>> if (ret) >>> return ret; >>> + ret = pm_runtime_get_sync(dev); >>> + if (ret < 0) { >>> + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret); >>> + return ret; >>> + } >>> + >>> dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size); >>> /* >>> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc, >>> const struct firmware *fw) >>> ret = rproc_enable_iommu(rproc); >>> if (ret) { >>> dev_err(dev, "can't enable iommu: %d\n", ret); >>> - return ret; >>> + goto put_pm_runtime; >>> } >>> rproc->bootaddr = rproc_get_boot_addr(rproc, fw); >>> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc, >>> const struct firmware *fw) >>> rproc->table_ptr = NULL; >>> disable_iommu: >>> rproc_disable_iommu(rproc); >>> +put_pm_runtime: >>> + pm_runtime_put(dev); >>> return ret; >>> } >>> @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc) >>> rproc_disable_iommu(rproc); >>> + pm_runtime_put(dev); >>> + >>> /* Free the copy of the resource table */ >>> kfree(rproc->cached_table); >>> rproc->cached_table = NULL; >>> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev, >>> const char *name, >>> rproc->state = RPROC_OFFLINE; >>> + pm_runtime_no_callbacks(&rproc->dev); >>> + pm_runtime_enable(&rproc->dev); >>> + >>> return rproc; >>> } >>> EXPORT_SYMBOL(rproc_alloc); >>> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc); >>> */ >>> void rproc_free(struct rproc *rproc) >>> { >>> + pm_runtime_disable(&rproc->dev); >>> put_device(&rproc->dev); >>> } >>> EXPORT_SYMBOL(rproc_free); >>> >> > >
Hi Suman, >>> On 5/15/20 5:43 AM, Paul Cercueil wrote: >>>> Call pm_runtime_get_sync() before the firmware is loaded, and >>>> pm_runtime_put() after the remote processor has been stopped. >>>> >>>> Even though the remoteproc device has no PM callbacks, this allows >>>> the >>>> parent device's PM callbacks to be properly called. >>> >>> I see this patch staged now for 5.8, and the latest -next branch >>> has broken the pm-runtime autosuspend feature we have in the OMAP >>> remoteproc driver. See commit 5f31b232c674 ("remoteproc/omap: Add >>> support for runtime auto-suspend/resume"). >>> >>> What was the original purpose of this patch, because there can be >>> differing backends across different SoCs. >> >> Did you try pm_suspend_ignore_children()? It looks like it was made >> for your use-case. > > Sorry for the delay in getting back. So, using > pm_suspend_ignore_children() does fix my current issue. > > But I still fail to see the original purpose of this patch in the > remoteproc core especially given that the core itself does not have > any callbacks. If the sole intention was to call the parent pdev's > callbacks, then I feel that state-machine is better managed within > that particular platform driver itself, as the sequencing/device > management can vary with different platform drivers. The problem is that with Ingenic SoCs some clocks must be enabled in order to load the firmware, and the core doesn't give you an option to register a callback to be called before loading it. The first version of my patchset added .prepare/.unprepare callbacks to the struct rproc_ops, but the feedback from the maintainers was that I should do it via runtime PM. However, it was not possible to keep it contained in the driver, since again the core doesn't provide a "prepare" callback, so no place to call pm_runtime_get_sync(). So we settled with having runtime PM in the core without callbacks, which will trigger the runtime PM callbacks of the driver at the right moment. Sorry if that caused you trouble. Cheers, -Paul >>> >>> >>>> >>>> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >>>> --- >>>> >>>> Notes: >>>> v2-v4: No change >>>> v5: Move calls to prepare/unprepare to >>>> rproc_fw_boot/rproc_shutdown >>>> v6: Instead of prepare/unprepare callbacks, use PM runtime >>>> callbacks >>>> v7: Check return value of pm_runtime_get_sync() >>>> >>>> drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++- >>>> 1 file changed, 16 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/remoteproc/remoteproc_core.c >>>> b/drivers/remoteproc/remoteproc_core.c >>>> index a7f96bc98406..e33d1ef27981 100644 >>>> --- a/drivers/remoteproc/remoteproc_core.c >>>> +++ b/drivers/remoteproc/remoteproc_core.c >>>> @@ -29,6 +29,7 @@ >>>> #include <linux/devcoredump.h> >>>> #include <linux/rculist.h> >>>> #include <linux/remoteproc.h> >>>> +#include <linux/pm_runtime.h> >>>> #include <linux/iommu.h> >>>> #include <linux/idr.h> >>>> #include <linux/elf.h> >>>> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc >>>> *rproc, const struct firmware *fw) >>>> if (ret) >>>> return ret; >>>> + ret = pm_runtime_get_sync(dev); >>>> + if (ret < 0) { >>>> + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> dev_info(dev, "Booting fw image %s, size %zd\n", name, >>>> fw->size); >>>> /* >>>> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc >>>> *rproc, const struct firmware *fw) >>>> ret = rproc_enable_iommu(rproc); >>>> if (ret) { >>>> dev_err(dev, "can't enable iommu: %d\n", ret); >>>> - return ret; >>>> + goto put_pm_runtime; >>>> } >>>> rproc->bootaddr = rproc_get_boot_addr(rproc, fw); >>>> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc >>>> *rproc, const struct firmware *fw) >>>> rproc->table_ptr = NULL; >>>> disable_iommu: >>>> rproc_disable_iommu(rproc); >>>> +put_pm_runtime: >>>> + pm_runtime_put(dev); >>>> return ret; >>>> } >>>> @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc) >>>> rproc_disable_iommu(rproc); >>>> + pm_runtime_put(dev); >>>> + >>>> /* Free the copy of the resource table */ >>>> kfree(rproc->cached_table); >>>> rproc->cached_table = NULL; >>>> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device >>>> *dev, const char *name, >>>> rproc->state = RPROC_OFFLINE; >>>> + pm_runtime_no_callbacks(&rproc->dev); >>>> + pm_runtime_enable(&rproc->dev); >>>> + >>>> return rproc; >>>> } >>>> EXPORT_SYMBOL(rproc_alloc); >>>> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc); >>>> */ >>>> void rproc_free(struct rproc *rproc) >>>> { >>>> + pm_runtime_disable(&rproc->dev); >>>> put_device(&rproc->dev); >>>> } >>>> EXPORT_SYMBOL(rproc_free); >>>> >>>
Hi Paul, On 6/8/20 5:46 PM, Paul Cercueil wrote: > Hi Suman, > >>>> On 5/15/20 5:43 AM, Paul Cercueil wrote: >>>>> Call pm_runtime_get_sync() before the firmware is loaded, and >>>>> pm_runtime_put() after the remote processor has been stopped. >>>>> >>>>> Even though the remoteproc device has no PM callbacks, this allows the >>>>> parent device's PM callbacks to be properly called. >>>> >>>> I see this patch staged now for 5.8, and the latest -next branch >>>> has broken the pm-runtime autosuspend feature we have in the >>>> OMAP remoteproc driver. See commit 5f31b232c674 ("remoteproc/omap: >>>> Add support for runtime auto-suspend/resume"). >>>> >>>> What was the original purpose of this patch, because there can be >>>> differing backends across different SoCs. >>> >>> Did you try pm_suspend_ignore_children()? It looks like it was made >>> for your use-case. >> >> Sorry for the delay in getting back. So, using >> pm_suspend_ignore_children() does fix my current issue. >> >> But I still fail to see the original purpose of this patch in the >> remoteproc core especially given that the core itself does not have >> any callbacks. If the sole intention was to call the parent pdev's >> callbacks, then I feel that state-machine is better managed within >> that particular platform driver itself, as the sequencing/device >> management can vary with different platform drivers. > > The problem is that with Ingenic SoCs some clocks must be enabled in > order to load the firmware, and the core doesn't give you an option to > register a callback to be called before loading it. Yep, I have similar usage in one of my remoteproc drivers (see keystone_remoteproc.c), and I think this all stems from the need to use/support loading into a processor's internal memories. My driver does leverage the pm-clks backend plugged into pm_runtime, so you won't see explicit calls on the clocks. I guess the question is what exact PM features you are looking for with the Ingenic SoC. I do see you are using pm_runtime autosuspend, and your callbacks are managing the clocks, but reset is managed only in start/stop. > The first version of > my patchset added .prepare/.unprepare callbacks to the struct rproc_ops, > but the feedback from the maintainers was that I should do it via > runtime PM. However, it was not possible to keep it contained in the > driver, since again the core doesn't provide a "prepare" callback, so no > place to call pm_runtime_get_sync(). FWIW, the .prepare/.unprepare callbacks is actually now part of the rproc core. Looks like multiple developers had a need for this, and this functionality went in at the same time as your driver :). Not sure if you looked up the prior patches, I leveraged the patch that Loic had submitted a long-time ago, and a revised version of it is now part of 5.8-rc1. So we settled with having runtime > PM in the core without callbacks, which will trigger the runtime PM > callbacks of the driver at the right moment. Looks like we can do some cleanup on the Ingenic SoC driver depending on the features you want. regards Suman > > Sorry if that caused you trouble. > > Cheers, > -Paul >>>> > >>>> >>>>> >>>>> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >>>>> --- >>>>> >>>>> Notes: >>>>> v2-v4: No change >>>>> v5: Move calls to prepare/unprepare to >>>>> rproc_fw_boot/rproc_shutdown >>>>> v6: Instead of prepare/unprepare callbacks, use PM runtime >>>>> callbacks >>>>> v7: Check return value of pm_runtime_get_sync() >>>>> >>>>> drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++- >>>>> 1 file changed, 16 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/remoteproc/remoteproc_core.c >>>>> b/drivers/remoteproc/remoteproc_core.c >>>>> index a7f96bc98406..e33d1ef27981 100644 >>>>> --- a/drivers/remoteproc/remoteproc_core.c >>>>> +++ b/drivers/remoteproc/remoteproc_core.c >>>>> @@ -29,6 +29,7 @@ >>>>> #include <linux/devcoredump.h> >>>>> #include <linux/rculist.h> >>>>> #include <linux/remoteproc.h> >>>>> +#include <linux/pm_runtime.h> >>>>> #include <linux/iommu.h> >>>>> #include <linux/idr.h> >>>>> #include <linux/elf.h> >>>>> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc >>>>> *rproc, const struct firmware *fw) >>>>> if (ret) >>>>> return ret; >>>>> + ret = pm_runtime_get_sync(dev); >>>>> + if (ret < 0) { >>>>> + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> dev_info(dev, "Booting fw image %s, size %zd\n", name, >>>>> fw->size); >>>>> /* >>>>> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc >>>>> *rproc, const struct firmware *fw) >>>>> ret = rproc_enable_iommu(rproc); >>>>> if (ret) { >>>>> dev_err(dev, "can't enable iommu: %d\n", ret); >>>>> - return ret; >>>>> + goto put_pm_runtime; >>>>> } >>>>> rproc->bootaddr = rproc_get_boot_addr(rproc, fw); >>>>> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc >>>>> *rproc, const struct firmware *fw) >>>>> rproc->table_ptr = NULL; >>>>> disable_iommu: >>>>> rproc_disable_iommu(rproc); >>>>> +put_pm_runtime: >>>>> + pm_runtime_put(dev); >>>>> return ret; >>>>> } >>>>> @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc) >>>>> rproc_disable_iommu(rproc); >>>>> + pm_runtime_put(dev); >>>>> + >>>>> /* Free the copy of the resource table */ >>>>> kfree(rproc->cached_table); >>>>> rproc->cached_table = NULL; >>>>> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device >>>>> *dev, const char *name, >>>>> rproc->state = RPROC_OFFLINE; >>>>> + pm_runtime_no_callbacks(&rproc->dev); >>>>> + pm_runtime_enable(&rproc->dev); >>>>> + >>>>> return rproc; >>>>> } >>>>> EXPORT_SYMBOL(rproc_alloc); >>>>> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc); >>>>> */ >>>>> void rproc_free(struct rproc *rproc) >>>>> { >>>>> + pm_runtime_disable(&rproc->dev); >>>>> put_device(&rproc->dev); >>>>> } >>>>> EXPORT_SYMBOL(rproc_free); >>>>> >>>> > >
Hi, Le lun. 8 juin 2020 à 18:10, Suman Anna <s-anna@ti.com> a écrit : > Hi Paul, > > On 6/8/20 5:46 PM, Paul Cercueil wrote: >> Hi Suman, >> >>>>> On 5/15/20 5:43 AM, Paul Cercueil wrote: >>>>>> Call pm_runtime_get_sync() before the firmware is loaded, and >>>>>> pm_runtime_put() after the remote processor has been stopped. >>>>>> >>>>>> Even though the remoteproc device has no PM callbacks, this >>>>>> allows the >>>>>> parent device's PM callbacks to be properly called. >>>>> >>>>> I see this patch staged now for 5.8, and the latest -next branch >>>>> has broken the pm-runtime autosuspend feature we have in >>>>> the OMAP remoteproc driver. See commit 5f31b232c674 >>>>> ("remoteproc/omap: Add support for runtime >>>>> auto-suspend/resume"). >>>>> >>>>> What was the original purpose of this patch, because there can be >>>>> differing backends across different SoCs. >>>> >>>> Did you try pm_suspend_ignore_children()? It looks like it was >>>> made for your use-case. >>> >>> Sorry for the delay in getting back. So, using >>> pm_suspend_ignore_children() does fix my current issue. >>> >>> But I still fail to see the original purpose of this patch in the >>> remoteproc core especially given that the core itself does not >>> have any callbacks. If the sole intention was to call the parent >>> pdev's callbacks, then I feel that state-machine is better >>> managed within that particular platform driver itself, as the >>> sequencing/device management can vary with different platform >>> drivers. >> >> The problem is that with Ingenic SoCs some clocks must be enabled in >> order to load the firmware, and the core doesn't give you an option >> to register a callback to be called before loading it. > > Yep, I have similar usage in one of my remoteproc drivers (see > keystone_remoteproc.c), and I think this all stems from the need to > use/support loading into a processor's internal memories. My driver > does leverage the pm-clks backend plugged into pm_runtime, so you > won't see explicit calls on the clocks. > > I guess the question is what exact PM features you are looking for > with the Ingenic SoC. I do see you are using pm_runtime autosuspend, > and your callbacks are managing the clocks, but reset is managed only > in start/stop. > >> The first version of my patchset added .prepare/.unprepare >> callbacks to the struct rproc_ops, but the feedback from the >> maintainers was that I should do it via runtime PM. However, it was >> not possible to keep it contained in the driver, since again the >> core doesn't provide a "prepare" callback, so no place to call >> pm_runtime_get_sync(). > FWIW, the .prepare/.unprepare callbacks is actually now part of the > rproc core. Looks like multiple developers had a need for this, and > this functionality went in at the same time as your driver :). Not > sure if you looked up the prior patches, I leveraged the patch that > Loic had submitted a long-time ago, and a revised version of it is > now part of 5.8-rc1. WTF maintainers, you refuse my patchset for adding a .prepare/.unprepare, ask me to do it via runtime PM, then merge another patchset that adds these callback. At least be constant in your decisions. Anyway, now we have two methods added to linux-next for doing the exact same thing. What should we do about it? -Paul > So we settled with having runtime >> PM in the core without callbacks, which will trigger the runtime PM >> callbacks of the driver at the right moment. > > Looks like we can do some cleanup on the Ingenic SoC driver depending > on the features you want. > > regards > Suman > >> >> Sorry if that caused you trouble. >> >> Cheers, >> -Paul >>>>> >> >>>>> >>>>>> >>>>>> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >>>>>> --- >>>>>> >>>>>> Notes: >>>>>> v2-v4: No change >>>>>> v5: Move calls to prepare/unprepare to >>>>>> rproc_fw_boot/rproc_shutdown >>>>>> v6: Instead of prepare/unprepare callbacks, use PM runtime >>>>>> callbacks >>>>>> v7: Check return value of pm_runtime_get_sync() >>>>>> >>>>>> drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++- >>>>>> 1 file changed, 16 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/remoteproc/remoteproc_core.c >>>>>> b/drivers/remoteproc/remoteproc_core.c >>>>>> index a7f96bc98406..e33d1ef27981 100644 >>>>>> --- a/drivers/remoteproc/remoteproc_core.c >>>>>> +++ b/drivers/remoteproc/remoteproc_core.c >>>>>> @@ -29,6 +29,7 @@ >>>>>> #include <linux/devcoredump.h> >>>>>> #include <linux/rculist.h> >>>>>> #include <linux/remoteproc.h> >>>>>> +#include <linux/pm_runtime.h> >>>>>> #include <linux/iommu.h> >>>>>> #include <linux/idr.h> >>>>>> #include <linux/elf.h> >>>>>> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc >>>>>> *rproc, const struct firmware *fw) >>>>>> if (ret) >>>>>> return ret; >>>>>> + ret = pm_runtime_get_sync(dev); >>>>>> + if (ret < 0) { >>>>>> + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> dev_info(dev, "Booting fw image %s, size %zd\n", name, >>>>>> fw->size); >>>>>> /* >>>>>> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc >>>>>> *rproc, const struct firmware *fw) >>>>>> ret = rproc_enable_iommu(rproc); >>>>>> if (ret) { >>>>>> dev_err(dev, "can't enable iommu: %d\n", ret); >>>>>> - return ret; >>>>>> + goto put_pm_runtime; >>>>>> } >>>>>> rproc->bootaddr = rproc_get_boot_addr(rproc, fw); >>>>>> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc >>>>>> *rproc, const struct firmware *fw) >>>>>> rproc->table_ptr = NULL; >>>>>> disable_iommu: >>>>>> rproc_disable_iommu(rproc); >>>>>> +put_pm_runtime: >>>>>> + pm_runtime_put(dev); >>>>>> return ret; >>>>>> } >>>>>> @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc) >>>>>> rproc_disable_iommu(rproc); >>>>>> + pm_runtime_put(dev); >>>>>> + >>>>>> /* Free the copy of the resource table */ >>>>>> kfree(rproc->cached_table); >>>>>> rproc->cached_table = NULL; >>>>>> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device >>>>>> *dev, const char *name, >>>>>> rproc->state = RPROC_OFFLINE; >>>>>> + pm_runtime_no_callbacks(&rproc->dev); >>>>>> + pm_runtime_enable(&rproc->dev); >>>>>> + >>>>>> return rproc; >>>>>> } >>>>>> EXPORT_SYMBOL(rproc_alloc); >>>>>> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc); >>>>>> */ >>>>>> void rproc_free(struct rproc *rproc) >>>>>> { >>>>>> + pm_runtime_disable(&rproc->dev); >>>>>> put_device(&rproc->dev); >>>>>> } >>>>>> EXPORT_SYMBOL(rproc_free); >>>>>> >>>>> >> >> >
On Wed 10 Jun 02:40 PDT 2020, Paul Cercueil wrote: > Hi, > > Le lun. 8 juin 2020 à 18:10, Suman Anna <s-anna@ti.com> a écrit : > > Hi Paul, > > > > On 6/8/20 5:46 PM, Paul Cercueil wrote: > > > Hi Suman, > > > > > > > > > On 5/15/20 5:43 AM, Paul Cercueil wrote: > > > > > > > Call pm_runtime_get_sync() before the firmware is loaded, and > > > > > > > pm_runtime_put() after the remote processor has been stopped. > > > > > > > > > > > > > > Even though the remoteproc device has no PM > > > > > > > callbacks, this allows the > > > > > > > parent device's PM callbacks to be properly called. > > > > > > > > > > > > I see this patch staged now for 5.8, and the latest > > > > > > -next branch has broken the pm-runtime autosuspend > > > > > > feature we have in the OMAP remoteproc driver. See > > > > > > commit 5f31b232c674 ("remoteproc/omap: Add support > > > > > > for runtime auto-suspend/resume"). > > > > > > > > > > > > What was the original purpose of this patch, because > > > > > > there can be differing backends across different > > > > > > SoCs. > > > > > > > > > > Did you try pm_suspend_ignore_children()? It looks like it > > > > > was made for your use-case. > > > > > > > > Sorry for the delay in getting back. So, using > > > > pm_suspend_ignore_children() does fix my current issue. > > > > > > > > But I still fail to see the original purpose of this patch in > > > > the remoteproc core especially given that the core itself does > > > > not have any callbacks. If the sole intention was to call the > > > > parent pdev's callbacks, then I feel that state-machine is > > > > better managed within that particular platform driver itself, > > > > as the sequencing/device management can vary with different > > > > platform drivers. > > > > > > The problem is that with Ingenic SoCs some clocks must be enabled in > > > order to load the firmware, and the core doesn't give you an option > > > to register a callback to be called before loading it. > > > > Yep, I have similar usage in one of my remoteproc drivers (see > > keystone_remoteproc.c), and I think this all stems from the need to > > use/support loading into a processor's internal memories. My driver does > > leverage the pm-clks backend plugged into pm_runtime, so you won't see > > explicit calls on the clocks. > > > > I guess the question is what exact PM features you are looking for with > > the Ingenic SoC. I do see you are using pm_runtime autosuspend, and your > > callbacks are managing the clocks, but reset is managed only in > > start/stop. > > > > > The first version of my patchset added .prepare/.unprepare > > > callbacks to the struct rproc_ops, but the feedback from the > > > maintainers was that I should do it via runtime PM. However, it was > > > not possible to keep it contained in the driver, since again the > > > core doesn't provide a "prepare" callback, so no place to call > > > pm_runtime_get_sync(). > > FWIW, the .prepare/.unprepare callbacks is actually now part of the > > rproc core. Looks like multiple developers had a need for this, and this > > functionality went in at the same time as your driver :). Not sure if > > you looked up the prior patches, I leveraged the patch that Loic had > > submitted a long-time ago, and a revised version of it is now part of > > 5.8-rc1. > > WTF maintainers, you refuse my patchset for adding a .prepare/.unprepare, > ask me to do it via runtime PM, then merge another patchset that adds these > callback. At least be constant in your decisions. > Sorry, I missed this when applying the two patches, but you're of course right. > Anyway, now we have two methods added to linux-next for doing the exact same > thing. What should we do about it? > I like the pm_runtime approach and as it was Arnaud that asked you to change it, perhaps he and Loic can agree on updating the ST driver so we can drop the prepare/unprepare ops again? Regards, Bjorn
On 6/10/20 11:39 PM, Bjorn Andersson wrote: > On Wed 10 Jun 02:40 PDT 2020, Paul Cercueil wrote: > >> Hi, >> >> Le lun. 8 juin 2020 à 18:10, Suman Anna <s-anna@ti.com> a écrit : >>> Hi Paul, >>> >>> On 6/8/20 5:46 PM, Paul Cercueil wrote: >>>> Hi Suman, >>>> >>>>>>> On 5/15/20 5:43 AM, Paul Cercueil wrote: >>>>>>>> Call pm_runtime_get_sync() before the firmware is loaded, and >>>>>>>> pm_runtime_put() after the remote processor has been stopped. >>>>>>>> >>>>>>>> Even though the remoteproc device has no PM >>>>>>>> callbacks, this allows the >>>>>>>> parent device's PM callbacks to be properly called. >>>>>>> >>>>>>> I see this patch staged now for 5.8, and the latest >>>>>>> -next branch has broken the pm-runtime autosuspend >>>>>>> feature we have in the OMAP remoteproc driver. See >>>>>>> commit 5f31b232c674 ("remoteproc/omap: Add support >>>>>>> for runtime auto-suspend/resume"). >>>>>>> >>>>>>> What was the original purpose of this patch, because >>>>>>> there can be differing backends across different >>>>>>> SoCs. >>>>>> >>>>>> Did you try pm_suspend_ignore_children()? It looks like it >>>>>> was made for your use-case. >>>>> >>>>> Sorry for the delay in getting back. So, using >>>>> pm_suspend_ignore_children() does fix my current issue. >>>>> >>>>> But I still fail to see the original purpose of this patch in >>>>> the remoteproc core especially given that the core itself does >>>>> not have any callbacks. If the sole intention was to call the >>>>> parent pdev's callbacks, then I feel that state-machine is >>>>> better managed within that particular platform driver itself, >>>>> as the sequencing/device management can vary with different >>>>> platform drivers. >>>> >>>> The problem is that with Ingenic SoCs some clocks must be enabled in >>>> order to load the firmware, and the core doesn't give you an option >>>> to register a callback to be called before loading it. >>> >>> Yep, I have similar usage in one of my remoteproc drivers (see >>> keystone_remoteproc.c), and I think this all stems from the need to >>> use/support loading into a processor's internal memories. My driver does >>> leverage the pm-clks backend plugged into pm_runtime, so you won't see >>> explicit calls on the clocks. >>> >>> I guess the question is what exact PM features you are looking for with >>> the Ingenic SoC. I do see you are using pm_runtime autosuspend, and your >>> callbacks are managing the clocks, but reset is managed only in >>> start/stop. >>> >>>> The first version of my patchset added .prepare/.unprepare >>>> callbacks to the struct rproc_ops, but the feedback from the >>>> maintainers was that I should do it via runtime PM. However, it was >>>> not possible to keep it contained in the driver, since again the >>>> core doesn't provide a "prepare" callback, so no place to call >>>> pm_runtime_get_sync(). >>> FWIW, the .prepare/.unprepare callbacks is actually now part of the >>> rproc core. Looks like multiple developers had a need for this, and this >>> functionality went in at the same time as your driver :). Not sure if >>> you looked up the prior patches, I leveraged the patch that Loic had >>> submitted a long-time ago, and a revised version of it is now part of >>> 5.8-rc1. >> >> WTF maintainers, you refuse my patchset for adding a .prepare/.unprepare, >> ask me to do it via runtime PM, then merge another patchset that adds these >> callback. At least be constant in your decisions. >> > > Sorry, I missed this when applying the two patches, but you're of course > right. > >> Anyway, now we have two methods added to linux-next for doing the exact same >> thing. What should we do about it? >> > > I like the pm_runtime approach and as it was Arnaud that asked you to > change it, perhaps he and Loic can agree on updating the ST driver so we > can drop the prepare/unprepare ops again? These callbacks were added primarily in preparation for the TI K3 rproc drivers, not just ST (the patch was resurrected from a very old patch from Loic). I still think prepare/unprepare is actually better suited to scale well for the long term. This pm_runtime logic will now make the early-boot scenarios complicated, as you would have to match its status, but all actual operations are on the actual parent remoteproc platform device and not the child remoteproc device. I think it serves to mess up the state-machines of different platform drivers due to additional refcounts acquired and maybe performing some operations out of sequence to what a platform driver wants esp. if there is automated backend usage like genpd, pm_clks etc. I am yet to review Mathieu's latest MCU sync series, but the concept of different sync_ops already scales w.r.t the prepare/unprepare. As for my K3 drivers, the callbacks are doing more than just turning on clocks, as the R5Fs in general as a complex power-on sequence. I do not have remoteproc auto-suspend atm on the K3 drivers, but that typically means shutting down and restoring the core and would involve all the hardware-specific sequences, so the rpm callback implementations will be more than just clocks. I looked through the patch history on the Ingenic remoteproc driver, and the only reason for either of runtime pm usage or prepare/unprepare ops usage is to ensure that clocks do not stay enabled in the case the processor is not loaded/started. The driver is using auto-boot, so when it probes, in general we expect the remoteproc to be running. So, the only failure case is if there is no firmware. Otherwise, Paul could have just used clk_bulk API in probe and remove. Anyway, I will provide some additional review comments on the pm_runtime usage within the Ingenic rproc driver. regards Suman
Hi, On 6/11/20 11:17 PM, Suman Anna wrote: > On 6/10/20 11:39 PM, Bjorn Andersson wrote: >> On Wed 10 Jun 02:40 PDT 2020, Paul Cercueil wrote: >> >>> Hi, >>> >>> Le lun. 8 juin 2020 à 18:10, Suman Anna <s-anna@ti.com> a écrit : >>>> Hi Paul, >>>> >>>> On 6/8/20 5:46 PM, Paul Cercueil wrote: >>>>> Hi Suman, >>>>> >>>>>>>> On 5/15/20 5:43 AM, Paul Cercueil wrote: >>>>>>>>> Call pm_runtime_get_sync() before the firmware is loaded, and >>>>>>>>> pm_runtime_put() after the remote processor has been stopped. >>>>>>>>> >>>>>>>>> Even though the remoteproc device has no PM >>>>>>>>> callbacks, this allows the >>>>>>>>> parent device's PM callbacks to be properly called. >>>>>>>> >>>>>>>> I see this patch staged now for 5.8, and the latest >>>>>>>> -next branch has broken the pm-runtime autosuspend >>>>>>>> feature we have in the OMAP remoteproc driver. See >>>>>>>> commit 5f31b232c674 ("remoteproc/omap: Add support >>>>>>>> for runtime auto-suspend/resume"). >>>>>>>> >>>>>>>> What was the original purpose of this patch, because >>>>>>>> there can be differing backends across different >>>>>>>> SoCs. >>>>>>> >>>>>>> Did you try pm_suspend_ignore_children()? It looks like it >>>>>>> was made for your use-case. >>>>>> >>>>>> Sorry for the delay in getting back. So, using >>>>>> pm_suspend_ignore_children() does fix my current issue. >>>>>> >>>>>> But I still fail to see the original purpose of this patch in >>>>>> the remoteproc core especially given that the core itself does >>>>>> not have any callbacks. If the sole intention was to call the >>>>>> parent pdev's callbacks, then I feel that state-machine is >>>>>> better managed within that particular platform driver itself, >>>>>> as the sequencing/device management can vary with different >>>>>> platform drivers. >>>>> >>>>> The problem is that with Ingenic SoCs some clocks must be enabled in >>>>> order to load the firmware, and the core doesn't give you an option >>>>> to register a callback to be called before loading it. >>>> >>>> Yep, I have similar usage in one of my remoteproc drivers (see >>>> keystone_remoteproc.c), and I think this all stems from the need to >>>> use/support loading into a processor's internal memories. My driver does >>>> leverage the pm-clks backend plugged into pm_runtime, so you won't see >>>> explicit calls on the clocks. >>>> >>>> I guess the question is what exact PM features you are looking for with >>>> the Ingenic SoC. I do see you are using pm_runtime autosuspend, and your >>>> callbacks are managing the clocks, but reset is managed only in >>>> start/stop. >>>> >>>>> The first version of my patchset added .prepare/.unprepare >>>>> callbacks to the struct rproc_ops, but the feedback from the >>>>> maintainers was that I should do it via runtime PM. However, it was >>>>> not possible to keep it contained in the driver, since again the >>>>> core doesn't provide a "prepare" callback, so no place to call >>>>> pm_runtime_get_sync(). >>>> FWIW, the .prepare/.unprepare callbacks is actually now part of the >>>> rproc core. Looks like multiple developers had a need for this, and this >>>> functionality went in at the same time as your driver :). Not sure if >>>> you looked up the prior patches, I leveraged the patch that Loic had >>>> submitted a long-time ago, and a revised version of it is now part of >>>> 5.8-rc1. >>> >>> WTF maintainers, you refuse my patchset for adding a .prepare/.unprepare, >>> ask me to do it via runtime PM, then merge another patchset that adds these >>> callback. At least be constant in your decisions. >>> >> >> Sorry, I missed this when applying the two patches, but you're of course >> right. >> >>> Anyway, now we have two methods added to linux-next for doing the exact same >>> thing. What should we do about it? >>> >> >> I like the pm_runtime approach and as it was Arnaud that asked you to >> change it, perhaps he and Loic can agree on updating the ST driver so we >> can drop the prepare/unprepare ops again? > > These callbacks were added primarily in preparation for the TI K3 rproc > drivers, not just ST (the patch was resurrected from a very old patch > from Loic). > > I still think prepare/unprepare is actually better suited to scale well > for the long term. This pm_runtime logic will now make the early-boot > scenarios complicated, as you would have to match its status, but all > actual operations are on the actual parent remoteproc platform device > and not the child remoteproc device. I think it serves to mess up the > state-machines of different platform drivers due to additional refcounts > acquired and maybe performing some operations out of sequence to what a > platform driver wants esp. if there is automated backend usage like > genpd, pm_clks etc. I am yet to review Mathieu's latest MCU sync series, > but the concept of different sync_ops already scales w.r.t the > prepare/unprepare. > > As for my K3 drivers, the callbacks are doing more than just turning on > clocks, as the R5Fs in general as a complex power-on sequence. I do not > have remoteproc auto-suspend atm on the K3 drivers, but that typically > means shutting down and restoring the core and would involve all the > hardware-specific sequences, so the rpm callback implementations will be > more than just clocks. > > I looked through the patch history on the Ingenic remoteproc driver, and > the only reason for either of runtime pm usage or prepare/unprepare ops > usage is to ensure that clocks do not stay enabled in the case the > processor is not loaded/started. The driver is using auto-boot, so when > it probes, in general we expect the remoteproc to be running. So, the > only failure case is if there is no firmware. Otherwise, Paul could have > just used clk_bulk API in probe and remove. > > Anyway, I will provide some additional review comments on the pm_runtime > usage within the Ingenic rproc driver. Sorry for the late answer... Prepare/unprepare was proposed by Loïc for the memory management series. We abandoned it and migrated the memory registrations in parse_fw ops. So we don't use it anymore in ST. I suggested the pm-runtime before TI patchset, as it was matching with Ingenic driver need. Now with the additional need introduced by Suman, seems that it is not sufficient for the K3-r5, due to the memory initialization. Anyway do we have to choice between the 2 implementations? Look to me that the pm_runtime could be used to manage the power for the remoteproc device (and perhaps associated power domain). While the .prepare is more for additional resources which ,need to be initialized before loading the firmware (such as some specific system resources). Regards, Arnaud > > regards > Suman >
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index a7f96bc98406..e33d1ef27981 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -29,6 +29,7 @@ #include <linux/devcoredump.h> #include <linux/rculist.h> #include <linux/remoteproc.h> +#include <linux/pm_runtime.h> #include <linux/iommu.h> #include <linux/idr.h> #include <linux/elf.h> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) if (ret) return ret; + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret); + return ret; + } + dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size); /* @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) ret = rproc_enable_iommu(rproc); if (ret) { dev_err(dev, "can't enable iommu: %d\n", ret); - return ret; + goto put_pm_runtime; } rproc->bootaddr = rproc_get_boot_addr(rproc, fw); @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) rproc->table_ptr = NULL; disable_iommu: rproc_disable_iommu(rproc); +put_pm_runtime: + pm_runtime_put(dev); return ret; } @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc) rproc_disable_iommu(rproc); + pm_runtime_put(dev); + /* Free the copy of the resource table */ kfree(rproc->cached_table); rproc->cached_table = NULL; @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, rproc->state = RPROC_OFFLINE; + pm_runtime_no_callbacks(&rproc->dev); + pm_runtime_enable(&rproc->dev); + return rproc; } EXPORT_SYMBOL(rproc_alloc); @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc); */ void rproc_free(struct rproc *rproc) { + pm_runtime_disable(&rproc->dev); put_device(&rproc->dev); } EXPORT_SYMBOL(rproc_free);
Call pm_runtime_get_sync() before the firmware is loaded, and pm_runtime_put() after the remote processor has been stopped. Even though the remoteproc device has no PM callbacks, this allows the parent device's PM callbacks to be properly called. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- Notes: v2-v4: No change v5: Move calls to prepare/unprepare to rproc_fw_boot/rproc_shutdown v6: Instead of prepare/unprepare callbacks, use PM runtime callbacks v7: Check return value of pm_runtime_get_sync() drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)