From 12a44bd9744829813e3757389aad0ef6047e5ac9 Mon Sep 17 00:00:00 2001 From: gaohedong Date: Fri, 23 Aug 2024 18:10:27 +0800 Subject: [PATCH] can: CAN code optimization Some macro definitions have already been defined in other header files, redundant macro definitions have been removed in include/nuttx/can.h. Align some code. Remove no use struct (can_response_s) and some variables. Signed-off-by: Xiang Xiao --- drivers/can/can.c | 92 +++++++----------------- include/nuttx/can.h | 151 +--------------------------------------- include/nuttx/can/can.h | 28 ++++---- 3 files changed, 40 insertions(+), 231 deletions(-) diff --git a/drivers/can/can.c b/drivers/can/can.c index c2076ea4c0..ed804247c8 100644 --- a/drivers/can/can.c +++ b/drivers/can/can.c @@ -43,15 +43,8 @@ #include #include #include - -#ifdef CONFIG_CAN_TXREADY -# include -#endif - #include -#ifdef CONFIG_CAN - /**************************************************************************** * Pre-processor Definitions ****************************************************************************/ @@ -196,9 +189,6 @@ static FAR struct can_reader_s *init_can_reader(FAR struct file *filep) FAR struct can_reader_s *reader = kmm_zalloc(sizeof(struct can_reader_s)); DEBUGASSERT(reader != NULL); - reader->fifo.rx_head = 0; - reader->fifo.rx_tail = 0; - nxsem_init(&reader->fifo.rx_sem, 0, 0); filep->f_priv = reader; @@ -298,7 +288,6 @@ static int can_close(FAR struct file *filep) FAR struct can_dev_s *dev = inode->i_private; irqstate_t flags; FAR struct list_node *node; - FAR struct list_node *tmp; int ret; #ifdef CONFIG_DEBUG_CAN_INFO @@ -311,7 +300,7 @@ static int can_close(FAR struct file *filep) return ret; } - list_for_every_safe(&dev->cd_readers, node, tmp) + list_for_every(&dev->cd_readers, node) { if (((FAR struct can_reader_s *)node) == ((FAR struct can_reader_s *)filep->f_priv)) @@ -372,14 +361,13 @@ errout: static ssize_t can_read(FAR struct file *filep, FAR char *buffer, size_t buflen) { - FAR struct can_reader_s *reader; - FAR struct can_rxfifo_s *fifo; - size_t nread; - irqstate_t flags; - int ret = 0; + FAR struct can_reader_s *reader; + FAR struct can_rxfifo_s *fifo; + irqstate_t flags; + int ret = 0; #ifdef CONFIG_CAN_ERRORS - FAR struct inode *inode = filep->f_inode; - FAR struct can_dev_s *dev = inode->i_private; + FAR struct inode *inode = filep->f_inode; + FAR struct can_dev_s *dev = inode->i_private; #endif caninfo("buflen: %zu\n", buflen); @@ -393,7 +381,6 @@ static ssize_t can_read(FAR struct file *filep, FAR char *buffer, { DEBUGASSERT(filep->f_priv != NULL); reader = (FAR struct can_reader_s *)filep->f_priv; - fifo = &reader->fifo; /* Interrupts must be disabled while accessing the cd_recv FIFO */ @@ -470,7 +457,6 @@ static ssize_t can_read(FAR struct file *filep, FAR char *buffer, * in the user buffer. */ - nread = 0; do { /* Will the next message in the FIFO fit into the user buffer? */ @@ -479,15 +465,15 @@ static ssize_t can_read(FAR struct file *filep, FAR char *buffer, int nbytes = can_dlc2bytes(msg->cm_hdr.ch_dlc); int msglen = CAN_MSGLEN(nbytes); - if (nread + msglen > buflen) + if (ret + msglen > buflen) { break; } /* Copy the message to the user buffer */ - memcpy(&buffer[nread], msg, msglen); - nread += msglen; + memcpy(&buffer[ret], msg, msglen); + ret += msglen; /* Increment the head of the circular message buffer */ @@ -508,10 +494,6 @@ static ssize_t can_read(FAR struct file *filep, FAR char *buffer, nxsem_post(&fifo->rx_sem); } - /* Return the number of bytes that were read. */ - - ret = nread; - return_with_irqdisabled: leave_critical_section(flags); } @@ -637,7 +619,7 @@ static ssize_t can_write(FAR struct file *filep, FAR const char *buffer, * shorter than the minimum. */ - while (((ssize_t)buflen - nsent) >= CAN_MSGLEN(0)) + while (buflen - nsent >= CAN_MSGLEN(0)) { /* Check if adding this new message would over-run the drivers ability * to enqueue xmit data. @@ -747,7 +729,7 @@ return_with_irqdisabled: static inline ssize_t can_rtrread(FAR struct file *filep, FAR struct canioc_rtr_s *request) { - FAR struct can_dev_s *dev = filep->f_inode->i_private; + FAR struct can_dev_s *dev = filep->f_inode->i_private; FAR struct can_rtrwait_s *wait = NULL; irqstate_t flags; int i; @@ -765,7 +747,6 @@ static inline ssize_t can_rtrread(FAR struct file *filep, FAR struct can_rtrwait_s *tmp = &dev->cd_rtr[i]; ret = nxsem_get_value(&tmp->cr_sem, &sval); - if (ret < 0) { continue; @@ -777,7 +758,6 @@ static inline ssize_t can_rtrread(FAR struct file *filep, tmp->cr_msg = request->ci_msg; dev->cd_npendrtr++; - wait = tmp; break; } @@ -814,7 +794,7 @@ static inline ssize_t can_rtrread(FAR struct file *filep, request->ci_msg->cm_hdr.ch_rtr = 1; ret = can_write(filep, - (const char *) request->ci_msg, + (FAR const char *)request->ci_msg, CAN_MSGLEN(request->ci_msg->cm_hdr.ch_dlc)); request->ci_msg->cm_hdr.ch_rtr = 0; #else @@ -846,8 +826,7 @@ static int can_ioctl(FAR struct file *filep, int cmd, unsigned long arg) FAR struct inode *inode = filep->f_inode; FAR struct can_dev_s *dev = inode->i_private; FAR struct can_reader_s *reader = filep->f_priv; - - int ret = OK; + int ret = OK; caninfo("cmd: %d arg: %ld\n", cmd, arg); @@ -951,10 +930,10 @@ static int can_ioctl(FAR struct file *filep, int cmd, unsigned long arg) static int can_poll(FAR struct file *filep, FAR struct pollfd *fds, bool setup) { - FAR struct inode *inode = (FAR struct inode *)filep->f_inode; + FAR struct inode *inode = filep->f_inode; FAR struct can_dev_s *dev = inode->i_private; FAR struct can_reader_s *reader = NULL; - pollevent_t eventset; + pollevent_t eventset = 0; int ndx; int sval; irqstate_t flags; @@ -1028,8 +1007,6 @@ static int can_poll(FAR struct file *filep, FAR struct pollfd *fds, * should, but that would be a little awkward). */ - eventset = 0; - DEBUGASSERT(dev->cd_ntxwaiters < 255); dev->cd_ntxwaiters++; do @@ -1099,7 +1076,6 @@ static int can_poll(FAR struct file *filep, FAR struct pollfd *fds, errout: nxmutex_unlock(&dev->cd_polllock); - return_with_irqdisabled: leave_critical_section(flags); return ret; @@ -1113,7 +1089,7 @@ return_with_irqdisabled: * Name: can_register * * Description: - * Register serial console and serial ports. + * Register a CAN driver. * ****************************************************************************/ @@ -1179,15 +1155,11 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_t *data) { FAR struct can_rxfifo_s *fifo; - FAR uint8_t *dest; FAR struct list_node *node; - FAR struct list_node *tmp; int nexttail; - int errcode = -ENOMEM; + int ret = -ENOMEM; int i; - int j; int sval; - int ret; caninfo("ID: %" PRId32 " DLC: %d\n", (uint32_t)hdr->ch_id, hdr->ch_dlc); @@ -1212,21 +1184,18 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, /* Check if the entry is in use and whether the ID matches */ - ret = nxsem_get_value(&wait->cr_sem, &sval); - - if (ret < 0) + if (nxsem_get_value(&wait->cr_sem, &sval) < 0) { continue; } - else if (sval < 0 #ifdef CONFIG_CAN_ERRORS - && hdr->ch_error == false + && hdr->ch_error == false #endif #ifdef CONFIG_CAN_EXTID - && waitmsg->cm_hdr.ch_extid == hdr->ch_extid + && waitmsg->cm_hdr.ch_extid == hdr->ch_extid #endif - && waitmsg->cm_hdr.ch_id == hdr->ch_id) + && waitmsg->cm_hdr.ch_id == hdr->ch_id) { int nbytes; @@ -1235,11 +1204,7 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, memcpy(&waitmsg->cm_hdr, hdr, sizeof(struct can_hdr_s)); nbytes = can_dlc2bytes(hdr->ch_dlc); - for (j = 0, dest = waitmsg->cm_data; j < nbytes; j++) - { - *dest++ = *data++; - } - + memcpy(waitmsg->cm_data, data, nbytes); dev->cd_npendrtr--; /* Restart the waiting thread and mark the entry unused */ @@ -1249,7 +1214,7 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, } } - list_for_every_safe(&dev->cd_readers, node, tmp) + list_for_every(&dev->cd_readers, node) { FAR struct can_reader_s *reader = (FAR struct can_reader_s *)node; fifo = &reader->fifo; @@ -1292,7 +1257,6 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, poll_notify(dev->cd_fds, CONFIG_CAN_NPOLLWAITERS, POLLIN); - sval = 0; if (nxsem_get_value(&fifo->rx_sem, &sval) < 0) { DEBUGPANIC(); @@ -1314,7 +1278,7 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, nxsem_post(&fifo->rx_sem); } - errcode = OK; + ret = OK; } #ifdef CONFIG_CAN_ERRORS else @@ -1326,7 +1290,7 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, #endif } - return errcode; + return ret; } /**************************************************************************** @@ -1387,8 +1351,6 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, * * Input Parameters: * dev - The specific CAN device - * hdr - The 16-bit CAN header - * data - An array contain the CAN data. * * Returned Value: * OK on success; a negated errno on failure. @@ -1686,5 +1648,3 @@ uint8_t can_dlc2bytes(uint8_t dlc) return dlc; } - -#endif /* CONFIG_CAN */ diff --git a/include/nuttx/can.h b/include/nuttx/can.h index 982a193106..dd69e9d0b7 100644 --- a/include/nuttx/can.h +++ b/include/nuttx/can.h @@ -27,11 +27,7 @@ #include -#ifdef CONFIG_CAN_TXREADY -# include -#endif - -#include +#include #ifdef CONFIG_NET_CAN @@ -39,142 +35,6 @@ * Pre-processor Definitions ****************************************************************************/ -/* Ioctl Commands ***********************************************************/ - -/* Ioctl commands supported by the upper half CAN driver. - * - * CANIOC_RTR: - * Description: Send the remote transmission request and wait for the - * response. - * Argument: A reference to struct canioc_rtr_s - * - * Ioctl commands that may or may not be supported by the lower half CAN - * driver. - * - * CANIOC_ADD_STDFILTER: - * Description: Add an address filter for a standard 11 bit address. - * Argument: A reference to struct canioc_stdfilter_s - * Returned Value: A non-negative filter ID is returned on success. - * Otherwise -1 (ERROR) is returned with the errno - * variable set to indicate the nature of the error. - * Dependencies: None - * - * CANIOC_ADD_EXTFILTER: - * Description: Add an address filter for a extended 29 bit address. - * Argument: A reference to struct canioc_extfilter_s - * Returned Value: A non-negative filter ID is returned on success. - * Otherwise -1 (ERROR) is returned with the errno - * variable set to indicate the nature of the error. - * Dependencies: Requires CONFIG_CAN_EXTID=y - * - * CANIOC_DEL_STDFILTER: - * Description: Remove an address filter for a standard 11 bit address. - * Argument: The filter index previously returned by the - * CANIOC_ADD_STDFILTER command - * 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_DEL_EXTFILTER: - * Description: Remove an address filter for a standard 29 bit address. - * Argument: The filter index previously returned by the - * CANIOC_ADD_EXTFILTER command - * 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: Requires CONFIG_CAN_EXTID=y - * - * CANIOC_GET_BITTIMING: - * Description: Return the current bit timing settings - * Argument: A pointer to a write-able instance of struct - * canioc_bittiming_s in which current bit timing values - * will be returned. - * 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_BITTIMING: - * Description: Set new current bit timing values - * Argument: A pointer to a read-able instance of struct - * canioc_bittiming_s in which the new bit timing values - * are provided. - * 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_CONNMODES: - * Description: Get the current bus connection modes - * Argument: A pointer to a write-able instance of struct - * canioc_connmodes_s in which the new bus modes will be - * returned. - * 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_CONNMODES: - * Description: Set new bus connection modes values - * Argument: A pointer to a read-able instance of struct - * canioc_connmodes_s in which the new bus modes are - * provided. - * 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_BUSOFF_RECOVERY: - * Description: Initiates the BUS-OFF recovery sequence - * Argument: None - * 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 - */ - -#define CANIOC_RTR _CANIOC(1) -#define CANIOC_GET_BITTIMING _CANIOC(2) -#define CANIOC_SET_BITTIMING _CANIOC(3) -#define CANIOC_ADD_STDFILTER _CANIOC(4) -#define CANIOC_ADD_EXTFILTER _CANIOC(5) -#define CANIOC_DEL_STDFILTER _CANIOC(6) -#define CANIOC_DEL_EXTFILTER _CANIOC(7) -#define CANIOC_GET_CONNMODES _CANIOC(8) -#define CANIOC_SET_CONNMODES _CANIOC(9) -#define CANIOC_BUSOFF_RECOVERY _CANIOC(10) - -#define CAN_FIRST 0x0001 /* First common command */ -#define CAN_NCMDS 10 /* Ten common commands */ - -/* User defined ioctl commands are also supported. These will be forwarded - * by the upper-half CAN driver to the lower-half CAN driver via the - * co_ioctl() method fo the CAN lower-half interface. - * However, the lower-half driver must reserve a block of commands as follows - * in order prevent IOCTL command numbers from overlapping. - * - * This is generally done as follows. The first reservation for CAN driver A - * would look like: - * - * CAN_A_FIRST (CAN_FIRST + CAN_NCMDS) <- First command - * CAN_A_NCMDS 42 <- Number of commands - * - * IOCTL commands for CAN driver A would then be defined in a CAN A header - * file like: - * - * CANIOC_A_CMD1 _CANIOC(CAN_A_FIRST+0) - * CANIOC_A_CMD2 _CANIOC(CAN_A_FIRST+1) - * CANIOC_A_CMD3 _CANIOC(CAN_A_FIRST+2) - * ... - * CANIOC_A_CMD42 _CANIOC(CAN_A_FIRST+41) - * - * The next reservation would look like: - * - * CAN_B_FIRST (CAN_A_FIRST + CAN_A_NCMDS) <- Next command - * CAN_B_NCMDS 77 <- Number of commands - */ - /* CAN payload length and DLC definitions according to ISO 11898-1 */ #define CAN_MAX_DLC 8 @@ -289,15 +149,6 @@ * Public Types ****************************************************************************/ -typedef FAR void *CAN_HANDLE; - -struct can_response_s -{ - sq_entry_t flink; - - /* Message-specific data may follow */ -}; /* FIXME remove */ - typedef uint32_t canid_t; /* Controller Area Network Error Message Frame Mask structure diff --git a/include/nuttx/can/can.h b/include/nuttx/can/can.h index 6eb7847ad7..054876418f 100644 --- a/include/nuttx/can/can.h +++ b/include/nuttx/can/can.h @@ -286,7 +286,7 @@ * CANIOC_A_CMD2 _CANIOC(CAN_A_FIRST+1) * CANIOC_A_CMD3 _CANIOC(CAN_A_FIRST+2) * ... - * CANIOC_A_CMD42 _CANIOC(CAN_A_FIRST+41) + * CANIOC_A_CMD42 _CANIOC(CAN_A_FIRST+41) * * The next reservation would look like: * @@ -296,16 +296,16 @@ /* Convenience macros *******************************************************/ -#define dev_reset(dev) dev->cd_ops->co_reset(dev) -#define dev_setup(dev) dev->cd_ops->co_setup(dev) -#define dev_shutdown(dev) dev->cd_ops->co_shutdown(dev) -#define dev_txint(dev,enable) dev->cd_ops->co_txint(dev,enable) -#define dev_rxint(dev,enable) dev->cd_ops->co_rxint(dev,enable) -#define dev_ioctl(dev,cmd,arg) dev->cd_ops->co_ioctl(dev,cmd,arg) -#define dev_remoterequest(dev,id) dev->cd_ops->co_remoterequest(dev,id) -#define dev_send(dev,m) dev->cd_ops->co_send(dev,m) -#define dev_txready(dev) dev->cd_ops->co_txready(dev) -#define dev_txempty(dev) dev->cd_ops->co_txempty(dev) +#define dev_reset(dev) (dev)->cd_ops->co_reset(dev) +#define dev_setup(dev) (dev)->cd_ops->co_setup(dev) +#define dev_shutdown(dev) (dev)->cd_ops->co_shutdown(dev) +#define dev_txint(dev,enable) (dev)->cd_ops->co_txint(dev,enable) +#define dev_rxint(dev,enable) (dev)->cd_ops->co_rxint(dev,enable) +#define dev_ioctl(dev,cmd,arg) (dev)->cd_ops->co_ioctl(dev,cmd,arg) +#define dev_remoterequest(dev,id) (dev)->cd_ops->co_remoterequest(dev,id) +#define dev_send(dev,m) (dev)->cd_ops->co_send(dev,m) +#define dev_txready(dev) (dev)->cd_ops->co_txready(dev) +#define dev_txempty(dev) (dev)->cd_ops->co_txempty(dev) /* CAN message support ******************************************************/ @@ -408,7 +408,7 @@ # define CANH_ERROR4_SHORT2VCC 0x03 # define CANH_ERROR4_SHORT2GND 0x04 -# define CANL_ERROR4_MASK 0xf0 /* Bits 0-3: CANL */ +# define CANL_ERROR4_MASK 0xf0 /* Bits 4-7: CANL */ # define CANL_ERROR4_NOWIRE 0x10 # define CANL_ERROR4_SHORT2BAT 0x20 # define CANL_ERROR4_SHORT2VCC 0x30 @@ -573,7 +573,7 @@ struct can_txfifo_s struct can_rtrwait_s { - sem_t cr_sem; /* Wait for response/is the cd_rtr entry available */ + sem_t cr_sem; /* Wait for response/is the cd_rtr entry available */ FAR struct can_msg_s *cr_msg; /* This is where the RTR response goes */ }; @@ -871,8 +871,6 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, * * Input Parameters: * dev - The specific CAN device - * hdr - The 16-bit CAN header - * data - An array contain the CAN data. * * Returned Value: * OK on success; a negated errno on failure.