Hello, Recently I've been working on LIN bus support with SAME70 based device. In general the integration was done smoothly and I would like to contribute my work to the community. The LIN is pretty similar to CAN and in Linux is abstracted as a SocketCAN device. During my integration I chose a more traditional character driver "/dev/linX", but with CAN driver interface.
Currently I see that "old" CAN character driver and new SocketCAN live in parallel with a decent amount of duplicated definitions that are already out of sync. I see a room for improvement here and want to share my thoughts. I think that it should be possible to adapt CAN lower interface "struct can_ops_s" so that it can be used by both CAN char and CAN netdev devices upper drive logic. Taking into account that upper layer driver may operate with messages of a different structure (might have padding fields, etc) the minimum common logic should be defined as well. I will express what should be done in a list: 1. All CAN char drivers use "cd_priv" field from "struct can_dev_s" while lower driver interface require pointer to "struct can_dev_s" to be passed. Here we need to define an intermediate type "struct can_updev_s" that will have first two members of "cd_ops" and "cd_priv" so "struct can_dev_s" and "struct can_netdev_s" can be inherited from "struct can_updev_s". 2. "co_send" interface in "struct can_ops_s" should be changed from "CODE int (*co_send)(FAR struct can_dev_s *dev, FAR struct can_msg_s *msg);" to "CODE int (*co_send)(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_t *data);" -- so we can get away from "struct can_msg_s" that is specific to CAN char driver. 3. The CAN upper logic is already decoupled from "struct can_msg_s" because of "int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_t *data);" already contain separation, so we need just change "struct can_dev_s" to "struct can_updev_s" here and char dev or netdev will register a proper input callback that will handle RX data or a proper input callback can be added into "struct can_updev_s" 4. "struct can_hdr_s" should be reworked so it can have maximum in common with SocketCAN. SocketCAN expects that lower layer can operate either with standard or extended CAN IDs, so SocketCAN will be available only if CONFIG_CAN_EXTID=y a) In case of CONFIG_CAN_EXTID=y the ch_id in "struct can_hdr_s" is uint32_t, so 3 upper bits are unused. SocketCAN stores supplementary information in those 3 bits like "std vs ext CAN-ID", "RTR vs non-RTR request" and "Error bit". I think we can adopt the same approach and eliminate "ch_rtr", "ch_extid" and "ch_error" bit fields from "struct can_hdr_s" with zero memory or performance hit. b) With "ch_dlc" and "ch_edl" / "ch_brs" / "ch_esi" it will be not so easy because SocketCAN defines two uint8_t fields for this "len" and "flags" while char driver packs everything using bit-fields. Here I need an advice. We can keep bit-fields in the header and then split into two uint8_t in the "can_receive()" variant of SocketCAN or we can extend "struct can_hdr_s" to have 2 uint8_t. The second will not give any memory overhead compared to what we have now because existing bit fields occupy exactly 2 uint8_t variables (but using bit-fields will allow us to save 1 byte of course). I would appreciate your feedback if above makes sense. Thank you in advance, Petro
