michallenc commented on code in PR #13127: URL: https://github.com/apache/nuttx/pull/13127#discussion_r1730823158
########## include/nuttx/can/can.h: ########## @@ -246,6 +257,46 @@ * is returned with the errno variable set to indicate the * nature of the error. * Dependencies: None + * + * CANIOC_SET_STATE Review Comment: How will this work? If I pass `CAN_STATE_STOP` via the ioctl then I stop the controller? What will happen with currently queued frames? Will the frame currently being transferred aborted or will the ioctlc all wait for the driver to finish? I generally think it is better to have a separate call to start or stop the controller. And then an ioctl to obtain current controller's state (including error active, passive, etc.). ########## include/nuttx/can/can.h: ########## @@ -263,6 +314,10 @@ #define CANIOC_IFLUSH _CANIOC(13) #define CANIOC_OFLUSH _CANIOC(14) #define CANIOC_IOFLUSH _CANIOC(15) +#define CANIOC_SET_STATE _CANIOC(16) +#define CANIOC_GET_STATE _CANIOC(17) +#define CANIOC_SET_TRANSVSTATE _CANIOC(18) +#define CANIOC_GET_TRANSVSTATE _CANIOC(19) #define CAN_FIRST 0x0001 /* First common command */ #define CAN_NCMDS 15 /* 16 common commands */ Review Comment: Update `CAN_NCMDS` to 19 to avoid collisions with driver specific calls. ########## include/nuttx/can/can.h: ########## @@ -438,6 +493,30 @@ #define CAN_FILTER_DUAL 1 /* Dual address match */ #define CAN_FILTER_RANGE 2 /* Match a range of addresses */ +/* the state is default state. Indicates that the can controller is closed */ + +#define CAN_STATE_STOP 0 + +/* Indicates that the can controller is in the awake state */ + +#define CAN_STATE_START 1 Review Comment: Maybe we can also add `ERROR` states (passive, active etc)? ########## include/nuttx/can/can.h: ########## @@ -246,6 +257,46 @@ * is returned with the errno variable set to indicate the * nature of the error. * Dependencies: None + * + * CANIOC_SET_STATE + * Description: Set specfic can controller state + * + * Argument: A pointer to an int type that describes the CAN + * controller state. + * returned Value: Zero (OK) is returned on success. Otherwise -1 (ERROR) + * is returned with the errno variable set to indicate the + * nature of the error. + * Dependencies: None + * + * CANIOC_GET_STATE + * Description: Get specfic can controller state + * + * Argument: A pointer to an int type that describes the CAN + * controller state. + * returned Value: Zero (OK) is returned on success. Otherwise -1 (ERROR) + * is returned with the errno variable set to indicate the + * nature of the error. + * Dependencies: None + * + * CANIOC_SET_TRANSV_STATE + * Description: Set specfic can transceiver state + * + * Argument: A pointer to an int type that describes the CAN Review Comment: Same, avoid passing pointer. ########## include/nuttx/can/can.h: ########## @@ -246,6 +257,46 @@ * is returned with the errno variable set to indicate the * nature of the error. * Dependencies: None + * + * CANIOC_SET_STATE + * Description: Set specfic can controller state + * + * Argument: A pointer to an int type that describes the CAN + * controller state. + * returned Value: Zero (OK) is returned on success. Otherwise -1 (ERROR) + * is returned with the errno variable set to indicate the + * nature of the error. + * Dependencies: None + * + * CANIOC_GET_STATE + * Description: Get specfic can controller state + * + * Argument: A pointer to an int type that describes the CAN Review Comment: We cannot avoid pointer here, but maybe the state can be passed as a returned value. I am not convinced which is better for NuttX. We used the returned value to obtain the controller state/info in RTEMS CAN stack implementation. ########## include/nuttx/can/can.h: ########## @@ -246,6 +257,46 @@ * is returned with the errno variable set to indicate the * nature of the error. * Dependencies: None + * + * CANIOC_SET_STATE + * Description: Set specfic can controller state + * + * Argument: A pointer to an int type that describes the CAN Review Comment: It is better to avoid passing pointers in ioctl calls. We can just pass integer here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
