Question about the state of rproc

[mengtanhzc]

I've noticed that the rproc->state changes even if the rproc->ops fails, this seems unnecessary and different from the linux remoteproc framework. Should the state not be changed if the ops fails?

https://github.com/OpenAMP/open-amp/blob/main/lib/remoteproc/remoteproc.c#L242

int remoteproc_start(struct remoteproc *rproc)
{
    int ret = -RPROC_ENODEV;

    if (rproc) {
        metal_mutex_acquire(&rproc->lock);
        if (rproc->state == RPROC_READY) {
            ret = rproc->ops->start(rproc);
>>>         rproc->state = RPROC_RUNNING;
        } else {
            ret = -RPROC_EINVAL;
        }
        metal_mutex_release(&rproc->lock);
    }
    return ret;
}

Linux remoteproc framwork: https://github.com/torvalds/linux/blob/master/drivers/remoteproc/remoteproc_core.c#L1302

static int rproc_start(struct rproc *rproc, const struct firmware *fw)
{
    ... ....

    /* power up the remote processor */
    ret = rproc->ops->start(rproc);
    if (ret) {
        dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
        goto unprepare_subdevices;
    }

    ... ...

    rproc->state = RPROC_RUNNING;

    dev_info(dev, "remote processor %s is now up\n", rproc->name);

    return 0;

stop_rproc:
    rproc->ops->stop(rproc);
unprepare_subdevices:
    rproc_unprepare_subdevices(rproc);
reset_table_ptr:
    rproc->table_ptr = rproc->cached_table;

    return ret;
}

[tnmysh]

I think this concern is correct. This should be fixed.

[arnopo]

Hello @mengtanhzc Don't hesitate to propose a pull request to fix it.

[mengtanhzc]

Hi, I make a fix in #600 . Please review it. Thanks a lot! @arnopo @tnmysh