openamp: change rx buffer hold flag to count

[CV-Bowen]

This commit fix one issue, before this commit, if rpmsg_hold_rx_buffer() is called in the endpoint callback to hold current received buffer and call rpmsg_release_rx_buffer() to release this buffer immediately.
This buffer will be returned to the virtqueue in rpmsg_virtio_release_rx_buffer() and returned to virtqueue again in rpmsg_virtio_return_buffer() after the ept->cb().
So same buffer returned to the virtqueue twice, error happened.
Follow shows this process:

rpmsg_virtio_rx_callback()
  - get rp_hdr
  - ept->cb(data = RPMSG_LOCATE_DATA(rp_hdr))
    - rpsmg_hold_rx_buffer(data)
    - rpmsg_release_rx_buffer(data) here has returned buffer to virtqueue
  - rpmsg_virtio_return_buffer() return same buffer to virtqueue again

The root cause of this issue is that the RPMSG_BUF_HELD has been cleared in rpmsg_release_rx_buffer() so rpmsg_virtio_rx_callback() will treat this buffer is not HELD and call rpmsg_virtio_return_buffer() to return this buffer again.

This commit change the HELD flag in rp_hdr to count to fix this issue and also support hold/release rx buffer recursively.

[arnopo]

Strange to me that you hold the buffer to release it in same function. I would say hold it at the end of the ept->cb if needed...
That say I think it is interesting to use a counter instead of a flag, as we could have several consumers.

What I'm expecting here is that we have same hold mechanism for the RX and the TX buffers. So please update also the TX part

[CV-Bowen]

Strange to me that you hold the buffer to release it in same function. I would say hold it at the end of the ept->cb if needed... That say I think it is interesting to use a counter instead of a flag, as we could have several consumers.

What I'm expecting here is that we have same hold mechanism for the RX and the TX buffers. So please update also the TX part

@arnopo Hi, I'm a little confused about the update also the TX part, do you mean add rpmsg_virtio_hold_tx_buffer() in rpmsg_virtio and rpmsg_hold_tx_buffer() in rpmsg, and also add hold_tx_buffer ops to the struct rpmsg_device_ops to make user can call this function?

[arnopo]

Strange to me that you hold the buffer to release it in same function. I would say hold it at the end of the ept->cb if needed... That say I think it is interesting to use a counter instead of a flag, as we could have several consumers.
What I'm expecting here is that we have same hold mechanism for the RX and the TX buffers. So please update also the TX part

@arnopo Hi, I'm a little confused about the update also the TX part, do you mean add rpmsg_virtio_hold_tx_buffer() in rpmsg_virtio and rpmsg_hold_tx_buffer() in rpmsg, and also add hold_tx_buffer ops to the struct rpmsg_device_ops to make user can call this function?

My request was not cristal clear.
No need to add user interface to hold TX buffer. my point was just to use RPMSG_BUF_HELD_INC() and RPMSG_BUF_HELD_DEC() internally for the TX buffers.

[CV-Bowen]

Strange to me that you hold the buffer to release it in same function. I would say hold it at the end of the ept->cb if needed... That say I think it is interesting to use a counter instead of a flag, as we could have several consumers.
What I'm expecting here is that we have same hold mechanism for the RX and the TX buffers. So please update also the TX part

@arnopo Hi, I'm a little confused about the update also the TX part, do you mean add rpmsg_virtio_hold_tx_buffer() in rpmsg_virtio and rpmsg_hold_tx_buffer() in rpmsg, and also add hold_tx_buffer ops to the struct rpmsg_device_ops to make user can call this function?

My request was not cristal clear. No need to add user interface to hold TX buffer. my point was just to use RPMSG_BUF_HELD_INC() and RPMSG_BUF_HELD_DEC() internally for the TX buffers.

@arnopo Got, and I has deleted the user interface. Could you take a look again.

[arnopo]

can be less than 0 ?

[CV-Bowen]

Release one buffer twice? Actually, I want to add a RPMSG_ASSERT here to check the incorrect usage of buffer hold/release API. Do you have some suggestions?

[arnopo]

I suppose thta the issue is that user call twice time rpmsg_virtio_buf_held_dec_test, right?
so a test could be added in rpmsg_virtio_buf_held_dec_test with a log error

[CV-Bowen]

Done

[xiaoxiang781216]

@arnopo do you have more suggestion?

[arnopo]

LGTM adressing @xiaoxiang781216 comment

[edmooring]

Looks good to me.

[arnopo]

Minor typos to fix before I merge it.
you can also remove "This commit fix one issue, " in the commit message

[CV-Bowen]

@arnopo Thanks, all the comments have been addressed.