On 10/2/20 5:41 PM, Vincent Mailhol wrote:

This driver supports the ES581.4, ES582.1 and ES584.1 interfaces from
ETAS GmbH (https://www.etas.com/en/products/es58x.php).

Co-developed-by: Arunachalam Santhanam <redacted>
Signed-off-by: Arunachalam Santhanam <redacted>
Signed-off-by: Vincent Mailhol <redacted>
---

Changes in v3:
  - Remove all the calls to likely() and unlikely().

Changes in v2:
  - Fixed -W1 warnings (v1 was tested with GCC -WExtra but not with -W1).
---
 drivers/net/can/usb/Kconfig                 |    9 +
 drivers/net/can/usb/Makefile                |    1 +
 drivers/net/can/usb/etas_es58x/Makefile     |    3 +
 drivers/net/can/usb/etas_es58x/es581_4.c    |  559 ++++
 drivers/net/can/usb/etas_es58x/es581_4.h    |  237 ++
 drivers/net/can/usb/etas_es58x/es58x_core.c | 2725 +++++++++++++++++++
 drivers/net/can/usb/etas_es58x/es58x_core.h |  700 +++++
 drivers/net/can/usb/etas_es58x/es58x_fd.c   |  648 +++++
 drivers/net/can/usb/etas_es58x/es58x_fd.h   |  243 ++
 9 files changed, 5125 insertions(+)
 create mode 100644 drivers/net/can/usb/etas_es58x/Makefile
 create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.c
 create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.h
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.c
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.h
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.c
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.h

[...]

Just one header file for now :)

DIV_ROUNDUP

+/* Size of a CAN Extended Frame (rounded up and ignoring bitsuffing). */
+#define ES58X_EFF_BYTES(data_len) (round_up(67, 8) / 8 + (data_len))

dame here

+/* Maximum size of a CAN frame (rounded up and ignoring bitsuffing). */
+#define ES58X_CAN_FRAME_BYTES_MAX ES58X_EFF_BYTES(CAN_MAX_DLEN)

please add a new file between the define and the doc of the next one

+/* Maximum size of a CAN-FD frame (rough estimation because
+ * ES58X_SFF_BYTES() and ES58X_EFF_BYTES() macros are using the
+ * constant values for CAN not CAN-FD).
+ */
+#define ES58X_CANFD_FRAME_BYTES_MAX ES58X_EFF_BYTES(CANFD_MAX_DLEN)
+
+/* Driver constants */
+#define ES58X_RX_URBS_MAX         5	// Empirical value
+#define ES58X_TX_URBS_MAX         8	// Empirical value

please use one space only

+
+#define ES58X_MAX(param)				\
+	(ES581_4_##param > ES58X_FD_##param ?		\
+		ES581_4_##param : ES58X_FD_##param)
+#define ES58X_TX_BULK_MAX ES58X_MAX(TX_BULK_MAX)
+#define ES58X_RX_BULK_MAX ES58X_MAX(RX_BULK_MAX)
+#define ES58X_RX_LOOPBACK_BULK_MAX ES58X_MAX(RX_LOOPBACK_BULK_MAX)
+#define ES58X_NUM_CAN_CH_MAX ES58X_MAX(NUM_CAN_CH)
+
+/* Use this when channel index is irrelevant (e.g. device
+ * timestamp).
+ */
+#define ES58X_CHANNEL_IDX_NA 0xFF
+#define ES58X_EMPTY_MSG NULL
+
+/* Threshold on consecutive CAN_STATE_ERROR_PASSIVE. If we receive
+ * ES58X_CONSECUTIVE_ERR_PASSIVE_MAX times the event
+ * ES58X_ERR_CRTL_PASSIVE in a row without any successful Rx or Tx,
+ * we force the device to switch to CAN_STATE_BUS_OFF state.
+ */
+#define ES58X_CONSECUTIVE_ERR_PASSIVE_MAX 254

Does the device recover from bus off automatically or why is this needed?

+
+enum es58x_self_reception_mode {
+	ES58X_SELF_RECEPTION_OFF = 0,
+	ES58X_SELF_RECEPTION_ON = 1
+};

nitpick: can you name all enums (here and below) according to the type?

e.g. ES58x_SELF_RECEPTION_MODE_OFF

+
+enum es58x_physical_media {
+	ES58X_MEDIA_HIGH_SPEED = 1,
+	ES58X_MEDIA_FAULT_TOLERANT = 2

You mean with FAULT_TOLERANT you mean ISO 11898-3? According to [1] they should
be named low speed.

[1]
https://can-cia.org/news/press-releases/view/harmonized-transceiver-naming/2020/7/16/

+};
+
+enum es58x_samples_per_bit {
+	ES58X_ONE_SAMPLE_PER_BIT = 1,
+
+	/* Some CAN controllers do not support three samples per
+	 * bit. In this case the default value of one sample per bit
+	 * is used, even if the configuration is set to
+	 * ES58X_THREE_SAMPLES_PER_BIT.
+	 */

Can you autodetect the controller and avoid announcing tripple sample mode to
the driver framework?

+	ES58X_THREE_SAMPLES_PER_BIT = 2
+};
+
+enum es58x_sync_edge {
+	/* ISO CAN specification defines the use of a single edge
+	 * synchronization. The synchronization should be done on
+	 * recessive to dominant level change.
+	 */
+	ES58X_SINGLE_SYNC_EDGE = 1,
+
+	/* In addition to the ISO CAN specification, a double
+	 * synchronization is also supported: recessive to dominant
+	 * level change and dominant to recessive level change.
+	 */
+	ES58X_DUAL_SYNC_EDGE = 2

We don't have a setting in the CAN framework for this....

+};
+
+/**
+ * enum es58x_flag_type - CAN flags for RX/TX messages.
+ * @ES58X_FLAG_EFF: Extended Frame Format (EFF).
+ * @ES58X_FLAG_RTR: Remote Transmission Request (RTR).
+ * @ES58X_FLAG_SELFRECEPTION: The message is a Self reception frame
+ *	(not used yet in this implementation).
+ * @ES58X_FLAG_FD_BRS: Bit rate switch (BRS): second bitrate for
+ *	payload data.
+ * @ES58X_FLAG_FD_MSG_TRUNCATED: FD message was truncated and padded
+ *	(not used).
+ * @ES58X_FLAG_FD_ESI: Error State Indicator (ESI): tell if the
+ *	transmitting node is in error passive mode.
+ * @ES58X_FLAG_FD_DATA: CAN FD frame.
+ */
+enum es58x_flag_type {
+	ES58X_FLAG_EFF = BIT(0),
+	ES58X_FLAG_RTR = BIT(1),
+	ES58X_FLAG_SELFRECEPTION = BIT(2),
+	ES58X_FLAG_FD_BRS = BIT(3),
+	ES58X_FLAG_FD_MSG_TRUNCATED = BIT(4),
+	ES58X_FLAG_FD_ESI = BIT(5),
+	ES58X_FLAG_FD_DATA = BIT(6)
+};
+
+/**
+ * enum es58x_error - CAN error detection.
+ * @ES58X_ERR_OK: No errors.
+ * @ES58X_ERR_PROT_STUFF: Bit stuffing error: more than 5 consecutive
+ *	equal bits.
+ * @ES58X_ERR_PROT_FORM: Frame format error.
+ * @ES58X_ERR_ACK: Received no ACK on transmission.
+ * @ES58X_ERR_PROT_BIT: Single bit error.
+ * @ES58X_ERR_PROT_CRC: Incorrect 15, 17 or 21 bits CRC.
+ * @ES58X_ERR_PROT_BIT1: Unable to send recessive bit: tried to send
+ *	recessive bit 1 but monitored dominant bit 0.
+ * @ES58X_ERR_PROT_BIT0: Unable to send dominant bit: tried to send
+ *	dominant bit 0 but monitored recessive bit 1.
+ * @ES58X_ERR_PROT_OVERLOAD: Bus overload.
+ * @ES58X_ERR_PROT_UNSPEC: Unspecified.
+ *
+ * Please refer to ISO 11898-1:2015, section 10.11 "Error detection"
+ * and section 10.13 "Overload signaling" for additional details.
+ */
+enum es58x_error {
+	ES58X_ERR_OK = 0,
+	ES58X_ERR_PROT_STUFF = BIT(0),
+	ES58X_ERR_PROT_FORM = BIT(1),
+	ES58X_ERR_ACK = BIT(2),
+	ES58X_ERR_PROT_BIT = BIT(3),
+	ES58X_ERR_PROT_CRC = BIT(4),
+	ES58X_ERR_PROT_BIT1 = BIT(5),
+	ES58X_ERR_PROT_BIT0 = BIT(6),
+	ES58X_ERR_PROT_OVERLOAD = BIT(7),
+	ES58X_ERR_PROT_UNSPEC = BIT(31)
+};
+
+/**
+ * enum es58x_event - CAN error codes returned by the device.
+ * @ES58X_EVENT_OK: No errors.
+ * @ES58X_ERR_CRTL_ACTIVE: Active state: both TR and RX error count is
+ *	less than 128.
+ * @ES58X_ERR_CRTL_PASSIVE: Passive state: either TX or RX error count
+ *	is greater than 127.
+ * @ES58X_ERR_CRTL_WARNING: Warning state: either TX or RX error count
+ *	is greater than 96.
+ * @ES58X_ERR_BUSOFF: Bus off.
+ * @ES58X_ERR_SINGLE_WIRE: Lost connection on either CAN high or CAN low.
+ *
+ * Please refer to ISO 11898-1:2015, section 12.1.4 "Rules of fault
+ * confinement" for additional details.
+ */
+enum es58x_event {
+	ES58X_EVENT_OK = 0,
+	ES58X_ERR_CRTL_ACTIVE = BIT(0),
+	ES58X_ERR_CRTL_PASSIVE = BIT(1),
+	ES58X_ERR_CRTL_WARNING = BIT(2),
+	ES58X_ERR_BUSOFF = BIT(3),
+	ES58X_ERR_SINGLE_WIRE = BIT(4)
+};
+
+/**
+ * enum es58x_dev_ret_code_u8 - Device error codes, 8 bit format.
+ *
+ * Specific to ES581.4.
+ */
+enum es58x_dev_ret_code_u8 {
+	ES58X_RET_U8_OK = 0x00,
+	ES58X_RET_U8_ERR_UNSPECIFIED_FAILURE = 0x80,
+	ES58X_RET_U8_ERR_NO_MEM = 0x81,
+	ES58X_RET_U8_ERR_BAD_CRC = 0x99
+};
+
+/**
+ * enum es58x_dev_ret_code_u32 - Device error codes, 32 bit format.
+ */
+enum es58x_cmd_ret_code_u32 {
+	ES58X_RET_U32_OK = 0x00000000UL,
+	ES58X_RET_U32_ERR_UNSPECIFIED_FAILURE = 0x80000000UL,
+	ES58X_RET_U32_ERR_NO_MEM = 0x80004001UL,
+	ES58X_RET_U32_WARN_PARAM_ADJUSTED = 0x40004000UL,
+	ES58X_RET_U32_WARN_TX_MAYBE_REORDER = 0x40004001UL,
+	ES58X_RET_U32_ERR_TIMEOUT = 0x80000008UL,
+	ES58X_RET_U32_ERR_FIFO_FULL = 0x80003002UL,
+	ES58X_RET_U32_ERR_BAD_CONFIG = 0x80004000UL,
+	ES58X_RET_U32_ERR_NO_RESOURCE = 0x80004002UL
+};
+
+enum es58x_cmd_ret_type {
+	ES58X_RET_TYPE_SET_BITTIMING,
+	ES58X_RET_TYPE_ENABLE_CHANNEL,
+	ES58X_RET_TYPE_DISABLE_CHANNEL,
+	ES58X_RET_TYPE_TX_MSG,
+	ES58X_RET_TYPE_RESET_RX,
+	ES58X_RET_TYPE_RESET_TX,
+	ES58X_RET_TYPE_DEVICE_ERR_FRAME,
+	ES58X_CMD_RET_TYPE_NUM_ENTRIES
+};
+
+/**
+ * struct es58x_abstracted_can_frame - Common structure to hold can
+ *	frame information.

why do you have an itermediate can frame format? We have the struct can_frame
and the skb for this.

+ * @timestamp: Hardware time stamp (only relevant in rx branches).
+ * @data: CAN payload.
+ * @can_id: CAN ID.
+ * @is_can_fd: false: non-FD CAN, true: CAN-FD.
+ * @flags: Please refer to enum es58x_flag_type.
+ * @dlc: Data Length Code (raw value). When using standard (non-FD)
+ *	CAN, ES58X devices allow to send DLC bigger than 8
+ *	(i.e. values 9 to 15 reserved for CAN-FD) as specified in ISO
+ *	11898-1:2015 section 8.4.2.4 "DLC field". In such case, the
+ *	@dlc field will contain whatever value was obtained when
+ *	sending or receiving but the @len field will contain the
+ *	sanitized length of the @data field (i.e. not more than
+ *	CAN_MAX_DLEN for standard CAN). To be able to send/receive
+ *	such "out of range" DLC values, you would need to modify the
+ *	first occurrence of the if conditions "cfd->len >
+ *	CAN_MAX_DLEN" into "cfd->len > CANFD_MAX_DLC" in functions
+ *	net/can/af_can.c:can_send() and net/can/af_can.c:can_rcv().
+ * @len: Length of @data field (sanitized in es58x_core.c).
+ *
+ * ES581.4 and the ES58X FD family uses different tx_can_msg
+ * structures (same fields but in different order). This abstracted
+ * structure allows to calculate the parameters once for all. The
+ * specific functions of each device model can then use the
+ * pre-computed information from this abstracted can frame.
+ */
+struct es58x_abstracted_can_frame {
+	u64 timestamp;
+	const u8 *data;
+	canid_t can_id;
+	bool is_can_fd;
+	u8 flags;
+	u8 dlc;
+	u8 len;
+};
+
+union es58x_urb_cmd {
+	u8 raw_cmd[0];

I have to polish my C, what's an empty array in the beginning of a struct?

+	struct es581_4_urb_cmd es581_4_urb_cmd;
+	struct es58x_fd_urb_cmd es58x_fd_urb_cmd;
+	struct {		// Common header parts of all variants

no // comments please

+		__le16 sof;
+		u8 cmd_type;
+		u8 cmd_id;
+	} __packed;
+};
+
+/**
+ * struct es58x_priv - All information specific to a can channel.
+ * @can: struct can_priv must be the first member (Socket CAN relies
+ *	on the fact that function netdev_priv() returns a pointer to
+ *	a struct can_priv).
+ * @es58x_dev: pointer to the corresponding ES58X device.
+ * @echo_skb_spinlock: Spinlock to protect the access to the echo skb
+ *	FIFO.
+ * @current_packet_idx: Keeps track of the packet indexes.
+ * @echo_skb_tail_idx: beginning of the echo skb FIFO, i.e. index of
+ *	the first element.
+ * @echo_skb_head_idx: end of the echo skb FIFO plus one, i.e. first
+ *	free index.
+ * @num_echo_skb: actual number of elements in the FIFO. Thus, the end
+ *	of the FIFO is echo_skb_head = (echo_skb_tail_idx +
+ *	num_echo_skb) % can.echo_skb_max.
+ * @tx_urb: Used as a buffer to concatenate the TX messages and to do
+ *	a bulk send. Please refer to es58x_start_xmit() for more
+ *	details.
+ * @tx_can_msg_is_fd: false: all messages in @tx_urb are non-FD CAN,
+ *	true: all messages in @tx_urb are CAN-FD. Rationale: ES58X FD
+ *	devices do not allow to mix standard and FD CAN in one single
+ *	bulk transmission.
+ * @tx_can_msg_cnt: Number of messages in @tx_urb.
+ * @err_passive_before_rtx_success: The ES58X device might enter in a
+ *	state in which it keeps alternating between error passive
+ *	and active state. This counter keeps track of the number of
+ *	error passive and if it gets bigger than
+ *	ES58X_CONSECUTIVE_ERR_PASSIVE_MAX, es58x_rx_err_msg() will
+ *	force the status to bus-off.

Is this a bug or a feature?

+ * @channel_idx: Channel index, starts at zero.
+ */
+struct es58x_priv {
+	struct can_priv can;
+	struct es58x_device *es58x_dev;
+
+	spinlock_t echo_skb_spinlock;	// Comments: c.f. supra

please no // comments

+	u32 current_packet_idx;
+	u16 echo_skb_tail_idx;
+	u16 echo_skb_head_idx;
+	u16 num_echo_skb;
+
+	struct urb *tx_urb;
+	bool tx_can_msg_is_fd;
+	u8 tx_can_msg_cnt;
+
+	u8 err_passive_before_rtx_success;
+
+	u8 channel_idx;
+};
+
+/**
+ * struct es58x_parameters - Constant parameters of a given hardware
+ *	variant.
+ * @can_bittiming_const: Nominal bittimming parameters (used for
+ *	non-FD CAN and arbitration field of CAN-FD).
+ * @data_bittiming_const: Data bittiming parameters (used for CAN-FD
+ *	payload)
+ * @bitrate_max: Maximum bitrate supported by the device.
+ * @clock: CAN clock parameters.
+ * @ctrlmode_supported: List of supported modes. Please refer to
+ *	can/netlink.h file for additional details.
+ * @tx_start_of_frame: Magic number at the beginning of each TX URB
+ *	command.
+ * @rx_start_of_frame: Magic number at the beginning of each RX URB
+ *	command.
+ * @tx_urb_cmd_max_len: Maximum length of a TX URB command.
+ * @rx_urb_cmd_max_len: Maximum length of a RX URB command.
+ * @echo_skb_max: Maximum number of echo SKB. This value must not
+ *	exceed the maximum size of the device internal TX FIFO
+ *	length. This parameter is used to control the network queue
+ *	wake/stop logic.
+ * @dql_limit_min: Dynamic Queue Limits (DQL) absolute minimum limit
+ *	of bytes allowed to be queued on this network device transmit
+ *	queue. Used by the Byte Queue Limits (BQL) to determine how
+ *	frequently the xmit_more flag will be set to true in
+ *	es58x_start_xmit(). Set this value higher to optimize for
+ *	throughput but be aware that it might have a negative impact
+ *	on the latency! This value can also be set dynamically. Please
+ *	refer to Documentation/ABI/testing/sysfs-class-net-queues for
+ *	more details.
+ * @tx_bulk_max: Maximum number of Tx messages that can be sent in one
+ *	single URB packet.
+ * @urb_cmd_header_len: Length of the URB command header.
+ * @rx_urb_max: Number of RX URB to be allocated during device probe.
+ * @tx_urb_max: Number of TX URB to be allocated during device probe.
+ * @channel_idx_offset: Some of the ES58x starts channel numbering
+ *	from 0 (ES58X FD), others from 1 (ES581.4).
+ */
+struct es58x_parameters {
+	const struct can_bittiming_const *bittiming_const;
+	const struct can_bittiming_const *data_bittiming_const;
+	u32 bitrate_max;
+	struct can_clock clock;
+	u32 ctrlmode_supported;
+	u16 tx_start_of_frame;
+	u16 rx_start_of_frame;
+	u16 tx_urb_cmd_max_len;
+	u16 rx_urb_cmd_max_len;
+	u16 echo_skb_max;
+	u16 dql_limit_min;
+	u8 tx_bulk_max;
+	u8 urb_cmd_header_len;
+	u8 rx_urb_max;
+	u8 tx_urb_max;
+	u8 channel_idx_offset;
+};
+
+/**
+ * struct es58x_operators - Function pointers used to encode/decode
+ *	the TX/RX messages.
+ * @get_msg_len: Get field msg_len of the urb_cmd. The offset of
+ *	msg_len inside urb_cmd depends of the device model.
+ * @handle_urb_cmd: Handle URB command received from the device and
+ *	manage the return code from device.
+ * @fill_urb_header: Fill the header of urb_cmd.
+ * @tx_can_msg: Encode a TX CAN message and add it to the bulk buffer
+ *	cmd_buf of es58x_dev.
+ * @set_bittiming: Encode the bittiming information and send it.
+ * @enable_channel: Start the CAN channel with index channel_idx.
+ * @disable_channel: Stop the CAN channel with index channel_idx.
+ * @reset_rx: Reset the RX queue of the ES58X device.
+ * @reset_tx: Reset the TX queue of the ES58X device.
+ * @get_timestamp: Request a timestamp from the ES58X device.
+ */
+struct es58x_operators {
+	u16 (*get_msg_len)(const union es58x_urb_cmd *urb_cmd);
+	int (*handle_urb_cmd)(struct es58x_device *es58x_dev,
+			      const union es58x_urb_cmd *urb_cmd);
+	void (*fill_urb_header)(union es58x_urb_cmd *urb_cmd, u8 cmd_type,
+				u8 cmd_id, u8 channel_idx, u16 cmd_len);
+	int (*tx_can_msg)(struct es58x_device *es58x_dev, int channel_idx,
+			  u32 packet_idx,
+			   struct es58x_abstracted_can_frame *es58x_frame,
+			   union es58x_urb_cmd *urb_cmd, u32 *urb_cmd_len);
+	int (*set_bittiming)(struct es58x_device *es58x_dev, int channel_idx);
+	int (*enable_channel)(struct es58x_device *es58x_dev,
+			      int channel_idx);
+	int (*disable_channel)(struct es58x_device *es58x_dev,
+			       int channel_idx);
+	int (*reset_rx)(struct es58x_device *es58x_dev, int channel_idx);
+	int (*reset_tx)(struct es58x_device *es58x_dev, int channel_idx);
+	int (*get_timestamp)(struct es58x_device *es58x_dev);
+};
+
+/**
+ * struct es58x_device - All information specific to an ES58X device.
+ * @dev: Device information.
+ * @udev: USB device information.
+ * @netdev: Array of our CAN channels.
+ * @param: The constant parameters.
+ * @ops: Operators.
+ * @rx_pipe: USB reception pipe.
+ * @tx_pipe: USB transmission pipe.
+ * @rx_urbs: Anchor for received URBs.
+ * @tx_urbs_busy: Anchor for TX URBs which were send to the device.
+ * @tx_urbs_idle: Anchor for TX USB which are idle. This driver
+ *	allocates the memory for the URBs during the probe. When a TX
+ *	URB is needed, it can be taken from this anchor. The network
+ *	queue wake/stop logic should prevent this URB from getting
+ *	empty. Please refer to es58x_get_tx_urb() for more details.
+ * @tx_urbs_idle_cnt: number of urbs in @tx_urbs_idle.
+ * @ktime_req_ns: kernel timestamp when es58x_set_realtime_diff_ns()
+ *	was called.
+ * @realtime_diff_ns: difference in nanoseconds between the clocks of
+ *	the ES58X device and the kernel.
+ * @timestamps: a temporary buffer to store the time stamps before
+ *	feeding them to es58x_can_get_echo_skb(). Can only be used
+ *	in rx branches.
+ * @can_frames: a temporary buffer to store the can frames before
+ *	feeding them to es58x_rx_can_msg(). Can only be used in rx
+ *	branches.
+ * @rx_max_packet_size: Maximum length of bulk-in URB.
+ * @num_can_ch: Number of CAN channel (i.e. number of elements of @netdev).
+ * @rx_cmd_buf_len: Length of @rx_cmd_buf.
+ * @rx_cmd_buf: The device might split the URB commands in an
+ *	arbitrary amount of pieces. This buffer is used to concatenate
+ *	all those pieces. Can only be used in rx branches. This field
+ *	has to be the last one of the structure because it is has a
+ *	flexible size (c.f. ES58X_SIZEOF_ES58X_DEVICE() macro).
+ */
+struct es58x_device {
+	struct device *dev;
+	struct usb_device *udev;
+	struct net_device *netdev[ES58X_NUM_CAN_CH_MAX];
+
+	const struct es58x_parameters *param;
+	const struct es58x_operators *ops;
+
+	int rx_pipe;
+	int tx_pipe;
+
+	struct usb_anchor rx_urbs;
+	struct usb_anchor tx_urbs_busy;
+	struct usb_anchor tx_urbs_idle;
+	atomic_t tx_urbs_idle_cnt;
+
+	u64 ktime_req_ns;
+	s64 realtime_diff_ns;
+
+	union {
+		u64 timestamps[ES58X_RX_LOOPBACK_BULK_MAX];
+		struct es58x_abstracted_can_frame can_frames[ES58X_RX_BULK_MAX];
+	};
+
+	u16 rx_max_packet_size;
+	u8 num_can_ch;
+
+	u16 rx_cmd_buf_len;
+	union es58x_urb_cmd rx_cmd_buf;
+};
+
+/**
+ * ES58X_SIZEOF_ES58X_DEVICE() - Calculate the maximum length of
+ *	struct es58x_device.
+ * @es58x_dev_param: The constant parameters of the device.
+ *
+ * The length of struct es58x_device depends on the length of its last
+ * field: rx_cmd_buf. This macro allows to optimize size for memory
+ * allocation.
+ *
+ * Return: length of struct es58x_device.
+ */
+#define ES58X_SIZEOF_ES58X_DEVICE(es58x_dev_param)			\
+	(offsetof(struct es58x_device, rx_cmd_buf) +			\
+		(es58x_dev_param)->rx_urb_cmd_max_len)

can this be made a static inline?

+
+/* Megabit per second (multiply x per one million) */
+#define ES58X_MBPS(x) (1000000UL * (x))
+/* Megahertz (multiply x per one million) */
+#define ES58X_MHZ(x)  (1000000UL * (x))
+
+/**
+ * es58x_check_msg_len() - Check the size of a received message.
+ * @dev: Device, used to print error messages.
+ * @msg: Received message, must not be a pointer.
+ * @actual_len: Length of the message as advertised in the command header.
+ *
+ * Must be a macro in order to retrieve the actual size using
+ * sizeof(). Can be use with any of the messages which have a fixed
+ * length. Check for an exact match of the size.

You can provide an outer macro that does the sizeof() and then calls the a
normal (static inline) function to do the actual work. Applied to the next 3 macros.

+ *
+ * Return: zero on success, -EMSGSIZE if @actual_len differs from the
+ * expected length.
+ */
+#define es58x_check_msg_len(dev, msg, actual_len)			\
+({									\
+	size_t __expected_len = sizeof(msg);				\
+	size_t __actual_len = (actual_len);				\
+	int __res = 0;							\
+	if (__expected_len != __actual_len) {				\
+		dev_err(dev,						\
+			"%s: Length of %s is %zu but received command is %zu.\n",		\
+			__func__, __stringify(msg),			\
+			__expected_len, __actual_len);			\
+		__res = -EMSGSIZE;					\
+	}								\
+	__res;								\
+})
+
+/**
+ * es58x_check_msg_max_len() - Check the maximum size of a received message.
+ * @dev: Device, used to print error messages.
+ * @msg: Received message, must not be a pointer.
+ * @actual_len: Length of the message as advertised in the command header.
+ *
+ * Must be a macro in order to retrieve the actual size using
+ * sizeof(). To be used with the messages of variable sizes. Only
+ * check that the message is not bigger than the maximum expected
+ * size.
+ *
+ * Return: zero on success, -EOVERFLOW if @actual_len is greater than
+ * the expected length.
+ */
+#define es58x_check_msg_max_len(dev, msg, actual_len)			\
+({									\
+	size_t __actual_len = (actual_len);				\
+	size_t __expected_len = sizeof(msg);				\
+	int __res = 0;							\
+	if (__actual_len > __expected_len) {				\
+		dev_err(dev,						\
+			"%s: Maximum length for %s is %zu but received command is %zu.\n",	\
+			__func__, __stringify(msg),			\
+			__expected_len, __actual_len);			\
+		__res = -EOVERFLOW;					\
+	}								\
+	__res;								\
+})
+
+/**
+ * es58x_msg_num_element() - Check size and give the number of
+ *	elements in a message of array type.
+ * @dev: Device, used to print error messages.
+ * @msg: Received message, must be an array.
+ * @actual_len: Length of the message as advertised in the command
+ *	header.
+ *
+ * Must be a macro in order to retrieve the actual size using
+ * sizeof(). To be used on message of array type. Array's element has
+ * to be of fixed size (else use es58x_check_msg_max_len()). Check
+ * that the total length is an exact multiple of the length of a
+ * single element.
+ *
+ * Return: number of elements in the array on success, -EOVERFLOW if
+ * @actual_len is greater than the expected length, -EMSGSIZE if
+ * @actual_len is not a multiple of a single element.
+ */
+#define es58x_msg_num_element(dev, msg, actual_len)			\
+({									\
+	const struct device *__dev = (dev);				\
+	size_t __actual_len = (actual_len);				\
+	size_t __elem_len = sizeof((msg)[0]) + __must_be_array(msg);	\
+	size_t __actual_num_elem = __actual_len / __elem_len;		\
+	size_t __expected_num_elem = sizeof(msg) / __elem_len;		\
+	int __res = __actual_num_elem;					\
+	if (__actual_num_elem == 0) {					\
+		dev_err(__dev,						\
+			"%s: Minimum length for %s is %zu but received command is %zu.\n",	\
+			__func__, __stringify(msg),			\
+			__elem_len, __actual_len);			\
+		__res = -EMSGSIZE;					\
+	} else if ((__actual_len % __elem_len) != 0) {			\
+		dev_err(__dev,						\
+			"%s: Received command length: %zu is not a multiple of %s[0]: %zu\n",	\
+			__func__, __actual_len,				\
+			__stringify(msg), __elem_len);			\
+		__res = -EMSGSIZE;					\
+	} else if (__actual_num_elem >	__expected_num_elem) {		\
+		dev_err(__dev,						\
+			"%s: Array %s is supposed to have %zu elements each of size %zu...\n",	\
+			__func__, __stringify(msg),			\
+			__expected_num_elem, __elem_len);		\
+		dev_err(__dev,						\
+			"... But received command has %zu elements (total length %zu).\n",	\
+			__actual_num_elem, __actual_len);		\
+		__res = -EOVERFLOW;					\
+	}								\
+	__res;								\
+})
+
+/**
+ * es58x_priv() - Get the priv member and cast it to struct es58x_priv.
+ * @netdev: CAN network device.
+ *
+ * Return: ES58X device.
+ */
+static inline struct es58x_priv *es58x_priv(struct net_device *netdev)
+{
+	return (struct es58x_priv *)netdev_priv(netdev);
+}
+
+/**
+ * ES58X_SIZEOF_URB_CMD() - Calculate the maximum length of an urb
+ *	command for a given message field name.
+ * @es58x_urb_cmd_type: type (either "struct es581_4_urb_cmd" or
+ *	"struct es58x_fd_urb_cmd").
+ * @msg_field: name of the message field.
+ *
+ * Return: length of the urb command.
+ */
+#define ES58X_SIZEOF_URB_CMD(es58x_urb_cmd_type, msg_field)		\
+	(offsetof(es58x_urb_cmd_type, raw_msg)				\
+		+ sizeof_field(es58x_urb_cmd_type, msg_field)		\
+		+ sizeof_field(es58x_urb_cmd_type,			\
+			reserved_for_crc16_do_not_use))

static inline?

+
+/**
+ * es58x_get_urb_cmd_len() - Calculate the actual length of an urb
+ *	command for a given message length.
+ * @es58x_dev: ES58X device.
+ * @msg_len: Length of the message.
+ *
+ * Add the header and CRC lengths to the message length.
+ *
+ * Return: length of the urb command.
+ */
+static inline size_t es58x_get_urb_cmd_len(struct es58x_device *es58x_dev,
+					   u16 msg_len)
+{
+	/* URB Length = URB header len + Message len + sizeof crc16 */
+	return es58x_dev->param->urb_cmd_header_len + msg_len + sizeof(u16);
+}
+
+/**
+ * es58x_get_netdev() - Get the network device.
+ * @es58x_dev: ES58X device.
+ * @channel_no: The channel number as advertised in the urb command.
+ * @netdev: CAN network device.
+ *
+ * ES581.4 starts the numbering on channels from 1, ES58X FD family
+ * starts it from 0. This method does the sanity check.
+ *
+ * Return: zero on success, -ECHRNG if the received channel number is
+ * out of range and -ENODEV if the network device is not yet
+ * configured.
+ */
+static inline int es58x_get_netdev(struct es58x_device *es58x_dev,
+				   int channel_no, struct net_device **netdev)
+{
+	int channel_idx = channel_no - es58x_dev->param->channel_idx_offset;
+
+	*netdev = NULL;
+	if (channel_idx < 0 || channel_idx >= es58x_dev->num_can_ch)
+		return -ECHRNG;
+
+	*netdev = es58x_dev->netdev[channel_idx];
+	if (!netdev || !netif_device_present(*netdev))
+		return -ENODEV;
+
+	return 0;
+}
+
+int es58x_can_get_echo_skb(struct net_device *netdev, u32 packet_idx,
+			   u64 *tstamps, unsigned int pkts);
+int es58x_tx_ack_msg(struct net_device *netdev, u16 tx_free_entries,
+		     enum es58x_cmd_ret_code_u32 rx_cmd_ret_u32);
+int es58x_rx_can_msg(struct net_device *netdev,
+		     struct es58x_abstracted_can_frame *es58x_cf,
+		     unsigned int pkts);
+int es58x_rx_err_msg(struct net_device *netdev, enum es58x_error error,
+		     enum es58x_event event, u64 timestamp);
+int es58x_rx_timestamp(struct es58x_device *es58x_dev, u64 timestamp);
+int es58x_rx_dev_ret_u8(struct device *dev,
+			enum es58x_cmd_ret_type cmd_ret_type,
+			enum es58x_dev_ret_code_u8 rx_dev_ret_u8);
+int es58x_rx_cmd_ret_u32(struct net_device *netdev,
+			 enum es58x_cmd_ret_type cmd_ret_type,
+			 enum es58x_cmd_ret_code_u32 rx_cmd_ret_u32);
+int es58x_send_msg(struct es58x_device *es58x_dev, u8 cmd_type, u8 cmd_id,
+		   const void *msg, u16 cmd_len, int channel_idx);
+
+extern const struct es58x_parameters es581_4_param;
+extern const struct es58x_operators es581_4_ops;
+
+extern const struct es58x_parameters es58x_fd_param;
+extern const struct es58x_operators es58x_fd_ops;
+
+#endif				//__ES58X_COMMON_H__

just one space please

regards.
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |