| Message ID | 20240430105307.1190615-3-b-padhi@ti.com (mailing list archive) |
|---|---|
| State | Queued |
| Headers | show |
| Series | remoteproc: k3-r5: Wait for core0 power-up before powering up core1 | expand |
Le 30/04/2024 à 12:53, Beleswar Padhi a écrit : > PSC controller has a limitation that it can only power-up the second > core when the first core is in ON state. Power-state for core0 should be > equal to or higher than core1. > > Therefore, prevent core1 from powering up before core0 during the start > process from sysfs. Similarly, prevent core0 from shutting down before > core1 has been shut down from sysfs. > > Fixes: 6dedbd1d5443 ("remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem") > > Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > --- > drivers/remoteproc/ti_k3_r5_remoteproc.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > index 6d6afd6beb3a..1799b4f6d11e 100644 > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > @@ -548,7 +548,7 @@ static int k3_r5_rproc_start(struct rproc *rproc) > struct k3_r5_rproc *kproc = rproc->priv; > struct k3_r5_cluster *cluster = kproc->cluster; > struct device *dev = kproc->dev; > - struct k3_r5_core *core; > + struct k3_r5_core *core0, *core; > u32 boot_addr; > int ret; > > @@ -574,6 +574,15 @@ static int k3_r5_rproc_start(struct rproc *rproc) > goto unroll_core_run; > } > } else { > + /* do not allow core 1 to start before core 0 */ > + core0 = list_first_entry(&cluster->cores, struct k3_r5_core, > + elem); > + if (core != core0 && core0->rproc->state == RPROC_OFFLINE) { > + dev_err(dev, "%s: can not start core 1 before core 0\n", > + __func__); > + return -EPERM; > + } > + > ret = k3_r5_core_run(core); > if (ret) > goto put_mbox; > @@ -619,7 +628,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > { > struct k3_r5_rproc *kproc = rproc->priv; > struct k3_r5_cluster *cluster = kproc->cluster; > - struct k3_r5_core *core = kproc->core; > + struct device *dev = kproc->dev; > + struct k3_r5_core *core1, *core = kproc->core; > int ret; > > /* halt all applicable cores */ > @@ -632,6 +642,15 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > } > } > } else { > + /* do not allow core 0 to stop before core 1 */ > + core1 = list_last_entry(&cluster->cores, struct k3_r5_core, > + elem); > + if (core != core1 && core1->rproc->state != RPROC_OFFLINE) { > + dev_err(dev, "%s: can not stop core 0 before core 1\n", > + __func__); > + return -EPERM; Hi, this patch has already reached -next, but should this "return -EPERM;" be : ret = -EPERM; goto put_mbox; instead? CJ > + } > + > ret = k3_r5_core_halt(core); > if (ret) > goto out;
On Sat, 18 May 2024 at 04:44, Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > Le 30/04/2024 à 12:53, Beleswar Padhi a écrit : > > PSC controller has a limitation that it can only power-up the second > > core when the first core is in ON state. Power-state for core0 should be > > equal to or higher than core1. > > > > Therefore, prevent core1 from powering up before core0 during the start > > process from sysfs. Similarly, prevent core0 from shutting down before > > core1 has been shut down from sysfs. > > > > Fixes: 6dedbd1d5443 ("remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem") > > > > Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > > --- > > drivers/remoteproc/ti_k3_r5_remoteproc.c | 23 +++++++++++++++++++++-- > > 1 file changed, 21 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > index 6d6afd6beb3a..1799b4f6d11e 100644 > > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > @@ -548,7 +548,7 @@ static int k3_r5_rproc_start(struct rproc *rproc) > > struct k3_r5_rproc *kproc = rproc->priv; > > struct k3_r5_cluster *cluster = kproc->cluster; > > struct device *dev = kproc->dev; > > - struct k3_r5_core *core; > > + struct k3_r5_core *core0, *core; > > u32 boot_addr; > > int ret; > > > > @@ -574,6 +574,15 @@ static int k3_r5_rproc_start(struct rproc *rproc) > > goto unroll_core_run; > > } > > } else { > > + /* do not allow core 1 to start before core 0 */ > > + core0 = list_first_entry(&cluster->cores, struct k3_r5_core, > > + elem); > > + if (core != core0 && core0->rproc->state == RPROC_OFFLINE) { > > + dev_err(dev, "%s: can not start core 1 before core 0\n", > > + __func__); > > + return -EPERM; > > + } > > + > > ret = k3_r5_core_run(core); > > if (ret) > > goto put_mbox; > > @@ -619,7 +628,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > > { > > struct k3_r5_rproc *kproc = rproc->priv; > > struct k3_r5_cluster *cluster = kproc->cluster; > > - struct k3_r5_core *core = kproc->core; > > + struct device *dev = kproc->dev; > > + struct k3_r5_core *core1, *core = kproc->core; > > int ret; > > > > /* halt all applicable cores */ > > @@ -632,6 +642,15 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > > } > > } > > } else { > > + /* do not allow core 0 to stop before core 1 */ > > + core1 = list_last_entry(&cluster->cores, struct k3_r5_core, > > + elem); > > + if (core != core1 && core1->rproc->state != RPROC_OFFLINE) { > > + dev_err(dev, "%s: can not stop core 0 before core 1\n", > > + __func__); > > + return -EPERM; > > Hi, > > this patch has already reached -next, but should this "return -EPERM;" be : > ret = -EPERM; > goto put_mbox; > > instead? > This has already been addressed: https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/commit/?h=rproc-next&id=1dc7242f6ee0c99852cb90676d7fe201cf5de422 > CJ > > > + } > > + > > ret = k3_r5_core_halt(core); > > if (ret) > > goto out; >
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index 6d6afd6beb3a..1799b4f6d11e 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -548,7 +548,7 @@ static int k3_r5_rproc_start(struct rproc *rproc) struct k3_r5_rproc *kproc = rproc->priv; struct k3_r5_cluster *cluster = kproc->cluster; struct device *dev = kproc->dev; - struct k3_r5_core *core; + struct k3_r5_core *core0, *core; u32 boot_addr; int ret; @@ -574,6 +574,15 @@ static int k3_r5_rproc_start(struct rproc *rproc) goto unroll_core_run; } } else { + /* do not allow core 1 to start before core 0 */ + core0 = list_first_entry(&cluster->cores, struct k3_r5_core, + elem); + if (core != core0 && core0->rproc->state == RPROC_OFFLINE) { + dev_err(dev, "%s: can not start core 1 before core 0\n", + __func__); + return -EPERM; + } + ret = k3_r5_core_run(core); if (ret) goto put_mbox; @@ -619,7 +628,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc) { struct k3_r5_rproc *kproc = rproc->priv; struct k3_r5_cluster *cluster = kproc->cluster; - struct k3_r5_core *core = kproc->core; + struct device *dev = kproc->dev; + struct k3_r5_core *core1, *core = kproc->core; int ret; /* halt all applicable cores */ @@ -632,6 +642,15 @@ static int k3_r5_rproc_stop(struct rproc *rproc) } } } else { + /* do not allow core 0 to stop before core 1 */ + core1 = list_last_entry(&cluster->cores, struct k3_r5_core, + elem); + if (core != core1 && core1->rproc->state != RPROC_OFFLINE) { + dev_err(dev, "%s: can not stop core 0 before core 1\n", + __func__); + return -EPERM; + } + ret = k3_r5_core_halt(core); if (ret) goto out;
PSC controller has a limitation that it can only power-up the second core when the first core is in ON state. Power-state for core0 should be equal to or higher than core1. Therefore, prevent core1 from powering up before core0 during the start process from sysfs. Similarly, prevent core0 from shutting down before core1 has been shut down from sysfs. Fixes: 6dedbd1d5443 ("remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem") Signed-off-by: Beleswar Padhi <b-padhi@ti.com> --- drivers/remoteproc/ti_k3_r5_remoteproc.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)