Re: [PATCH v3 6/7] can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces

From: Marc Kleine-Budde
Date: Sun Oct 04 2020 - 08:07:26 EST

Next message: Pratik Rajesh Sampat: "[RFC v5 0/1] Selftest for cpuidle latency measurement"
Previous message: Peter Geis: "Re: [BUG] slab: double free detected in cache 'kmalloc-128', objp daff5780"
In reply to: Vincent Mailhol: "[PATCH v3 6/7] can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces"
Next in thread: Vincent Mailhol: "[PATCH v3 7/7] usb: cdc-acm: add quirk to blacklist ETAS ES58X devices"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 <arunachalam.santhanam@xxxxxxxxxxxx>
> Signed-off-by: Arunachalam Santhanam <arunachalam.santhanam@xxxxxxxxxxxx>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
> ---
>
> 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 :)

> diff --git a/drivers/net/can/usb/etas_es58x/es58x_core.h b/drivers/net/can/usb/etas_es58x/es58x_core.h
> new file mode 100644
> index 000000000000..359ddc44a3ad
> --- /dev/null
> +++ b/drivers/net/can/usb/etas_es58x/es58x_core.h
> @@ -0,0 +1,700 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/* Driver for ETAS GmbH ES58X USB CAN(-FD) Bus Interfaces.
> + *
> + * File es58x_core.h: All common definitions and declarations.
> + *
> + * Copyright (C) 2019 Robert Bosch Engineering and Business
> + * Solutions. All rights reserved.
> + * Copyright (C) 2020 ETAS K.K.. All rights reserved.
> + */
> +
> +#ifndef __ES58X_COMMON_H__
> +#define __ES58X_COMMON_H__
> +
> +#include <linux/types.h>
> +#include <linux/usb.h>
> +#include <linux/netdevice.h>
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +
> +#include "es581_4.h"
> +#include "es58x_fd.h"
> +
> +/* Size of a CAN Standard Frame (rounded up and ignoring bitsuffing). */
> +#define ES58X_SFF_BYTES(data_len) (round_up(47, 8) / 8 + (data_len))

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 |

Attachment: signature.asc
Description: OpenPGP digital signature

Next message: Pratik Rajesh Sampat: "[RFC v5 0/1] Selftest for cpuidle latency measurement"
Previous message: Peter Geis: "Re: [BUG] slab: double free detected in cache 'kmalloc-128', objp daff5780"
In reply to: Vincent Mailhol: "[PATCH v3 6/7] can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces"
Next in thread: Vincent Mailhol: "[PATCH v3 7/7] usb: cdc-acm: add quirk to blacklist ETAS ES58X devices"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]