Re: [PATCH v10 1/1] can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/12/21 2:05 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>

[...]

> diff --git a/drivers/net/can/usb/etas_es58x/es58x_core.c b/drivers/net/can/usb/etas_es58x/es58x_core.c
> new file mode 100644
> index 000000000000..30692d78d8e6
> --- /dev/null
> +++ b/drivers/net/can/usb/etas_es58x/es58x_core.c
> @@ -0,0 +1,2589 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/* Driver for ETAS GmbH ES58X USB CAN(-FD) Bus Interfaces.
> + *
> + * File es58x_core.c: Core logic to manage the network devices and the
> + * USB interface.
> + *
> + * Copyright (C) 2019 Robert Bosch Engineering and Business
> + * Solutions. All rights reserved.
> + * Copyright (C) 2020 ETAS K.K.. All rights reserved.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/usb.h>
> +#include <linux/crc16.h>
> +#include <linux/spinlock.h>
> +#include <asm/unaligned.h>
> +
> +#include "es58x_core.h"
> +
> +#define DRV_VERSION "1.00"
> +MODULE_AUTHOR("Mailhol Vincent <mailhol.vincent@xxxxxxxxxx>");
> +MODULE_AUTHOR("Arunachalam Santhanam <arunachalam.santhanam@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Socket CAN driver for ETAS ES58X USB adapters");
> +MODULE_VERSION(DRV_VERSION);
> +MODULE_LICENSE("GPL v2");
> +
> +/* Vendor and product id */
> +#define ES58X_MODULE_NAME "etas_es58x"
> +#define ES58X_VENDOR_ID 0x108C
> +#define ES581_4_PRODUCT_ID 0x0159
> +#define ES582_1_PRODUCT_ID 0x0168
> +#define ES584_1_PRODUCT_ID 0x0169
> +
> +/* Table of devices which work with this driver */
> +static const struct usb_device_id es58x_id_table[] = {
> +	{USB_DEVICE(ES58X_VENDOR_ID, ES581_4_PRODUCT_ID)},
> +	{USB_DEVICE(ES58X_VENDOR_ID, ES582_1_PRODUCT_ID)},
> +	{USB_DEVICE(ES58X_VENDOR_ID, ES584_1_PRODUCT_ID)},
> +	{}			/* Terminating entry */
> +};
> +
> +MODULE_DEVICE_TABLE(usb, es58x_id_table);
> +
> +#define es58x_print_hex_dump(buf, len)					\
> +	print_hex_dump(KERN_DEBUG,					\
> +		       ES58X_MODULE_NAME " " __stringify(buf) ": ",	\
> +		       DUMP_PREFIX_NONE, 16, 1, buf, len, false)
> +
> +#define es58x_print_hex_dump_debug(buf, len)				 \
> +	print_hex_dump_debug(ES58X_MODULE_NAME " " __stringify(buf) ": ",\
> +			     DUMP_PREFIX_NONE, 16, 1, buf, len, false)
> +
> +/* The last two bytes of an ES58X command is a CRC16. The first two
> + * bytes (the start of frame) are skipped and the CRC calculation
> + * starts on the third byte.
> + */
> +#define ES58X_CRC_CALC_OFFSET	2
> +
> +/**
> + * es58x_calculate_crc() - Compute the crc16 of a given URB.
> + * @urb_cmd: The URB command for which we want to calculate the CRC.
> + * @urb_len: Length of @urb_cmd. Must be at least bigger than 4
> + *	(ES58X_CRC_CALC_OFFSET + sizeof(crc))
> + *
> + * Return: crc16 value.
> + */
> +static u16 es58x_calculate_crc(const union es58x_urb_cmd *urb_cmd, u16 urb_len)
> +{
> +	u16 crc;
> +	ssize_t len = urb_len - ES58X_CRC_CALC_OFFSET - sizeof(crc);
> +
> +	WARN_ON(len < 0);

Is it possible to ensure earlier, that the urbs are of correct length?

> +	crc = crc16(0, &urb_cmd->raw_cmd[ES58X_CRC_CALC_OFFSET], len);
> +	return crc;
> +}

[...]

> +/**
> + * 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.
> + * @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.
> + * @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_total_frame_len: sum, in bytes, of the length of each of the
> + *	CAN messages contained in @tx_urb. To be used as an input of
> + *	netdev_sent_queue() for BQL.
> + * @tx_can_msg_cnt: Number of messages in @tx_urb.
> + * @tx_can_msg_is_fd: false: all messages in @tx_urb are Classical
> + *	CAN, true: all messages in @tx_urb are CAN FD. Rationale:
> + *	ES58X FD devices do not allow to mix Classical CAN and FD CAN
> + *	frames in one single bulk transmission.
> + * @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.
> + * @channel_idx: Channel index, starts at zero.
> + */
> +struct es58x_priv {
> +	struct can_priv can;
> +	struct es58x_device *es58x_dev;
> +	struct urb *tx_urb;
> +
> +	spinlock_t echo_skb_spinlock;	/* Comments: c.f. supra */
> +	u32 current_packet_idx;
> +	u16 echo_skb_tail_idx;
> +	u16 echo_skb_head_idx;
> +	u16 num_echo_skb;

Can you explain me how the tx-path works, especially why you need the
current_packet_idx.

In the mcp251xfd driver, the number of TX buffers is a power of two, that makes
things easier. tx_heads % len points to the next buffer to be filled, tx_tail %
len points to the next buffer to be completed. tx_head - tx_tail is the fill
level of the FIFO. This works without spinlocks.

> +
> +	u16 tx_total_frame_len;
> +	u8 tx_can_msg_cnt;
> +	bool tx_can_msg_is_fd;
> +
> +	u8 err_passive_before_rtx_success;
> +
> +	u8 channel_idx;
> +};

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


[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux