ppisa commented on code in PR #11868: URL: https://github.com/apache/nuttx/pull/11868#discussion_r1521671256
########## include/nuttx/can/sja1000.h: ########## @@ -0,0 +1,163 @@ +/**************************************************************************** + * include/nuttx/can/sja1000.h + * + * SJA1000 CAN driver based on esp32c3_twai.h + * + * License header retained from original source. + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef __DRIVERS_INCLUDE_CAN_SJA1000_H +#define __DRIVERS_INCLUDE_CAN_SJA1000_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Public Types + ****************************************************************************/ + +/* CAN hardware-dependent bit-timing constant + * + * Used for calculating and checking bit-timing parameters + */ + +struct sja1000_can_bittiming_const +{ + char name[16]; /* Name of the CAN controller hardware */ + uint32_t tseg1_min; /* Time segment 1 = prop_seg + phase_seg1 */ + uint32_t tseg1_max; + uint32_t tseg2_min; /* Time segment 2 = phase_seg2 */ + uint32_t tseg2_max; + uint32_t sjw_min; /* Synchronization jump width */ + uint32_t sjw_max; /* Synchronisation jump width */ + uint32_t brp_min; /* Bit-rate prescaler */ + uint32_t brp_max; + uint32_t brp_inc; +}; + +/* Type of the SJA1000 interrupt handling callback */ + +struct sja1000_config_s; /* Forward reference */ +typedef CODE void (*sja1000_handler_t)(FAR struct sja1000_config_s *config, + FAR void *arg); + +/* A reference to a structure of this type must be passed to the SJA1000 + * driver when the driver is instantiated. This structure provides + * information about the configuration of the SJA1000 and provides some + * board-specific hooks. + * + * Memory for this structure is provided by the caller. It is not copied by + * the driver and is presumed to persist while the driver is active. The + * memory must be writeable because, under certain circumstances, the driver + * may modify the frequency. + */ + +struct sja1000_config_s +{ + /* Device configuration */ + + const struct sja1000_can_bittiming_const *bittiming_const; + uint8_t port; /* SJA1000 device port number */ + uint8_t periph; /* Peripheral ID (optional) */ + uint8_t irq; /* IRQ associated with this SJA1000 */ + uint8_t cpu; /* CPU ID */ + int8_t cpuint; /* CPU interrupt assigned to this SJA1000 */ + uint32_t bitrate; /* Configured bit rate */ + uint32_t samplep; /* Configured sample point */ + uint32_t sjw; /* Synchronization jump width */ + uint32_t base; /* SJA1000 register base address */ + uint32_t clk_freq; /* Peripheral clock frequency */ + bool loopback; /* Enable loopback mode */ + bool triple_sample; /* Enable triple-sampling of CAN BUS */ + + /* Device characterization */ + + /* IRQ/GPIO access callbacks. These operations all hidden behind + * callbacks to isolate the driver from differences in GPIO + * interrupt handling by varying boards and MCUs. + * + * attach - Attach interrupt handler to the interrupt + * detach - Detach interrupt handler from the interrupt + */ + + CODE int (*attach)(FAR struct sja1000_config_s *config, + sja1000_handler_t handler, FAR void *arg); + CODE int (*detach)(FAR struct sja1000_config_s *config); +}; + +/* This structure provides the current state of a CAN peripheral */ + +struct sja1000_dev_s +{ + FAR struct sja1000_config_s *config; /* The constant configuration */ + uint8_t filters; /* STD/EXT filter bit allocator. */ + uint8_t nalloc; /* Number of allocated filters */ + +#ifdef CONFIG_ARCH_HAVE_MULTICPU + spinlock_t lock; /* Device specific lock */ +#endif /* CONFIG_ARCH_HAVE_MULTICPU */ + + /* Register read/write callbacks. These operations all hidden behind + * callbacks to isolate the driver from differences in register read/write + * handling by varying boards and MCUs. + * + * getreg - Read from a register address + * putreg - Write to a register address + */ + + CODE uint32_t (*getreg)(uint32_t addr); + CODE void (*putreg)(uint32_t addr, uint32_t value); + CODE void (*modifyreg32)(uint32_t addr, uint32_t clearbits, + uint32_t setbits); +}; Review Comment: In general, I agree with getreg and putreg configurable implementation option but I suggest not to call them directly as ``` priv->getreg(priv->getreg(SJA1000_INT_ENA_REG(base_addr)); ``` I would suggest inline wraper ``` static inline uint32_t sja1000_getreg(struct sja1000_dev_s *sja_priv, uint32_t addr) { return sja_priv->getreg(sja_priv, addr); } ``` and option to use wrapper alternative on the small targets where indirection can make code optimization significantly worse. If ```static inline``` is not allowed in architecture neutral NuttX parts yet, then use preprocessor macros. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
