remoteproc_virtio_notify return status not used

[tammyleino]

The remoteproc_virtio_notify function is used in functions defined in remoteproc_virtio.c as rpvdev->notify. The implementation is ultimately defined by the BSP. This function returns the status of the operation but the status is not checked by any of the calling routines. Should the return status of remoteproc_virtio_notify be changed to void?

[edmooring]

I hate losing potential error information, but I also can't see what the caller could sanely do in the face of an error. I am not opposed to changing the return status. However, this is an API change that will break existing code, so we have to be careful.

@arnopo Do you think we could do this with a CMake setting?

[arnopo]

I can see 2 issues:

1. The rpvdev->notify in remoteproc_virtio.c can be null, leading to crash

=> rpvdev->notify == 0 is valid if user doesn't use a mailbox to notify the remote processor. The call should be protected by if (rpvdev->notify)

2. The status is not checked.

=> what to do in case of error? This depends on the virtio backend implementation. The remoteproc_virtio is only one.
So change the API seems not a solution.
I would be in favor of testing the status in remoteproc_virtio.c and printing a warning.

[tammyleino]

@arnopo Yes, the function pointer should definitely be checked before using it. I will open a PR eventually for this and possibly other remoteproc op function pointers. I think logging the error will be fine for OpenAMP in general, but I think in my particular case, I need to not allow an error to be returned from the driver's notify routine. My driver is just triggering an inter-core interrupt via a register, which won't fail. The potential fail case is if the incoming remoteproc pointer is null. This should have been caught at the API level before getting all the way to the driver.

[github-actions]

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