Skip to content

Conversation

@Hadatko
Copy link
Member

@Hadatko Hadatko commented Oct 21, 2019

This function adds deinit function for RPMSG LITE TTY transport. @MichalPrincNXP could you provide feedback please. Is it ok to deinitialize master/remote tty RPMSG instance like this?

@Hadatko Hadatko force-pushed the add_rpmsg_tty_deinit branch from 19007f9 to dbc27bc Compare October 21, 2019 12:57
@MichalPrincNXP
Copy link
Member

@Hadatko, the order of deinitialization is not correct. It should be done in this order:
rpmsg_lite_destroy_ept(s_rpmsg, m_rpmsg_ept);
rpmsg_queue_destroy(s_rpmsg, m_rpmsg_queue);
rpmsg_lite_deinit(s_rpmsg);
RL_SUCCESS return value should be checked and if not returned the kErpcStatus_DeinitFailed (newly introduced in your PR) should be used

@Hadatko
Copy link
Member Author

Hadatko commented Oct 21, 2019

What should be done when deinit failed in destructor?

@MichalPrincNXP
Copy link
Member

What about to issue a global error or just keep the s_initialized = 1?

@Hadatko
Copy link
Member Author

Hadatko commented Oct 21, 2019

s_initialized to one seems good. And thanks for point me to missing call function

@Hadatko
Copy link
Member Author

Hadatko commented Oct 21, 2019

Should be fine now. Please recheck.

@MichalPrincNXP
Copy link
Member

additional comments:
a. what about renaming void erpc_transport_rpmsg_lite_tty_rtos_deinit(void) setup function to void erpc_transport_rpmsg_lite_tty_rtos_**remote_**deinit(void) - for RPMSG lite tty rtos transport we have just implementation for remote
b. to behave consistently, other rpmsg_lite - based transport layers and setup functions should be updated accordingly (deinitialization provided) . Will you be able to do so?
c. how about copyright update?

@Hadatko
Copy link
Member Author

Hadatko commented Oct 21, 2019

a. Well it depends. rpmsg denit function looks common for master and remote. So for other eRPC RPMSG transport layers can be created also deinit functions which will be common for both master/remote. So in this context it make sense to keep it this way.
Or we can create remote/master deinit functions always which will call static(private) common deinit function
b. Actually i can't right now. I would like to keep this pull request simple. For case which is bothering us.
c. Am i responsible for copyright update? If yes i will update it accordingly.

@MichalPrincNXP MichalPrincNXP merged commit 74b2811 into EmbeddedRPC:develop Oct 23, 2019
@MichalPrincNXP
Copy link
Member

Thank you, Dusan.

@Hadatko Hadatko deleted the add_rpmsg_tty_deinit branch October 23, 2019 13:42
Hadatko added a commit that referenced this pull request Sep 23, 2020
Add deinit function for rpmsg tty transport. #78
Hadatko added a commit to Hadatko/erpc that referenced this pull request Sep 23, 2020
Add deinit function for rpmsg tty transport. EmbeddedRPC#78
Hadatko added a commit to Hadatko/erpc that referenced this pull request Sep 23, 2020
Add deinit function for rpmsg tty transport. EmbeddedRPC#78
CBohler99 pushed a commit to VERO-Biotech/erpc that referenced this pull request Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants