openamp: add error log when ept->cb return error

[CV-Bowen]

Add helpful log when rpmsg endpoint callback return error.

[CV-Bowen]

@arnopo Hi, why the docs/readthedocs is always failed. Did I miss some things to do?

[arnopo]

@arnopo Hi, why the docs/readthedocs is always failed. Did I miss some things to do?

you should rebase your work on main branch, seems that your 6 commits behind

[CV-Bowen]

@arnopo Thanks, now CI passed.

[tnmysh]

Do we need to print ept->cb function pointer address ? only printing ept->name should be enough.

[tnmysh]

Also, you could use "ml_err" macro in place of "metal_log(METAL_LOG_ERROR",

[CV-Bowen]

After some thinking, only print ept->name is enough. Thanks.

[xiaoxiang781216]

@arnopo how about this patch?

[arnopo]

@arnopo how about this patch?

we have a duplication with the assert and the error message. The question is what is the expected behavior: Crash on error, just display an error message or both?

Another question is why this error message is not displayed in the callback itself? Seems to me that this would be more efficient for the debug. And the user can decide to print an error and return a status 0 to not crash the system.

an alternative could be to update the RPMSG_ASSERT macro to have variadic arguments, something like this ( not tested)

#define RPMSG_ASSERT(_exp, format, ...) do { \
		if (!(_exp)) { \
			metal_log(METAL_LOG_EMERGENCY, \
				  "FATAL: %s - " format,  __func__, ##__VA_ARGS__); \
			metal_assert(_exp); \
		} \
	} while (0)

but sound to me better to display the message in the endpoint callback

[CV-Bowen]

@arnopo OK, maybe add error log to the endpoint callback is better although it needs more works.

[github-actions]

This pull request has been marked as a stale pull request because it has been open (more than) 45 days with no activity.

[xiaoxiang781216]

@arnopo how about this patch?

we have a duplication with the assert and the error message. The question is what is the expected behavior: Crash on error, just display an error message or both?

Another question is why this error message is not displayed in the callback itself? Seems to me that this would be more efficient for the debug. And the user can decide to print an error and return a status 0 to not crash the system.

an alternative could be to update the RPMSG_ASSERT macro to have variadic arguments, something like this ( not tested)

#define RPMSG_ASSERT(_exp, format, ...) do { \
		if (!(_exp)) { \
			metal_log(METAL_LOG_EMERGENCY, \
				  "FATAL: %s - " format,  __func__, ##__VA_ARGS__); \
			metal_assert(_exp); \
		} \
	} while (0)

but sound to me better to display the message in the endpoint callback

@CV-Bowen let's update the patch as @arnopo suggestion or close this patch.

[github-actions]

This pull request has been marked as a stale pull request because it has been open (more than) 45 days with no activity.