On Sun, Sep 21, 2025 at 8:30 AM Bjorn Andersson <[email protected]> wrote: > > On Thu, Aug 28, 2025 at 02:56:15PM +0530, Souradeep Chowdhury wrote: > > If an user stops and starts rproc by using the sysfs interface, then on > > pm suspension the firmware fails to load as the request_firmware call > > under adsp_load relies on usermodehelper process via firmware_fallback_sysfs > > which gets frozen on pm suspension. > > How does it fail? Is the firmware load aborted? Does it time out while > we're suspended?
Firmware load is aborted due to failure from usermodehelper_read_trylock(). It returns an error and there is no timeout. > > The usermodehelper is optional, adsp_load() doesn't rely on > usermodehelper, it relies on the firmware class, which might perform > usermodehelper. yes in this usecase usermodehelper is being used. > > Please describe how and why it fail, so that help educate others (me > included) about what the actual problem you're seeing is. Sure, will include further details in next version. > > > Currently pm_awake calls are present > > in the recovery path, add the same in start and stop path of rproc for > > proper loading of the firmware in non-recovery path. > > I would expect/hope that the git log tells us that the pm_stay_awake() > in the recovery path is there to prevent the system from being suspended > while we're restarting the remotproc, as this is expected to lead to > functional degradation and suboptimal low power state. Yes,this was merged as a part of the following patch: [PATCH] remoteproc: core: Prevent system suspend during remoteproc recovery - Rishabh Bhatnagar > > "They call this function over there" is not sufficient motivation. > > > But just to be clear, I'm not saying that this patch is wrong. I'm > saying I don't understand your problem/motivation. I have elaborated further, hope this clarifies the issue. > > > > > Signed-off-by: Souradeep Chowdhury <[email protected]> > > Signed-off-by: Souradeep Chowdhury <[email protected]> > > This is both you, no need to carry both. Ack > > > Reviewed-by: Mukesh Ojha <[email protected]> > > --- > > Changes in v7 > > > > *Justify this fix by adding more details in commit message > > Please start use b4, so we get proper links to old submissions here. Ack > > Regards, > Bjorn > > > Changes in v6 > > > > *Add some correction to commit message > > > > Changes in v5 > > > > *Added more details to commit description > > > > Changes in v4 > > > > *Remove stability from mailing list > > *Remove the extra tab in v3 > > *Change the commit description > > > > drivers/remoteproc/remoteproc_core.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/remoteproc/remoteproc_core.c > > b/drivers/remoteproc/remoteproc_core.c > > index c2cf0d277729..5d6c4e694b4c 100644 > > --- a/drivers/remoteproc/remoteproc_core.c > > +++ b/drivers/remoteproc/remoteproc_core.c > > @@ -1917,6 +1917,7 @@ int rproc_boot(struct rproc *rproc) > > return -EINVAL; > > } > > > > + pm_stay_awake(rproc->dev.parent); > > dev = &rproc->dev; > > > > ret = mutex_lock_interruptible(&rproc->lock); > > @@ -1961,6 +1962,7 @@ int rproc_boot(struct rproc *rproc) > > atomic_dec(&rproc->power); > > unlock_mutex: > > mutex_unlock(&rproc->lock); > > + pm_relax(rproc->dev.parent); > > return ret; > > } > > EXPORT_SYMBOL(rproc_boot); > > @@ -1991,6 +1993,7 @@ int rproc_shutdown(struct rproc *rproc) > > struct device *dev = &rproc->dev; > > int ret = 0; > > > > + pm_stay_awake(rproc->dev.parent); > > ret = mutex_lock_interruptible(&rproc->lock); > > if (ret) { > > dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret); > > @@ -2027,6 +2030,7 @@ int rproc_shutdown(struct rproc *rproc) > > rproc->table_ptr = NULL; > > out: > > mutex_unlock(&rproc->lock); > > + pm_relax(rproc->dev.parent); > > return ret; > > } > > EXPORT_SYMBOL(rproc_shutdown); > > -- > > 2.34.1 > >
