| Message ID | 20241224091457.1050233-2-b-padhi@ti.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Rework TI K3 R5F remoteproc driver | expand |
On 12/24/24 03:14, Beleswar Padhi wrote: > /** > @@ -194,8 +196,11 @@ static void k3_r5_rproc_mbox_callback(struct mbox_client *client, void *data) > const char *name = kproc->rproc->name; > u32 msg = omap_mbox_message(data); > > - /* Do not forward message from a detached core */ > - if (kproc->rproc->state == RPROC_DETACHED) > + /* > + * Do not forward messages from a detached core, except when the core > + * is in the process of being attached in IPC-only mode. > + */ > + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == RPROC_DETACHED) > return; > Instead of introducing a new state variable, is it possible to use device virtio status? And i wonder what if you remove this conditional check altogether? If the device is detached and with no virtio queues, does not the mailbox message gets dropped?
Hi Hari, On 27/12/24 20:08, Hari Nagalla wrote: > On 12/24/24 03:14, Beleswar Padhi wrote: >> /** >> @@ -194,8 +196,11 @@ static void k3_r5_rproc_mbox_callback(struct >> mbox_client *client, void *data) >> const char *name = kproc->rproc->name; >> u32 msg = omap_mbox_message(data); >> - /* Do not forward message from a detached core */ >> - if (kproc->rproc->state == RPROC_DETACHED) >> + /* >> + * Do not forward messages from a detached core, except when the >> core >> + * is in the process of being attached in IPC-only mode. >> + */ >> + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == >> RPROC_DETACHED) >> return; > Instead of introducing a new state variable, is it possible to use > device virtio status? See below related comment. > And i wonder what if you remove this conditional check altogether? If > the device is detached and with no virtio queues, does not the mailbox > message gets dropped? This check is necessary for mailbox level communication between cores. Some Mbox messages directly use payloads like RP_MBOX_ECHO_REQUEST/RP_MBOX_CRASH etc. which do not rely on virtqueue read/writes for communication (see omap_remoteproc.h). In that case, mailbox message won't be dropped even if virtio queues are not initialized. IMO, when we say core is detached in "IPC-only" mode, this mbox communication should also not happen. What do you think?
On Tue, Dec 24, 2024 at 02:44:55PM +0530, Beleswar Padhi wrote: > Commit f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle during > probe routine") introduced a check in the "k3_r5_rproc_mbox_callback()" > and "k3_r5_rproc_kick()" callbacks to exit if the remote core's state > was "RPROC_DETACHED". However, this caused issues in IPC-only mode, as > the default state of the core is set to "RPROC_DETACHED", and the > transition to "RPROC_ATTACHED" happens only after the "__rproc_attach()" > function has invoked "rproc_start_subdevices()". > > The "rproc_start_subdevices()" function triggers the probe of Virtio > RPMsg subdevices, which require the mailbox callbacks to be functional. > To resolve this, a new variable, "is_attach_ongoing", is introduced to > distinguish between core states: when a core is actually detached and > when it is in the process of being attached. The callbacks are updated > to return early only if the core is actually detached and not during an > ongoing attach operation in IPC-only mode. > > Reported-by: Siddharth Vadapalli <s-vadapalli@ti.com> > Closes: https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ > Fixes: f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle during probe routine") > Signed-off-by: Beleswar Padhi <b-padhi@ti.com> Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com> Regards, Siddharth.
On 12/24/2024 2:44 PM, Beleswar Padhi wrote: > Commit f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle during > probe routine") introduced a check in the "k3_r5_rproc_mbox_callback()" > and "k3_r5_rproc_kick()" callbacks to exit if the remote core's state > was "RPROC_DETACHED". However, this caused issues in IPC-only mode, as > the default state of the core is set to "RPROC_DETACHED", and the > transition to "RPROC_ATTACHED" happens only after the "__rproc_attach()" > function has invoked "rproc_start_subdevices()". > > The "rproc_start_subdevices()" function triggers the probe of Virtio > RPMsg subdevices, which require the mailbox callbacks to be functional. > To resolve this, a new variable, "is_attach_ongoing", is introduced to > distinguish between core states: when a core is actually detached and > when it is in the process of being attached. The callbacks are updated > to return early only if the core is actually detached and not during an > ongoing attach operation in IPC-only mode. > > Reported-by: Siddharth Vadapalli <s-vadapalli@ti.com> > Closes: https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ > Fixes: f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle during probe routine") > Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > --- > Link to RFC version: > https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ > > Improvements in v1: > 1. Ensured these mbox callbacks are functional when the core is in the proccess > of getting attached in IPC-Only mode. > 2. Ensured these mbox callbacks are _not_ functional when the core state is > actually detached. > > drivers/remoteproc/ti_k3_r5_remoteproc.c | 53 +++++++++++++++++------- > 1 file changed, 39 insertions(+), 14 deletions(-) > > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > index dbc513c5569c..e218a803fdb5 100644 > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > @@ -131,6 +131,7 @@ struct k3_r5_cluster { > * @btcm_enable: flag to control BTCM enablement > * @loczrama: flag to dictate which TCM is at device address 0x0 > * @released_from_reset: flag to signal when core is out of reset > + * @is_attach_ongoing: flag to indicate if IPC-only "attach()" is in progress Variable name is misleading, Core are attached after call to rproc_add from this driver. but variables says still attach_ongoing , I suggest to use is_attached > */ > struct k3_r5_core { > struct list_head elem; > @@ -148,6 +149,7 @@ struct k3_r5_core { > u32 btcm_enable; > u32 loczrama; > bool released_from_reset; > + bool is_attach_ongoing; > }; > > /** > @@ -194,8 +196,11 @@ static void k3_r5_rproc_mbox_callback(struct mbox_client *client, void *data) > const char *name = kproc->rproc->name; > u32 msg = omap_mbox_message(data); > > - /* Do not forward message from a detached core */ > - if (kproc->rproc->state == RPROC_DETACHED) > + /* > + * Do not forward messages from a detached core, except when the core > + * is in the process of being attached in IPC-only mode. > + */ > + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == RPROC_DETACHED) > return; > > dev_dbg(dev, "mbox msg: 0x%x\n", msg); > @@ -233,8 +238,11 @@ static void k3_r5_rproc_kick(struct rproc *rproc, int vqid) > mbox_msg_t msg = (mbox_msg_t)vqid; > int ret; > > - /* Do not forward message to a detached core */ > - if (kproc->rproc->state == RPROC_DETACHED) > + /* > + * Do not forward messages to a detached core, except when the core is > + * in the process of being attached in IPC-only mode. > + */ > + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == RPROC_DETACHED) > return; > > /* send the index of the triggered virtqueue in the mailbox payload */ > @@ -671,22 +679,39 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > /* > * Attach to a running R5F remote processor (IPC-only mode) > * > - * The R5F attach callback is a NOP. The remote processor is already booted, and > - * all required resources have been acquired during probe routine, so there is > - * no need to issue any TI-SCI commands to boot the R5F cores in IPC-only mode. > - * This callback is invoked only in IPC-only mode and exists because > - * rproc_validate() checks for its existence. > + * The R5F attach callback only needs to set the "is_attach_ongoing" flag to > + * notify k3_r5_rproc_{kick/mbox_callback} functions that the core is in the > + * process of getting attached in IPC-only mode. The remote processor is > + * already booted, and all required resources have been acquired during probe > + * routine, so there is no need to issue any TI-SCI commands to boot the R5F > + * cores in IPC-only mode. This callback is invoked only in IPC-only mode. > */ > -static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; } > +static int k3_r5_rproc_attach(struct rproc *rproc) > +{ > + struct k3_r5_rproc *kproc = rproc->priv; > + > + kproc->core->is_attach_ongoing = true; > + > + return 0; > +} > > /* > * Detach from a running R5F remote processor (IPC-only mode) > * > - * The R5F detach callback is a NOP. The R5F cores are not stopped and will be > - * left in booted state in IPC-only mode. This callback is invoked only in > - * IPC-only mode and exists for sanity sake. > + * The R5F detach callback performs the opposite operation to attach callback > + * and only needs to clear the "is_attach_ongoing" flag to ensure no mailbox > + * messages are sent to or received from a detached core. The R5F cores are > + * not stopped and will be left in booted state in IPC-only mode. This > + * callback is invoked only in IPC-only mode. > */ > -static int k3_r5_rproc_detach(struct rproc *rproc) { return 0; } > +static int k3_r5_rproc_detach(struct rproc *rproc) > +{ > + struct k3_r5_rproc *kproc = rproc->priv; > + > + kproc->core->is_attach_ongoing = false; > + > + return 0; > +} > > /* > * This function implements the .get_loaded_rsc_table() callback and is used
On 03/01/25 16:18, Kumar, Udit wrote: > > On 12/24/2024 2:44 PM, Beleswar Padhi wrote: >> Commit f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle during >> probe routine") introduced a check in the "k3_r5_rproc_mbox_callback()" >> and "k3_r5_rproc_kick()" callbacks to exit if the remote core's state >> was "RPROC_DETACHED". However, this caused issues in IPC-only mode, as >> the default state of the core is set to "RPROC_DETACHED", and the >> transition to "RPROC_ATTACHED" happens only after the "__rproc_attach()" >> function has invoked "rproc_start_subdevices()". >> >> The "rproc_start_subdevices()" function triggers the probe of Virtio >> RPMsg subdevices, which require the mailbox callbacks to be functional. >> To resolve this, a new variable, "is_attach_ongoing", is introduced to >> distinguish between core states: when a core is actually detached and >> when it is in the process of being attached. The callbacks are updated >> to return early only if the core is actually detached and not during an >> ongoing attach operation in IPC-only mode. >> >> Reported-by: Siddharth Vadapalli <s-vadapalli@ti.com> >> Closes: >> https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ >> Fixes: f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle >> during probe routine") >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> >> --- >> Link to RFC version: >> https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ >> >> Improvements in v1: >> 1. Ensured these mbox callbacks are functional when the core is in >> the proccess >> of getting attached in IPC-Only mode. >> 2. Ensured these mbox callbacks are _not_ functional when the core >> state is >> actually detached. >> >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 53 +++++++++++++++++------- >> 1 file changed, 39 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c >> b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> index dbc513c5569c..e218a803fdb5 100644 >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> @@ -131,6 +131,7 @@ struct k3_r5_cluster { >> * @btcm_enable: flag to control BTCM enablement >> * @loczrama: flag to dictate which TCM is at device address 0x0 >> * @released_from_reset: flag to signal when core is out of reset >> + * @is_attach_ongoing: flag to indicate if IPC-only "attach()" is in >> progress > > Variable name is misleading, Core are attached after call to rproc_add > from this driver. Right, but from all locations where this variable is checked (mbox_kick()/mbox_callback()), .attach would be still in progress. > > but variables says still attach_ongoing , I suggest to use is_attached Thought so, but that is confusing with rproc->state. E.g, in mbox_kick(), rproc->state = detached, but we are saying kproc->is_attached = true. What do you think? > > >> */ >> struct k3_r5_core { >> struct list_head elem; >> @@ -148,6 +149,7 @@ struct k3_r5_core { >> u32 btcm_enable; >> u32 loczrama; >> bool released_from_reset; >> + bool is_attach_ongoing; >> }; >> /** >> @@ -194,8 +196,11 @@ static void k3_r5_rproc_mbox_callback(struct >> mbox_client *client, void *data) >> const char *name = kproc->rproc->name; >> u32 msg = omap_mbox_message(data); >> - /* Do not forward message from a detached core */ >> - if (kproc->rproc->state == RPROC_DETACHED) >> + /* >> + * Do not forward messages from a detached core, except when the >> core >> + * is in the process of being attached in IPC-only mode. >> + */ >> + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == >> RPROC_DETACHED) >> return; >> dev_dbg(dev, "mbox msg: 0x%x\n", msg); >> @@ -233,8 +238,11 @@ static void k3_r5_rproc_kick(struct rproc >> *rproc, int vqid) >> mbox_msg_t msg = (mbox_msg_t)vqid; >> int ret; >> - /* Do not forward message to a detached core */ >> - if (kproc->rproc->state == RPROC_DETACHED) >> + /* >> + * Do not forward messages to a detached core, except when the >> core is >> + * in the process of being attached in IPC-only mode. >> + */ >> + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == >> RPROC_DETACHED) >> return; >> /* send the index of the triggered virtqueue in the mailbox >> payload */ >> @@ -671,22 +679,39 @@ static int k3_r5_rproc_stop(struct rproc *rproc) >> /* >> * Attach to a running R5F remote processor (IPC-only mode) >> * >> - * The R5F attach callback is a NOP. The remote processor is already >> booted, and >> - * all required resources have been acquired during probe routine, >> so there is >> - * no need to issue any TI-SCI commands to boot the R5F cores in >> IPC-only mode. >> - * This callback is invoked only in IPC-only mode and exists because >> - * rproc_validate() checks for its existence. >> + * The R5F attach callback only needs to set the "is_attach_ongoing" >> flag to >> + * notify k3_r5_rproc_{kick/mbox_callback} functions that the core >> is in the >> + * process of getting attached in IPC-only mode. The remote >> processor is >> + * already booted, and all required resources have been acquired >> during probe >> + * routine, so there is no need to issue any TI-SCI commands to boot >> the R5F >> + * cores in IPC-only mode. This callback is invoked only in IPC-only >> mode. >> */ >> -static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; } >> +static int k3_r5_rproc_attach(struct rproc *rproc) >> +{ >> + struct k3_r5_rproc *kproc = rproc->priv; >> + >> + kproc->core->is_attach_ongoing = true; >> + >> + return 0; >> +} >> /* >> * Detach from a running R5F remote processor (IPC-only mode) >> * >> - * The R5F detach callback is a NOP. The R5F cores are not stopped >> and will be >> - * left in booted state in IPC-only mode. This callback is invoked >> only in >> - * IPC-only mode and exists for sanity sake. >> + * The R5F detach callback performs the opposite operation to attach >> callback >> + * and only needs to clear the "is_attach_ongoing" flag to ensure no >> mailbox >> + * messages are sent to or received from a detached core. The R5F >> cores are >> + * not stopped and will be left in booted state in IPC-only mode. This >> + * callback is invoked only in IPC-only mode. >> */ >> -static int k3_r5_rproc_detach(struct rproc *rproc) { return 0; } >> +static int k3_r5_rproc_detach(struct rproc *rproc) >> +{ >> + struct k3_r5_rproc *kproc = rproc->priv; >> + >> + kproc->core->is_attach_ongoing = false; >> + >> + return 0; >> +} >> /* >> * This function implements the .get_loaded_rsc_table() callback >> and is used
On 1/3/2025 4:27 PM, Beleswar Prasad Padhi wrote: > > On 03/01/25 16:18, Kumar, Udit wrote: >> >> On 12/24/2024 2:44 PM, Beleswar Padhi wrote: >>> Commit f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle during >>> probe routine") introduced a check in the "k3_r5_rproc_mbox_callback()" >>> and "k3_r5_rproc_kick()" callbacks to exit if the remote core's state >>> was "RPROC_DETACHED". However, this caused issues in IPC-only mode, as >>> the default state of the core is set to "RPROC_DETACHED", and the >>> transition to "RPROC_ATTACHED" happens only after the >>> "__rproc_attach()" >>> function has invoked "rproc_start_subdevices()". >>> >>> The "rproc_start_subdevices()" function triggers the probe of Virtio >>> RPMsg subdevices, which require the mailbox callbacks to be functional. >>> To resolve this, a new variable, "is_attach_ongoing", is introduced to >>> distinguish between core states: when a core is actually detached and >>> when it is in the process of being attached. The callbacks are updated >>> to return early only if the core is actually detached and not during an >>> ongoing attach operation in IPC-only mode. >>> >>> Reported-by: Siddharth Vadapalli <s-vadapalli@ti.com> >>> Closes: >>> https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ >>> >>> Fixes: f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle >>> during probe routine") >>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> >>> --- >>> Link to RFC version: >>> https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ >>> >>> >>> Improvements in v1: >>> 1. Ensured these mbox callbacks are functional when the core is in >>> the proccess >>> of getting attached in IPC-Only mode. >>> 2. Ensured these mbox callbacks are _not_ functional when the core >>> state is >>> actually detached. >>> >>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 53 >>> +++++++++++++++++------- >>> 1 file changed, 39 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c >>> b/drivers/remoteproc/ti_k3_r5_remoteproc.c >>> index dbc513c5569c..e218a803fdb5 100644 >>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >>> @@ -131,6 +131,7 @@ struct k3_r5_cluster { >>> * @btcm_enable: flag to control BTCM enablement >>> * @loczrama: flag to dictate which TCM is at device address 0x0 >>> * @released_from_reset: flag to signal when core is out of reset >>> + * @is_attach_ongoing: flag to indicate if IPC-only "attach()" is >>> in progress >> >> Variable name is misleading, Core are attached after call to >> rproc_add from this driver. > > > Right, but from all locations where this variable is checked > (mbox_kick()/mbox_callback()), .attach would be still in progress. > >> >> but variables says still attach_ongoing , I suggest to use is_attached > > > Thought so, but that is confusing with rproc->state. E.g, in > mbox_kick(), rproc->state = detached, but we are saying > kproc->is_attached = true. > > What do you think? In code, you are doing same work, if core is attached. In remote proc core driver POV, you are is attaching state but action you are taking as attached. Since, variable is being internal to driver, i still suggest to use is_attached > >> >> >>> */ >>> struct k3_r5_core { >>> struct list_head elem; >>> @@ -148,6 +149,7 @@ struct k3_r5_core { >>> u32 btcm_enable; >>> u32 loczrama; >>> bool released_from_reset; >>> + bool is_attach_ongoing; >>> }; >>> /** >>> @@ -194,8 +196,11 @@ static void k3_r5_rproc_mbox_callback(struct >>> mbox_client *client, void *data) >>> const char *name = kproc->rproc->name; >>> u32 msg = omap_mbox_message(data); >>> - /* Do not forward message from a detached core */ >>> - if (kproc->rproc->state == RPROC_DETACHED) >>> + /* >>> + * Do not forward messages from a detached core, except when >>> the core >>> + * is in the process of being attached in IPC-only mode. >>> + */ >>> + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == >>> RPROC_DETACHED) >>> return; >>> dev_dbg(dev, "mbox msg: 0x%x\n", msg); >>> @@ -233,8 +238,11 @@ static void k3_r5_rproc_kick(struct rproc >>> *rproc, int vqid) >>> mbox_msg_t msg = (mbox_msg_t)vqid; >>> int ret; >>> - /* Do not forward message to a detached core */ >>> - if (kproc->rproc->state == RPROC_DETACHED) >>> + /* >>> + * Do not forward messages to a detached core, except when the >>> core is >>> + * in the process of being attached in IPC-only mode. >>> + */ >>> + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == >>> RPROC_DETACHED) >>> return; >>> /* send the index of the triggered virtqueue in the mailbox >>> payload */ >>> @@ -671,22 +679,39 @@ static int k3_r5_rproc_stop(struct rproc *rproc) >>> /* >>> * Attach to a running R5F remote processor (IPC-only mode) >>> * >>> - * The R5F attach callback is a NOP. The remote processor is >>> already booted, and >>> - * all required resources have been acquired during probe routine, >>> so there is >>> - * no need to issue any TI-SCI commands to boot the R5F cores in >>> IPC-only mode. >>> - * This callback is invoked only in IPC-only mode and exists because >>> - * rproc_validate() checks for its existence. >>> + * The R5F attach callback only needs to set the >>> "is_attach_ongoing" flag to >>> + * notify k3_r5_rproc_{kick/mbox_callback} functions that the core >>> is in the >>> + * process of getting attached in IPC-only mode. The remote >>> processor is >>> + * already booted, and all required resources have been acquired >>> during probe >>> + * routine, so there is no need to issue any TI-SCI commands to >>> boot the R5F >>> + * cores in IPC-only mode. This callback is invoked only in >>> IPC-only mode. >>> */ >>> -static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; } >>> +static int k3_r5_rproc_attach(struct rproc *rproc) >>> +{ >>> + struct k3_r5_rproc *kproc = rproc->priv; >>> + >>> + kproc->core->is_attach_ongoing = true; >>> + >>> + return 0; >>> +} >>> /* >>> * Detach from a running R5F remote processor (IPC-only mode) >>> * >>> - * The R5F detach callback is a NOP. The R5F cores are not stopped >>> and will be >>> - * left in booted state in IPC-only mode. This callback is invoked >>> only in >>> - * IPC-only mode and exists for sanity sake. >>> + * The R5F detach callback performs the opposite operation to >>> attach callback >>> + * and only needs to clear the "is_attach_ongoing" flag to ensure >>> no mailbox >>> + * messages are sent to or received from a detached core. The R5F >>> cores are >>> + * not stopped and will be left in booted state in IPC-only mode. This >>> + * callback is invoked only in IPC-only mode. >>> */ >>> -static int k3_r5_rproc_detach(struct rproc *rproc) { return 0; } >>> +static int k3_r5_rproc_detach(struct rproc *rproc) >>> +{ >>> + struct k3_r5_rproc *kproc = rproc->priv; >>> + >>> + kproc->core->is_attach_ongoing = false; >>> + >>> + return 0; >>> +} >>> /* >>> * This function implements the .get_loaded_rsc_table() callback >>> and is used
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index dbc513c5569c..e218a803fdb5 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -131,6 +131,7 @@ struct k3_r5_cluster { * @btcm_enable: flag to control BTCM enablement * @loczrama: flag to dictate which TCM is at device address 0x0 * @released_from_reset: flag to signal when core is out of reset + * @is_attach_ongoing: flag to indicate if IPC-only "attach()" is in progress */ struct k3_r5_core { struct list_head elem; @@ -148,6 +149,7 @@ struct k3_r5_core { u32 btcm_enable; u32 loczrama; bool released_from_reset; + bool is_attach_ongoing; }; /** @@ -194,8 +196,11 @@ static void k3_r5_rproc_mbox_callback(struct mbox_client *client, void *data) const char *name = kproc->rproc->name; u32 msg = omap_mbox_message(data); - /* Do not forward message from a detached core */ - if (kproc->rproc->state == RPROC_DETACHED) + /* + * Do not forward messages from a detached core, except when the core + * is in the process of being attached in IPC-only mode. + */ + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == RPROC_DETACHED) return; dev_dbg(dev, "mbox msg: 0x%x\n", msg); @@ -233,8 +238,11 @@ static void k3_r5_rproc_kick(struct rproc *rproc, int vqid) mbox_msg_t msg = (mbox_msg_t)vqid; int ret; - /* Do not forward message to a detached core */ - if (kproc->rproc->state == RPROC_DETACHED) + /* + * Do not forward messages to a detached core, except when the core is + * in the process of being attached in IPC-only mode. + */ + if (!kproc->core->is_attach_ongoing && kproc->rproc->state == RPROC_DETACHED) return; /* send the index of the triggered virtqueue in the mailbox payload */ @@ -671,22 +679,39 @@ static int k3_r5_rproc_stop(struct rproc *rproc) /* * Attach to a running R5F remote processor (IPC-only mode) * - * The R5F attach callback is a NOP. The remote processor is already booted, and - * all required resources have been acquired during probe routine, so there is - * no need to issue any TI-SCI commands to boot the R5F cores in IPC-only mode. - * This callback is invoked only in IPC-only mode and exists because - * rproc_validate() checks for its existence. + * The R5F attach callback only needs to set the "is_attach_ongoing" flag to + * notify k3_r5_rproc_{kick/mbox_callback} functions that the core is in the + * process of getting attached in IPC-only mode. The remote processor is + * already booted, and all required resources have been acquired during probe + * routine, so there is no need to issue any TI-SCI commands to boot the R5F + * cores in IPC-only mode. This callback is invoked only in IPC-only mode. */ -static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; } +static int k3_r5_rproc_attach(struct rproc *rproc) +{ + struct k3_r5_rproc *kproc = rproc->priv; + + kproc->core->is_attach_ongoing = true; + + return 0; +} /* * Detach from a running R5F remote processor (IPC-only mode) * - * The R5F detach callback is a NOP. The R5F cores are not stopped and will be - * left in booted state in IPC-only mode. This callback is invoked only in - * IPC-only mode and exists for sanity sake. + * The R5F detach callback performs the opposite operation to attach callback + * and only needs to clear the "is_attach_ongoing" flag to ensure no mailbox + * messages are sent to or received from a detached core. The R5F cores are + * not stopped and will be left in booted state in IPC-only mode. This + * callback is invoked only in IPC-only mode. */ -static int k3_r5_rproc_detach(struct rproc *rproc) { return 0; } +static int k3_r5_rproc_detach(struct rproc *rproc) +{ + struct k3_r5_rproc *kproc = rproc->priv; + + kproc->core->is_attach_ongoing = false; + + return 0; +} /* * This function implements the .get_loaded_rsc_table() callback and is used
Commit f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle during probe routine") introduced a check in the "k3_r5_rproc_mbox_callback()" and "k3_r5_rproc_kick()" callbacks to exit if the remote core's state was "RPROC_DETACHED". However, this caused issues in IPC-only mode, as the default state of the core is set to "RPROC_DETACHED", and the transition to "RPROC_ATTACHED" happens only after the "__rproc_attach()" function has invoked "rproc_start_subdevices()". The "rproc_start_subdevices()" function triggers the probe of Virtio RPMsg subdevices, which require the mailbox callbacks to be functional. To resolve this, a new variable, "is_attach_ongoing", is introduced to distinguish between core states: when a core is actually detached and when it is in the process of being attached. The callbacks are updated to return early only if the core is actually detached and not during an ongoing attach operation in IPC-only mode. Reported-by: Siddharth Vadapalli <s-vadapalli@ti.com> Closes: https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ Fixes: f3f11cfe8907 ("remoteproc: k3-r5: Acquire mailbox handle during probe routine") Signed-off-by: Beleswar Padhi <b-padhi@ti.com> --- Link to RFC version: https://lore.kernel.org/all/20240916083131.2801755-1-s-vadapalli@ti.com/ Improvements in v1: 1. Ensured these mbox callbacks are functional when the core is in the proccess of getting attached in IPC-Only mode. 2. Ensured these mbox callbacks are _not_ functional when the core state is actually detached. drivers/remoteproc/ti_k3_r5_remoteproc.c | 53 +++++++++++++++++------- 1 file changed, 39 insertions(+), 14 deletions(-)