Hello all,
I found out that there is discrepancy between remoteproc state definition in kernel and open-amp library:
Linux kernel side definition:
https://github.com/torvalds/linux/blob/52da431bf03b5506203bca27fe14a97895c80...
enum rproc_state { RPROC_OFFLINE = 0, RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, RPROC_DELETED = 4, RPROC_ATTACHED = 5, RPROC_DETACHED = 6, RPROC_LAST = 7, };
open-amp library side definition:
https://github.com/OpenAMP/open-amp/blob/391671ba24840833d882c1a75c5d7307703...
/** * @brief Remote processor states */ enum remoteproc_state { /** Remote is offline */ RPROC_OFFLINE = 0, /** Remote is configured */ RPROC_CONFIGURED = 1, /** Remote is ready to start */ RPROC_READY = 2, /** Remote is up and running */ RPROC_RUNNING = 3, /** Remote is suspended */ RPROC_SUSPENDED = 4, /** Remote is has error; need to recover */ RPROC_ERROR = 5, /** Remote is stopped */ RPROC_STOPPED = 6, /** Just keep this one at the end */ RPROC_LAST = 7, };
IIUC, both side state definition should match, so that if remote needs to report crash error, it can use same name and mapped int value in the code. Please let me know if I am missing something in this understanding.
Should we fix this?
If so, I believe it should be library side.
I am looking for suggestion to fix this without breaking backward compatibility:
Approach 1: deprecate library side remoteproc_state definition.
deprecate current enum (with __deprecated attribute or add comment) and introduce new one with same definition as in linux kernel. Then after 2 year (as per code of conduct policy) we can remove remoteproc_state.
Approach 2: Platform driver uses library side remoteproc state definition.
If we don't want to fix this, then another approach is, platform driver in linux kernel should have library-side remoteproc state definition and convert interprete it to kernel side remoteproc definition.
With this second approach we don't have to deprecate anything, but platform drivers are responsible to maintain different remoteproc_state definition. (Might create confusion).
I am open to any other suggestion regarding this.
Thanks, Tanmay
Hi Tanmay,
-----Original Message----- From: Tanmay Shah via Openamp-rp <openamp- rp@lists.openampproject.org> Sent: Wednesday, June 18, 2025 6:45 PM To: openamp-rp@lists.openampproject.org Cc: Arnaud POULIQUEN - foss arnaud.pouliquen@foss.st.com; bill.mills@linaro.org; Mathieu Poirier mathieu.poirier@linaro.org Subject: [Openamp-rp] [RFC]: remoteproc state definition discrepancy in openamp library and Linux kernel framework
Hello all,
I found out that there is discrepancy between remoteproc state definition in kernel and open-amp library:
Linux kernel side definition:
https://github.com/torvalds/linux/blob/52da431bf03b5506203bca27fe14a978 95c80faf/include/linux/remoteproc.h#L428
enum rproc_state { RPROC_OFFLINE = 0, RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, RPROC_DELETED = 4, RPROC_ATTACHED = 5, RPROC_DETACHED = 6, RPROC_LAST = 7, };
open-amp library side definition:
https://github.com/OpenAMP/open- amp/blob/391671ba24840833d882c1a75c5d7307703b1cf1/lib/include/opena mp/remoteproc.h#L534
/**
- @brief Remote processor states
*/ enum remoteproc_state { /** Remote is offline */ RPROC_OFFLINE = 0, /** Remote is configured */ RPROC_CONFIGURED = 1, /** Remote is ready to start */ RPROC_READY = 2, /** Remote is up and running */ RPROC_RUNNING = 3, /** Remote is suspended */ RPROC_SUSPENDED = 4, /** Remote is has error; need to recover */ RPROC_ERROR = 5, /** Remote is stopped */ RPROC_STOPPED = 6, /** Just keep this one at the end */ RPROC_LAST = 7, };
IIUC, both side state definition should match, so that if remote needs to report crash error, it can use same name and mapped int value in the code. Please let me know if I am missing something in this understanding.
These states are internal state of the remoteproc Frameworks. At first glance, I see no reason to align them. Do you see an issue ?
Regards, Arnaud
Should we fix this?
If so, I believe it should be library side.
I am looking for suggestion to fix this without breaking backward compatibility:
Approach 1: deprecate library side remoteproc_state definition.
deprecate current enum (with __deprecated attribute or add comment) and introduce new one with same definition as in linux kernel. Then after 2 year (as per code of conduct policy) we can remove remoteproc_state.
Approach 2: Platform driver uses library side remoteproc state definition.
If we don't want to fix this, then another approach is, platform driver in linux kernel should have library-side remoteproc state definition and convert interprete it to kernel side remoteproc definition.
With this second approach we don't have to deprecate anything, but platform drivers are responsible to maintain different remoteproc_state definition. (Might create confusion).
I am open to any other suggestion regarding this.
Thanks, Tanmay
-- Openamp-rp mailing list -- openamp-rp@lists.openampproject.org To unsubscribe send an email to openamp-rp- leave@lists.openampproject.org
Hi Tanmay,
On 19/06/25 13:45, Arnaud POULIQUEN via Openamp-rp wrote:
Hi Tanmay,
-----Original Message----- From: Tanmay Shah via Openamp-rp <openamp- rp@lists.openampproject.org> Sent: Wednesday, June 18, 2025 6:45 PM To: openamp-rp@lists.openampproject.org Cc: Arnaud POULIQUEN - foss arnaud.pouliquen@foss.st.com; bill.mills@linaro.org; Mathieu Poirier mathieu.poirier@linaro.org Subject: [Openamp-rp] [RFC]: remoteproc state definition discrepancy in openamp library and Linux kernel framework
Hello all,
I found out that there is discrepancy between remoteproc state definition in kernel and open-amp library:
Linux kernel side definition:
https://github.com/torvalds/linux/blob/52da431bf03b5506203bca27fe14a978 95c80faf/include/linux/remoteproc.h#L428
enum rproc_state { RPROC_OFFLINE = 0, RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, RPROC_DELETED = 4, RPROC_ATTACHED = 5, RPROC_DETACHED = 6, RPROC_LAST = 7, };
open-amp library side definition:
https://github.com/OpenAMP/open- amp/blob/391671ba24840833d882c1a75c5d7307703b1cf1/lib/include/opena mp/remoteproc.h#L534
/**
- @brief Remote processor states
*/ enum remoteproc_state { /** Remote is offline */ RPROC_OFFLINE = 0, /** Remote is configured */ RPROC_CONFIGURED = 1, /** Remote is ready to start */ RPROC_READY = 2, /** Remote is up and running */ RPROC_RUNNING = 3, /** Remote is suspended */ RPROC_SUSPENDED = 4, /** Remote is has error; need to recover */ RPROC_ERROR = 5, /** Remote is stopped */ RPROC_STOPPED = 6, /** Just keep this one at the end */ RPROC_LAST = 7, };
IIUC, both side state definition should match, so that if remote needs to report crash error, it can use same name and mapped int value in the code.
Currently for crash management, implementations send a crash type to the rproc crash handler for reporting. But rproc->state will always be set to its internal RPROC_CRASHED state.
If it was the case that it was rproc->state was being populated from the argument sent by implementations, then matching these structs makes sense to me. What do you think?
Please let me know if I am missing something in this understanding.
These states are internal state of the remoteproc Frameworks. At first glance, I see no reason to align them. Do you see an issue ?
+1..
Thanks, Beleswar
Regards, Arnaud
Should we fix this?
If so, I believe it should be library side.
I am looking for suggestion to fix this without breaking backward compatibility:
Approach 1: deprecate library side remoteproc_state definition.
deprecate current enum (with __deprecated attribute or add comment) and introduce new one with same definition as in linux kernel. Then after 2 year (as per code of conduct policy) we can remove remoteproc_state.
Approach 2: Platform driver uses library side remoteproc state definition.
If we don't want to fix this, then another approach is, platform driver in linux kernel should have library-side remoteproc state definition and convert interprete it to kernel side remoteproc definition.
With this second approach we don't have to deprecate anything, but platform drivers are responsible to maintain different remoteproc_state definition. (Might create confusion).
I am open to any other suggestion regarding this.
Thanks, Tanmay
-- Openamp-rp mailing list -- openamp-rp@lists.openampproject.org To unsubscribe send an email to openamp-rp- leave@lists.openampproject.org
Thank You Arnaud and Beleswar.
I was trying to notify the state of remote processor to host. And so was using remoterpoc_state for that, but looks like that shouldn't be the approach.
As there is no standard process to notify crash to host processor, then I think I need to introduce vendor specific enum in rproc firmware to report the crash and use that in the platform driver.
Do you think it's reasnoable to introduce same crash reason enum as kernel in remoteproc.h like following ?
enum rproc_crash_type { RPROC_MMUFAULT, RPROC_WATCHDOG, RPROC_FATAL_ERROR, };
Or each vendor must define and use this as needed?
Tanmay
On 6/19/25 4:14 AM, Beleswar Prasad Padhi wrote:
Hi Tanmay,
On 19/06/25 13:45, Arnaud POULIQUEN via Openamp-rp wrote:
Hi Tanmay,
-----Original Message----- From: Tanmay Shah via Openamp-rp <openamp- rp@lists.openampproject.org> Sent: Wednesday, June 18, 2025 6:45 PM To: openamp-rp@lists.openampproject.org Cc: Arnaud POULIQUEN - foss arnaud.pouliquen@foss.st.com; bill.mills@linaro.org; Mathieu Poirier mathieu.poirier@linaro.org Subject: [Openamp-rp] [RFC]: remoteproc state definition discrepancy in openamp library and Linux kernel framework
Hello all,
I found out that there is discrepancy between remoteproc state definition in kernel and open-amp library:
Linux kernel side definition:
https://github.com/torvalds/linux/blob/52da431bf03b5506203bca27fe14a978 95c80faf/include/linux/remoteproc.h#L428
enum rproc_state { RPROC_OFFLINE = 0, RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, RPROC_DELETED = 4, RPROC_ATTACHED = 5, RPROC_DETACHED = 6, RPROC_LAST = 7, };
open-amp library side definition:
https://github.com/OpenAMP/open- amp/blob/391671ba24840833d882c1a75c5d7307703b1cf1/lib/include/opena mp/remoteproc.h#L534
/**
- @brief Remote processor states
*/ enum remoteproc_state { /** Remote is offline */ RPROC_OFFLINE = 0, /** Remote is configured */ RPROC_CONFIGURED = 1, /** Remote is ready to start */ RPROC_READY = 2, /** Remote is up and running */ RPROC_RUNNING = 3, /** Remote is suspended */ RPROC_SUSPENDED = 4, /** Remote is has error; need to recover */ RPROC_ERROR = 5, /** Remote is stopped */ RPROC_STOPPED = 6, /** Just keep this one at the end */ RPROC_LAST = 7, };
IIUC, both side state definition should match, so that if remote needs to report crash error, it can use same name and mapped int value in the code.
Currently for crash management, implementations send a crash type to the rproc crash handler for reporting. But rproc->state will always be set to its internal RPROC_CRASHED state.
If it was the case that it was rproc->state was being populated from the argument sent by implementations, then matching these structs makes sense to me. What do you think?
Please let me know if I am missing something in this understanding.
These states are internal state of the remoteproc Frameworks. At first glance, I see no reason to align them. Do you see an issue ?
+1..
Thanks, Beleswar
Regards, Arnaud
Should we fix this?
If so, I believe it should be library side.
I am looking for suggestion to fix this without breaking backward compatibility:
Approach 1: deprecate library side remoteproc_state definition.
deprecate current enum (with __deprecated attribute or add comment) and introduce new one with same definition as in linux kernel. Then after 2 year (as per code of conduct policy) we can remove remoteproc_state.
Approach 2: Platform driver uses library side remoteproc state definition.
If we don't want to fix this, then another approach is, platform driver in linux kernel should have library-side remoteproc state definition and convert interprete it to kernel side remoteproc definition.
With this second approach we don't have to deprecate anything, but platform drivers are responsible to maintain different remoteproc_state definition. (Might create confusion).
I am open to any other suggestion regarding this.
Thanks, Tanmay
-- Openamp-rp mailing list -- openamp-rp@lists.openampproject.org To unsubscribe send an email to openamp-rp- leave@lists.openampproject.org
openamp-rp@lists.openampproject.org