Fix port binding commands issues. #42
Conversation
cli/main.c
Outdated
| int state = (status.bind_state & 0xF0) >> 4; | ||
|
|
||
| switch (result) { | ||
| case 0: |
There was a problem hiding this comment.
We should probably have an enum for these case values somewhere.
cli/main.c
Outdated
| @@ -940,23 +962,19 @@ static int port_bind_info(int argc, char **argv) | |||
|
|
|||
| ret = switchtec_bind_info(cfg.dev, &bind_status, cfg.phy_port); | |||
|
|
|||
| if (ret < 0) { | |||
| printf("bind_info_error: %f", ret); | |||
| if (ret > 0) { | |||
There was a problem hiding this comment.
You still need to handle the case where switchtec_bind_info() returns less than zero (indicating an error submitting the command or something like that.
There was a problem hiding this comment.
The function switchtec_bind_info()/switchtec_bind()/switchtec_unbind() cannot return a value that is less than zero. If the return value is bigger than zero, it indicates a error.
There was a problem hiding this comment.
That's wrong. switchtec_bind_info() calls switchtec_cmd() and returns the result. switchtec_cmd() can always return negative numbers.
cli/main.c
Outdated
| if (ret < 0) { | ||
| printf("bind_info_error: %f", ret); | ||
| if (ret > 0) { | ||
| printf("bind_info_error: %d\n", ret); |
There was a problem hiding this comment.
Probably should also be using switchtec_perror()
There was a problem hiding this comment.
Because same error codes were used by different MRPC commands and have different meaning in firmware. so we just display the error code now.
There was a problem hiding this comment.
Well returning a number is incredibly user unfriendly. You should fix that and decode the number into a useful error message. If you can use switchtec_perror() that would be great, if not you should create something else. switchtec_bind_perror() or something.
There was a problem hiding this comment.
Use the switchtec_perror() to handle the errorno. Thanks
ghost
force-pushed
the
bugfix_bind_info
branch
3 times, most recently
from
401582d
to
6a6b075
Compare
| struct switchtec_bind_status_in sub_cmd_id = { | ||
| .sub_cmd = MRPC_PORT_INFO, | ||
| .phys_port_id = phy_port | ||
| }; | ||
|
|
||
| ret = switchtec_cmd(dev, MRPC_PORTPARTP2P, &sub_cmd_id, | ||
| return switchtec_cmd(dev, MRPC_PORTPARTP2P, &sub_cmd_id, |
There was a problem hiding this comment.
Nice clean up, but can this be another commit?
The output of command port-bind-info does not display strings for bind state. Port binding commands use wrong condition to check error and do not display strings for error code.
ghost
force-pushed
the
bugfix_bind_info
branch
from
6a6b075
to
8fe9019
Compare
|
Thanks for the review. I has split the commit. Thanks |
|
Following enum overlap the system error number: ERR_PHYC_PORT_ARDY_BIND = 0x00000001, Which lead to false prompt in case of non-port-bind related commands by following code in switchtec_strerror function: |
The output of command port-bind-info does not display strings for
bind state and port binding commands use wrong condition for error
check.