* [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices
@ 2018-03-22 13:53 Jakob Unterwurzacher
2018-03-22 13:53 ` [PATCH v3 1/1] " Jakob Unterwurzacher
2018-03-23 8:32 ` [PATCH v3 0/1] " Wolfgang Grandegger
0 siblings, 2 replies; 11+ messages in thread
From: Jakob Unterwurzacher @ 2018-03-22 13:53 UTC (permalink / raw)
To: jakob.unterwurzacher
Cc: Martin Elshuber, Philipp Tomsich, Wolfgang Grandegger,
Marc Kleine-Budde, linux-can, linux-kernel
This is v3 of the Theobroma Systems CAN/USB adapter driver
upstreaming effort.
Featured v2 -> v3 changes:
* count error frames as data packets
* use canid_t for all can ids
* use BIT(x) instead of (1 << x)
* use __le16 / __le32 for little-endian fields
* add spinlock around context allocation (fixes a possible race)
* fix comment style
* use WARN_ON return value
* fix state logic bug that did not allow return to ERROR_ACTIVE
* drop echo_index from context_array (not needed)
* rename "tx_contexts" -> "context_array" to prevent confusion
* add __func__ to all errors and warnings, and to info where it made sense
Jakob Unterwurzacher (1):
can: ucan: add driver for Theobroma Systems UCAN devices
Documentation/networking/can_ucan_protocol.rst | 315 +++++
Documentation/networking/index.rst | 1 +
drivers/net/can/usb/Kconfig | 10 +
drivers/net/can/usb/Makefile | 1 +
drivers/net/can/usb/ucan.c | 1628 ++++++++++++++++++++++++
5 files changed, 1955 insertions(+)
create mode 100644 Documentation/networking/can_ucan_protocol.rst
create mode 100644 drivers/net/can/usb/ucan.c
--
2.11.0
Cc: Martin Elshuber <martin.elshuber@theobroma-systems.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: linux-can@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-22 13:53 [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices Jakob Unterwurzacher
@ 2018-03-22 13:53 ` Jakob Unterwurzacher
2018-03-23 8:42 ` Wolfgang Grandegger
2018-03-24 11:43 ` kbuild test robot
2018-03-23 8:32 ` [PATCH v3 0/1] " Wolfgang Grandegger
1 sibling, 2 replies; 11+ messages in thread
From: Jakob Unterwurzacher @ 2018-03-22 13:53 UTC (permalink / raw)
To: jakob.unterwurzacher
Cc: Martin Elshuber, Philipp Tomsich, Wolfgang Grandegger,
Marc Kleine-Budde, linux-can, linux-kernel
The UCAN driver supports the microcontroller-based USB/CAN
adapters from Theobroma Systems. There are two form-factors
that run essentially the same firmware:
* Seal: standalone USB stick ( https://www.theobroma-systems.com/seal )
* Mule: integrated on the PCB of various System-on-Modules from
Theobroma Systems like the A31-µQ7 and the RK3399-Q7
( https://www.theobroma-systems.com/rk3399-q7 )
The USB wire protocol has been designed to be as generic and
hardware-indendent as possible in the hope of being useful for
implementation on other microcontrollers.
Signed-off-by: Martin Elshuber <martin.elshuber@theobroma-systems.com>
Signed-off-by: Jakob Unterwurzacher <jakob.unterwurzacher@theobroma-systems.com>
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
Documentation/networking/can_ucan_protocol.rst | 315 +++++
Documentation/networking/index.rst | 1 +
drivers/net/can/usb/Kconfig | 10 +
drivers/net/can/usb/Makefile | 1 +
drivers/net/can/usb/ucan.c | 1628 ++++++++++++++++++++++++
5 files changed, 1955 insertions(+)
create mode 100644 Documentation/networking/can_ucan_protocol.rst
create mode 100644 drivers/net/can/usb/ucan.c
diff --git a/Documentation/networking/can_ucan_protocol.rst b/Documentation/networking/can_ucan_protocol.rst
new file mode 100644
index 000000000000..d859b36200b4
--- /dev/null
+++ b/Documentation/networking/can_ucan_protocol.rst
@@ -0,0 +1,315 @@
+=================
+The UCAN Protocol
+=================
+
+UCAN is the protocol used by the microcontroller-based USB-CAN
+adapter that is integrated on System-on-Modules from Theobroma Systems
+and that is also available as a standalone USB stick.
+
+The UCAN protocol has been designed to be hardware-independent.
+It is modeled closely after how Linux represents CAN devices
+internally. All multi-byte integers are encoded as Little Endian.
+
+All structures mentioned in this document are defined in
+``drivers/net/can/usb/ucan.c``.
+
+USB Endpoints
+=============
+
+UCAN devices use three USB endpoints:
+
+CONTROL endpoint
+ The driver sends device management commands on this endpoint
+
+IN endpoint
+ The device sends CAN data frames and CAN error frames
+
+OUT endpoint
+ The driver sends CAN data frames on the out endpoint
+
+
+CONTROL Messages
+================
+
+UCAN devices are configured using vendor requests on the control pipe.
+
+To support multiple CAN interfaces in a single USB device all
+configuration commands target the corresponding interface in the USB
+descriptor.
+
+The driver uses ``ucan_ctrl_command_in/out`` and
+``ucan_device_request_in`` to deliver commands to the device.
+
+Setup Packet
+------------
+
+================= =====================================================
+``bmRequestType`` Direction | Vendor | (Interface or Device)
+``bRequest`` Command Number
+``wValue`` Subcommand Number (16 Bit) or 0 if not used
+``wIndex`` USB Interface Index (0 for device commands)
+``wLength`` * Host to Device - Number of bytes to transmit
+ * Device to Host - Maximum Number of bytes to
+ receive. If the device send less. Commom ZLP
+ semantics are used.
+================= =====================================================
+
+Error Handling
+--------------
+
+The device indicates failed control commands by stalling the
+pipe.
+
+Device Commands
+---------------
+
+UCAN_DEVICE_GET_FW_STRING
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+*Dev2Host; optional*
+
+Request the device firmware string.
+
+
+Interface Commands
+------------------
+
+UCAN_COMMAND_START
+~~~~~~~~~~~~~~~~~~
+
+*Host2Dev; mandatory*
+
+Bring the CAN interface up.
+
+Payload Format
+ ``ucan_ctl_payload_t.cmd_start``
+
+==== ============================
+mode or mask of ``UCAN_MODE_*``
+==== ============================
+
+UCAN_COMMAND_STOP
+~~~~~~~~~~~~~~~~~~
+
+*Host2Dev; mandatory*
+
+Stop the CAN interface
+
+Payload Format
+ *empty*
+
+UCAN_COMMAND_RESET
+~~~~~~~~~~~~~~~~~~
+
+*Host2Dev; mandatory*
+
+Reset the CAN controller (including error counters)
+
+Payload Format
+ *empty*
+
+UCAN_COMMAND_GET
+~~~~~~~~~~~~~~~~
+
+*Host2Dev; mandatory*
+
+Get Information from the Device
+
+Subcommands
+^^^^^^^^^^^
+
+UCAN_COMMAND_GET_INFO
+ Request the device information structure ``ucan_ctl_payload_t.device_info``.
+
+ See the ``device_info`` field for details, and
+ ``uapi/linux/can/netlink.h`` for an explanation of the
+ ``can_bittiming fields``.
+
+ Payload Format
+ ``ucan_ctl_payload_t.device_info``
+
+UCAN_COMMAND_GET_PROTOCOL_VERSION
+
+ Request the device protocol version
+ ``ucan_ctl_payload_t.protocol_version``. The current protocol version is 3.
+
+ Payload Format
+ ``ucan_ctl_payload_t.protocol_version``
+
+.. note:: Devices that do not implement this command use the old
+ protocol version 1
+
+UCAN_COMMAND_SET_BITTIMING
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+*Host2Dev; mandatory*
+
+Setup bittiming by sending the the structure
+``ucan_ctl_payload_t.cmd_set_bittiming`` (see ``struct bittiming`` for
+details)
+
+Payload Format
+ ``ucan_ctl_payload_t.cmd_set_bittiming``.
+
+UCAN_SLEEP/WAKE
+~~~~~~~~~~~~~~~
+
+*Host2Dev; optional*
+
+Configure sleep and wake modes. Not yet supported by the driver.
+
+UCAN_FILTER
+~~~~~~~~~~~
+
+*Host2Dev; optional*
+
+Setup hardware CAN filters. Not yet supported by the driver.
+
+Allowed interface commands
+--------------------------
+
+================== =================== ==================
+Legal Device State Command New Device State
+================== =================== ==================
+stopped SET_BITTIMING stopped
+stopped START started
+started STOP or RESET stopped
+stopped STOP or RESET stopped
+started RESTART started
+any GET *no change*
+================== =================== ==================
+
+IN Message Format
+=================
+
+A data packet on the USB IN endpoint contains one or more
+``ucan_message_in`` values. If multiple messages are batched in a USB
+data packet, the ``len`` field can be used to jump to the next
+``ucan_message_in`` value (take care to sanity-check the ``len`` value
+against the actual data size).
+
+.. _can_ucan_in_message_len:
+
+``len`` field
+-------------
+
+Each ``ucan_message_in`` must be aligned to a 4-byte boundary (relative
+to the start of the start of the data buffer). That means that there
+may be padding bytes between multiple ``ucan_message_in`` values:
+
+.. code::
+
+ +----------------------------+ < 0
+ | |
+ | struct ucan_message_in |
+ | |
+ +----------------------------+ < len
+ [padding]
+ +----------------------------+ < round_up(len, 4)
+ | |
+ | struct ucan_message_in |
+ | |
+ +----------------------------+
+ [...]
+
+``type`` field
+--------------
+
+The ``type`` field specifies the type of the message.
+
+UCAN_IN_RX
+~~~~~~~~~~
+
+``subtype``
+ zero
+
+Data received from the CAN bus (ID + payload).
+
+UCAN_IN_TX_COMPLETE
+~~~~~~~~~~~~~~~~~~~
+
+``subtype``
+ zero
+
+The CAN device has sent a message to the CAN bus. It answers with a
+set of echo-ids from previous UCAN_OUT_TX messages
+
+Flow Control
+------------
+
+When receiving CAN messages there is no flow control on the USB
+buffer. The driver has to handle inbound message quickly enough to
+avoid drops. I case the device buffer overflow the condition is
+reported by sending corresponding error frames (see
+:ref:`can_ucan_error_handling`)
+
+
+OUT Message Format
+==================
+
+A data packet on the USB OUT endpoint contains one or more ``struct
+ucan_message_out`` values. If multiple messages are batched into one
+data packet, the device uses the ``len`` field to jump to the next
+ucan_message_out value. Each ucan_message_out must be aligned to 4
+bytes (relative to the start of the data buffer). The mechanism is
+same as described in :ref:`can_ucan_in_message_len`.
+
+.. code::
+
+ +----------------------------+ < 0
+ | |
+ | struct ucan_message_out |
+ | |
+ +----------------------------+ < len
+ [padding]
+ +----------------------------+ < round_up(len, 4)
+ | |
+ | struct ucan_message_out |
+ | |
+ +----------------------------+
+ [...]
+
+``type`` field
+--------------
+
+In protocol version 3 only ``UCAN_OUT_TX`` is defined, others are used
+only by legacy devices (protocol version 1).
+
+UCAN_OUT_TX
+~~~~~~~~~~~
+``subtype``
+ echo id to be replied within a CAN_IN_TX_COMPLETE message
+
+Transmit a CAN frame. (parameters: ``id``, ``data``)
+
+Flow Control
+------------
+
+When the device outbound buffers are full it starts sending *NAKs* on
+the *OUT* pipe until more buffers are available. The driver stops the
+queue when a certain threshold of out packets are incomplete.
+
+.. _can_ucan_error_handling:
+
+CAN Error Handling
+==================
+
+If error reporting is turned on the device encodes errors into CAN
+error frames (see ``uapi/linux/can/error.h``) and sends it using the
+IN endpoint. The driver updates its error statistics and forwards
+it.
+
+Although UCAN devices can suppress error frames completely, in Linux
+the driver is always interested. Hence, the device is always started with
+the ``UCAN_MODE_BERR_REPORT`` set. Filtering those messages for the
+user space is done by the driver.
+
+Example Conversation
+====================
+
+#) Device is connected to USB
+#) Host sends command ``UCAN_COMMAND_RESET``, subcmd 0
+#) Host sends command ``UCAN_COMMAND_GET``, subcmd ``UCAN_COMMAND_GET_INFO``
+#) Device sends ``UCAN_IN_DEVICE_INFO``
+#) Host sends command ``UCAN_OUT_SET_BITTIMING``
+#) Host sends command ``UCAN_COMMAND_START``, subcmd 0, mode ``UCAN_MODE_BERR_REPORT``
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 90966c2692d8..18903968cebf 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -8,6 +8,7 @@ Contents:
batman-adv
can
+ can_ucan_protocol
kapi
z8530book
msg_zerocopy
diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index c36f4bdcbf4f..490cdce1f1da 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -89,4 +89,14 @@ config CAN_MCBA_USB
This driver supports the CAN BUS Analyzer interface
from Microchip (http://www.microchip.com/development-tools/).
+config CAN_UCAN
+ tristate "Theobroma Systems UCAN interface"
+ ---help---
+ This driver supports the Theobroma Systems
+ UCAN USB-CAN interface.
+
+ UCAN is an microcontroller-based USB-CAN interface that
+ is integrated on System-on-Modules made by Theobroma Systems
+ (https://www.theobroma-systems.com/som-products).
+
endmenu
diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
index 49ac7b99ba32..4176e8358232 100644
--- a/drivers/net/can/usb/Makefile
+++ b/drivers/net/can/usb/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/
obj-$(CONFIG_CAN_8DEV_USB) += usb_8dev.o
obj-$(CONFIG_CAN_MCBA_USB) += mcba_usb.o
+obj-$(CONFIG_CAN_UCAN) += ucan.o
diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
new file mode 100644
index 000000000000..5a98ef5d95da
--- /dev/null
+++ b/drivers/net/can/usb/ucan.c
@@ -0,0 +1,1628 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/* Driver for Theobroma Systems UCAN devices, Protocol Version 3
+ *
+ * Copyright (C) 2018 Theobroma Systems Design und Consulting GmbH
+ *
+ *
+ * General Description:
+ *
+ * The USB Device uses three Endpoints:
+ *
+ * CONTROL Endpoint: Is used the setup the device (start, stop,
+ * info, configure).
+ *
+ * IN Endpoint: The device sends CAN Frame Messages and Device
+ * Information using the IN endpoint.
+ *
+ * OUT Endpoint: The driver sends configuration requests, and CAN
+ * Frames on the out endpoint.
+ *
+ * Error Handling:
+ *
+ * If error reporting is turned on the device encodes error into CAN
+ * error frames (see uapi/linux/can/error.h) and sends it using the
+ * IN Endpoint. The driver updates statistics and forward it.
+ */
+
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/signal.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+
+#define UCAN_MAX_RX_URBS 8
+/* the CAN controller needs a while to enable/disable the bus */
+#define UCAN_USB_CTL_PIPE_TIMEOUT 1000
+/* this driver currently supports protocol version 3 only */
+#define UCAN_PROTOCOL_VERSION_MIN 3
+#define UCAN_PROTOCOL_VERSION_MAX 3
+
+/* UCAN Message Definitions
+ * ------------------------
+ *
+ * ucan_message_out_t and ucan_message_in_t define the messages
+ * transmitted on the OUT and IN endpoint.
+ *
+ * Multibyte fields are transmitted with little endianness
+ *
+ * INTR Endpoint: a single uint32_t storing the current space in the fifo
+ *
+ * OUT Endpoint: single message of type ucan_message_out_t is
+ * transmitted on the out endpoint
+ *
+ * IN Endpoint: multiple messages ucan_message_in_t concateted in
+ * the following way:
+ *
+ * m[n].len <=> the length if message n(including the header in bytes)
+ * m[n] is is aligned to a 4 byte boundary, hence
+ * offset(m[0]) := 0;
+ * offset(m[n+1]) := offset(m[n]) + (m[n].len + 3) & 3
+ *
+ * this implies that
+ * offset(m[n]) % 4 <=> 0
+ */
+
+/* Device Global Commands */
+enum {
+ UCAN_DEVICE_GET_FW_STRING = 0,
+};
+
+/* UCAN Commands */
+enum {
+ /* start the can transceiver - val defines the operation mode */
+ UCAN_COMMAND_START = 0,
+ /* cancel pending transmissions and stop the can transceiver */
+ UCAN_COMMAND_STOP = 1,
+ /* send can transceiver into low-power sleep mode */
+ UCAN_COMMAND_SLEEP = 2,
+ /* wake up can transceiver from low-power sleep mode */
+ UCAN_COMMAND_WAKEUP = 3,
+ /* reset the can transceiver */
+ UCAN_COMMAND_RESET = 4,
+ /* get piece of info from the can transceiver - subcmd defines what
+ * piece
+ */
+ UCAN_COMMAND_GET = 5,
+ /* clear or disable hardware filter - subcmd defines which of the two */
+ UCAN_COMMAND_FILTER = 6,
+ /* Setup bittiming */
+ UCAN_COMMAND_SET_BITTIMING = 7,
+ /* recover from bus-off state */
+ UCAN_COMMAND_RESTART = 8,
+};
+
+/* UCAN_COMMAND_START and UCAN_COMMAND_GET_INFO operation modes (bitmap).
+ * Undefined bits must be set to 0.
+ */
+enum {
+ UCAN_MODE_LOOPBACK = BIT(0),
+ UCAN_MODE_SILENT = BIT(1),
+ UCAN_MODE_3_SAMPLES = BIT(2),
+ UCAN_MODE_ONE_SHOT = BIT(3),
+ UCAN_MODE_BERR_REPORT = BIT(4),
+};
+
+/* UCAN_COMMAND_GET subcommands */
+enum {
+ UCAN_COMMAND_GET_INFO = 0,
+ UCAN_COMMAND_GET_PROTOCOL_VERSION = 1,
+};
+
+/* UCAN_COMMAND_FILTER subcommands */
+enum {
+ UCAN_FILTER_CLEAR = 0,
+ UCAN_FILTER_DISABLE = 1,
+ UCAN_FILTER_ENABLE = 2,
+};
+
+/* OUT endpoint message types */
+enum {
+ UCAN_OUT_TX = 2, /* transmit a CAN frame */
+};
+
+/* IN endpoint message types */
+enum {
+ UCAN_IN_TX_COMPLETE = 1, /* CAN frame transmission completed */
+ UCAN_IN_RX = 2, /* CAN frame received */
+};
+
+struct ucan_ctl_cmd_start {
+ __le16 mode; /* OR-ing any of UCAN_MODE_* */
+} __packed;
+
+struct ucan_ctl_cmd_set_bittiming {
+ __le32 tq; /* Time quanta (TQ) in nanoseconds */
+ __le16 brp; /* TQ Prescaler */
+ __le16 sample_point; /* Samplepoint on tenth percent */
+ u8 prop_seg; /* Propagation segment in TQs */
+ u8 phase_seg1; /* Phase buffer segment 1 in TQs */
+ u8 phase_seg2; /* Phase buffer segment 2 in TQs */
+ u8 sjw; /* Synchronisation jump width in TQs */
+} __packed;
+
+struct ucan_ctl_cmd_device_info {
+ __le32 freq; /* Clock Frequency for tq generation */
+ u8 tx_fifo; /* Size of the transmission fifo */
+ u8 sjw_max; /* can_bittiming fields... */
+ u8 tseg1_min;
+ u8 tseg1_max;
+ u8 tseg2_min;
+ u8 tseg2_max;
+ __le16 brp_inc;
+ __le32 brp_min;
+ __le32 brp_max; /* ...can_bittiming fields */
+ __le16 ctrlmodes; /* supported control modes */
+ __le16 hwfilter; /* Number of HW filter banks */
+ __le16 rxmboxes; /* Number of receive Mailboxes */
+} __packed;
+
+struct ucan_ctl_cmd_get_protocol_version {
+ __le32 version;
+} __packed;
+
+union ucan_ctl_payload {
+ /* Setup Bittiming
+ * bmRequest == UCAN_COMMAND_START
+ */
+ struct ucan_ctl_cmd_start cmd_start;
+ /* Setup Bittiming
+ * bmRequest == UCAN_COMMAND_SET_BITTIMING
+ */
+ struct ucan_ctl_cmd_set_bittiming cmd_set_bittiming;
+ /* Get Device Information
+ * bmRequest == UCAN_COMMAND_GET; wValue = UCAN_COMMAND_GET_INFO
+ */
+ struct ucan_ctl_cmd_device_info cmd_get_device_info;
+ /* Get Protocol Version
+ * bmRequest == UCAN_COMMAND_GET;
+ * wValue = UCAN_COMMAND_GET_PROTOCOL_VERSION
+ */
+ struct ucan_ctl_cmd_get_protocol_version cmd_get_protocol_version;
+
+ u8 raw[128];
+} __packed;
+
+enum {
+ UCAN_TX_COMPLETE_SUCCESS = BIT(0),
+};
+
+/* Transmission Complete within ucan_message_in */
+struct ucan_tx_complete_entry_t {
+ u8 echo_index;
+ u8 flags;
+} __packed __aligned(0x2);
+
+/* CAN Data message format within ucan_message_in/out */
+struct ucan_can_msg {
+ /* note DLC is computed by
+ * msg.len - sizeof (msg.len)
+ * - sizeof (msg.type)
+ * - sizeof (msg.can_msg.id)
+ */
+ __le32 id;
+
+ union {
+ u8 data[CAN_MAX_DLEN]; /* Data of CAN frames */
+ u8 dlc; /* RTR dlc */
+ };
+} __packed;
+
+/* OUT Endpoint, outbound messages */
+struct ucan_message_out {
+ __le16 len; /* Length of the content include header */
+ u8 type; /* UCAN_OUT_TX and friends */
+ u8 subtype; /* command sub type */
+
+ union {
+ /* Transmit CAN frame
+ * (type == UCAN_TX) && ((msg.can_msg.id & CAN_RTR_FLAG) == 0)
+ * subtype stores the echo id
+ */
+ struct ucan_can_msg can_msg;
+ } msg;
+} __packed __aligned(0x4);
+
+/* IN Endpoint, inbound messages */
+struct ucan_message_in {
+ __le16 len; /* Length of the content include header */
+ u8 type; /* UCAN_IN_RX and friends */
+ u8 subtype; /* command sub type */
+
+ union {
+ /* CAN Frame received
+ * (type == UCAN_IN_RX)
+ * && ((msg.can_msg.id & CAN_RTR_FLAG) == 0)
+ */
+ struct ucan_can_msg can_msg;
+
+ /* CAN transmission complete
+ * (type == UCAN_IN_TX_COMPLETE)
+ */
+ struct ucan_tx_complete_entry_t can_tx_complete_msg[0];
+ } __aligned(0x4) msg;
+} __packed;
+
+/* Macros to calculate message lengths */
+#define UCAN_OUT_HDR_SIZE offsetof(struct ucan_message_out, msg)
+
+#define UCAN_IN_HDR_SIZE offsetof(struct ucan_message_in, msg)
+#define UCAN_IN_LEN(member) (UCAN_OUT_HDR_SIZE + sizeof(member))
+
+struct ucan_priv;
+
+/* Context Information for transmission URBs */
+struct ucan_urb_context {
+ struct ucan_priv *up;
+ u8 dlc;
+ bool allocated;
+};
+
+/* Information reported by the USB device */
+struct ucan_device_info {
+ struct can_bittiming_const bittiming_const;
+ u8 tx_fifo;
+};
+
+/* Driver private data */
+struct ucan_priv {
+ struct can_priv can; /* must be the first member */
+
+ u8 intf_index;
+ struct usb_device *udev;
+ struct usb_interface *intf;
+ struct net_device *netdev;
+
+ struct usb_endpoint_descriptor *out_ep;
+ struct usb_endpoint_descriptor *in_ep;
+
+ struct usb_anchor rx_urbs;
+ struct usb_anchor tx_urbs;
+
+ union ucan_ctl_payload *ctl_msg_buffer;
+ struct ucan_device_info device_info;
+
+ /* protects available_tx_urbs and context_array */
+ spinlock_t context_lock;
+ unsigned int available_tx_urbs;
+ struct ucan_urb_context *context_array;
+};
+
+static u8 ucan_get_can_dlc(struct ucan_can_msg *msg, u16 len)
+{
+ if (msg->id & CAN_RTR_FLAG)
+ return get_can_dlc(msg->dlc);
+ else
+ return get_can_dlc(len - (UCAN_IN_HDR_SIZE + sizeof(msg->id)));
+}
+
+static void ucan_release_context_array(struct ucan_priv *up)
+{
+ if (!up->context_array)
+ return;
+
+ /* lock is not needed because, driver is currently opening or closing */
+ up->available_tx_urbs = 0;
+
+ kfree(up->context_array);
+ up->context_array = NULL;
+}
+
+static int ucan_alloc_context_array(struct ucan_priv *up)
+{
+ int i;
+
+ /* release contexts if any */
+ ucan_release_context_array(up);
+
+ up->context_array = kcalloc(up->device_info.tx_fifo,
+ sizeof(*up->context_array),
+ GFP_KERNEL);
+ if (!up->context_array) {
+ dev_err(&up->udev->dev,
+ "%s: Not enough memory to allocate tx contexts\n",
+ __func__);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < up->device_info.tx_fifo; i++) {
+ up->context_array[i].allocated = false;
+ up->context_array[i].up = up;
+ }
+
+ /* lock is not needed because, driver is currently opening */
+ up->available_tx_urbs = up->device_info.tx_fifo;
+
+ return 0;
+}
+
+static struct ucan_urb_context *ucan_alloc_context(struct ucan_priv *up)
+{
+ int i;
+ unsigned long flags;
+ struct ucan_urb_context *ret = NULL;
+
+ if (WARN_ON_ONCE(!up->context_array))
+ return NULL;
+
+ /* execute context operation atomically */
+ spin_lock_irqsave(&up->context_lock, flags);
+
+ for (i = 0; i < up->device_info.tx_fifo; i++) {
+ if (!up->context_array[i].allocated) {
+ /* update context */
+ ret = &up->context_array[i];
+ up->context_array[i].allocated = true;
+
+ /* stop queue if necessary */
+ up->available_tx_urbs--;
+ if (!up->available_tx_urbs)
+ netif_stop_queue(up->netdev);
+
+ goto done_restore;
+ }
+ }
+
+done_restore:
+ spin_unlock_irqrestore(&up->context_lock, flags);
+ return ret;
+}
+
+static bool ucan_release_context(struct ucan_priv *up,
+ struct ucan_urb_context *ctx)
+{
+ unsigned long flags;
+ bool ret = false;
+
+ if (WARN_ON_ONCE(!up->context_array))
+ return false;
+
+ /* execute context operation atomically */
+ spin_lock_irqsave(&up->context_lock, flags);
+
+ /* context was not allocated, maybe the device sent garbage */
+ if (!ctx->allocated)
+ goto done_restore;
+ ctx->allocated = false;
+
+ /* check if the queue needs to be woken */
+ if (!up->available_tx_urbs)
+ netif_wake_queue(up->netdev);
+ up->available_tx_urbs++;
+
+done_restore:
+ spin_unlock_irqrestore(&up->context_lock, flags);
+ return ret;
+}
+
+static int ucan_ctrl_command_out(struct ucan_priv *up,
+ u8 cmd, u16 subcmd, u16 datalen)
+{
+ return usb_control_msg(up->udev,
+ usb_sndctrlpipe(up->udev, 0),
+ cmd,
+ USB_DIR_OUT | USB_TYPE_VENDOR |
+ USB_RECIP_INTERFACE,
+ cpu_to_le16(subcmd),
+ up->intf_index,
+ up->ctl_msg_buffer,
+ datalen,
+ UCAN_USB_CTL_PIPE_TIMEOUT);
+}
+
+static int ucan_device_request_in(struct ucan_priv *up,
+ u8 cmd, u16 subcmd, u16 datalen)
+{
+ return usb_control_msg(up->udev,
+ usb_rcvctrlpipe(up->udev, 0),
+ cmd,
+ USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ cpu_to_le16(subcmd),
+ 0,
+ up->ctl_msg_buffer,
+ datalen,
+ UCAN_USB_CTL_PIPE_TIMEOUT);
+}
+
+/* Parse the device information structure reported by the device and
+ * setup private variables accordingly
+ */
+static void ucan_parse_device_info(struct ucan_priv *up,
+ struct ucan_ctl_cmd_device_info *ctl_cmd_device_info)
+{
+ struct can_bittiming_const *bittiming =
+ &up->device_info.bittiming_const;
+ u16 ctrlmodes;
+
+ /* store the data */
+ up->can.clock.freq = le32_to_cpu(ctl_cmd_device_info->freq);
+ up->device_info.tx_fifo = ctl_cmd_device_info->tx_fifo;
+ strcpy(bittiming->name, "ucan");
+ bittiming->tseg1_min = ctl_cmd_device_info->tseg1_min;
+ bittiming->tseg1_max = ctl_cmd_device_info->tseg1_max;
+ bittiming->tseg2_min = ctl_cmd_device_info->tseg2_min;
+ bittiming->tseg2_max = ctl_cmd_device_info->tseg2_max;
+ bittiming->sjw_max = ctl_cmd_device_info->sjw_max;
+ bittiming->brp_min = le32_to_cpu(ctl_cmd_device_info->brp_min);
+ bittiming->brp_max = le32_to_cpu(ctl_cmd_device_info->brp_max);
+ bittiming->brp_inc = le16_to_cpu(ctl_cmd_device_info->brp_inc);
+
+ ctrlmodes = le16_to_cpu(ctl_cmd_device_info->ctrlmodes);
+
+ up->can.ctrlmode_supported = 0;
+
+ if (ctrlmodes & UCAN_MODE_LOOPBACK)
+ up->can.ctrlmode_supported |= CAN_CTRLMODE_LOOPBACK;
+ if (ctrlmodes & UCAN_MODE_SILENT)
+ up->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
+ if (ctrlmodes & UCAN_MODE_3_SAMPLES)
+ up->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
+ if (ctrlmodes & UCAN_MODE_ONE_SHOT)
+ up->can.ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT;
+ if (ctrlmodes & UCAN_MODE_BERR_REPORT)
+ up->can.ctrlmode_supported |= CAN_CTRLMODE_BERR_REPORTING;
+}
+
+/* Handle a CAN error frame that we have received from the device.
+ * Returns true if the can state has changed.
+ */
+static bool ucan_handle_error_frame(struct ucan_priv *up,
+ struct ucan_message_in *m, canid_t canid)
+{
+ enum can_state new_state = up->can.state;
+ struct net_device_stats *net_stats = &up->netdev->stats;
+ struct can_device_stats *can_stats = &up->can.can_stats;
+
+ if (canid & CAN_ERR_LOSTARB)
+ can_stats->arbitration_lost++;
+
+ if (canid & CAN_ERR_BUSERROR)
+ can_stats->bus_error++;
+
+ if (canid & CAN_ERR_ACK)
+ net_stats->tx_errors++;
+
+ if (canid & CAN_ERR_BUSOFF)
+ new_state = CAN_STATE_BUS_OFF;
+
+ /* controller problems, details in data[1] */
+ if (canid & CAN_ERR_CRTL) {
+ u8 d1 = m->msg.can_msg.data[1];
+
+ if (d1 & CAN_ERR_CRTL_RX_OVERFLOW)
+ net_stats->rx_over_errors++;
+
+ /* controller state bits: if multiple are set the worst wins */
+ if (d1 & CAN_ERR_CRTL_ACTIVE)
+ new_state = CAN_STATE_ERROR_ACTIVE;
+
+ if (d1 & (CAN_ERR_CRTL_RX_WARNING | CAN_ERR_CRTL_TX_WARNING))
+ new_state = CAN_STATE_ERROR_WARNING;
+
+ if (d1 & (CAN_ERR_CRTL_RX_PASSIVE | CAN_ERR_CRTL_TX_PASSIVE))
+ new_state = CAN_STATE_ERROR_PASSIVE;
+ }
+
+ /* protocol error, details in data[2] */
+ if (canid & CAN_ERR_PROT) {
+ u8 d2 = m->msg.can_msg.data[2];
+
+ if (d2 & CAN_ERR_PROT_TX)
+ net_stats->tx_errors++;
+ else
+ net_stats->rx_errors++;
+ }
+
+ /* no state change - we are done */
+ if (up->can.state == new_state)
+ return false;
+
+ /* we switched into a better state */
+ if (up->can.state > new_state) {
+ up->can.state = new_state;
+ return true;
+ }
+
+ /* we switched into a worse state */
+ up->can.state = new_state;
+ switch (new_state) {
+ case CAN_STATE_BUS_OFF:
+ can_stats->bus_off++;
+ can_bus_off(up->netdev);
+ netdev_info(up->netdev, "link has gone into BUS-OFF state\n");
+ break;
+ case CAN_STATE_ERROR_PASSIVE:
+ can_stats->error_passive++;
+ break;
+ case CAN_STATE_ERROR_WARNING:
+ can_stats->error_warning++;
+ break;
+ default:
+ break;
+ }
+ return true;
+}
+
+/* Callback on reception of a can frame via the IN endpoint
+ *
+ * This function allocates an skb and transferres it to the Linux
+ * network stack
+ */
+static void ucan_rx_can_msg(struct ucan_priv *up, struct ucan_message_in *m)
+{
+ int len;
+ canid_t canid, canid_mask;
+ struct can_frame *cf;
+ struct sk_buff *skb;
+ struct net_device_stats *stats = &up->netdev->stats;
+
+ /* get the contents of the length field */
+ len = le16_to_cpu(m->len);
+
+ /* check sanity */
+ if (len < UCAN_IN_HDR_SIZE + sizeof(m->msg.can_msg.id)) {
+ dev_warn(&up->udev->dev,
+ "%s: invalid input message len: %d\n", __func__, len);
+ return;
+ }
+
+ /* handle error frames */
+ canid = le32_to_cpu(m->msg.can_msg.id);
+ if (canid & CAN_ERR_FLAG) {
+ bool busstate_changed = ucan_handle_error_frame(up, m, canid);
+ /* if berr-reporting is off only state changes get through */
+ if (!(up->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
+ !busstate_changed)
+ return;
+ }
+
+ /* allocate skb */
+ skb = alloc_can_skb(up->netdev, &cf);
+ if (!skb)
+ return;
+
+ /* compute the mask for canid */
+ canid_mask = CAN_RTR_FLAG | CAN_ERR_FLAG;
+ if (canid & CAN_EFF_FLAG)
+ canid_mask |= CAN_EFF_MASK | CAN_EFF_FLAG;
+ else
+ canid_mask |= CAN_SFF_MASK;
+
+ /* fill the can frame */
+ cf->can_id = canid & canid_mask;
+
+ /* compute DLC taking RTR_FLAG into account */
+ cf->can_dlc = ucan_get_can_dlc(&m->msg.can_msg, len);
+
+ /* copy the payload of non RTR frames */
+ if (!(cf->can_id & CAN_RTR_FLAG))
+ memcpy(cf->data, m->msg.can_msg.data, cf->can_dlc);
+
+ /* don't count error frames as real packets */
+ stats->rx_packets++;
+ stats->rx_bytes += cf->can_dlc;
+
+ /* pass it to Linux */
+ netif_rx(skb);
+}
+
+/* callback indicating completed transmission */
+static void ucan_tx_complete_msg(struct ucan_priv *up,
+ struct ucan_message_in *m)
+{
+ u16 count, i;
+ u8 echo_index, dlc;
+ u16 len = le16_to_cpu(m->len);
+
+ struct ucan_urb_context *context;
+
+ if (len < UCAN_IN_HDR_SIZE || (len % 2 != 0)) {
+ dev_err(&up->udev->dev,
+ "%s: invalid tx complete length\n",
+ __func__);
+ return;
+ }
+
+ count = (len - UCAN_IN_HDR_SIZE) / 2;
+ for (i = 0; i < count; i++) {
+ /* we did not submit such echo ids */
+ echo_index = m->msg.can_tx_complete_msg[i].echo_index;
+ if (echo_index >= up->device_info.tx_fifo) {
+ up->netdev->stats.tx_errors++;
+ dev_err(&up->udev->dev,
+ "%s: device answered with invalid echo_index\n",
+ __func__);
+ continue;
+ }
+
+ /* gather information from the context */
+ context = &up->context_array[echo_index];
+ dlc = READ_ONCE(context->dlc);
+
+ /* Release context and restart queue if necessary.
+ * Also check if the context was allocated
+ */
+ if (ucan_release_context(up, context))
+ continue;
+
+ if (m->msg.can_tx_complete_msg[i].flags &
+ UCAN_TX_COMPLETE_SUCCESS) {
+ /* update statistics */
+ up->netdev->stats.tx_packets++;
+ up->netdev->stats.tx_bytes += dlc;
+ can_get_echo_skb(up->netdev, echo_index);
+ } else {
+ up->netdev->stats.tx_dropped++;
+ can_free_echo_skb(up->netdev, echo_index);
+ }
+ }
+}
+
+/* callback on reception of a USB message */
+static void ucan_read_bulk_callback(struct urb *urb)
+{
+ int ret;
+ int pos;
+ struct ucan_priv *up = urb->context;
+ struct net_device *netdev = up->netdev;
+ struct ucan_message_in *m;
+
+ /* the device is not up and the driver should not receive any
+ * data on the bulk in pipe
+ */
+ if (WARN_ON(!up->context_array)) {
+ usb_free_coherent(up->udev,
+ up->in_ep->wMaxPacketSize,
+ urb->transfer_buffer,
+ urb->transfer_dma);
+ return;
+ }
+
+ /* check URB status */
+ switch (urb->status) {
+ case 0:
+ break;
+ case -ENOENT:
+ case -EPIPE:
+ case -EPROTO:
+ case -ESHUTDOWN:
+ case -ETIME:
+ /* urb is not resubmitted -> free dma data */
+ usb_free_coherent(up->udev,
+ up->in_ep->wMaxPacketSize,
+ urb->transfer_buffer,
+ urb->transfer_dma);
+ dev_dbg(&up->udev->dev,
+ "%s: ENOENT|EPIPE|EPROTO|ESHUTDOWN|ETIME\n",
+ __func__);
+ return;
+ default:
+ goto resubmit;
+ }
+
+ /* sanity check */
+ if (!netif_device_present(netdev))
+ return;
+
+ /* iterate over input */
+ pos = 0;
+ while (pos < urb->actual_length) {
+ int len;
+
+ /* check sanity (length of header) */
+ if ((urb->actual_length - pos) < UCAN_IN_HDR_SIZE) {
+ dev_warn(&up->udev->dev,
+ "%s: invalid input message %d; too short (no header)\n",
+ __func__,
+ urb->actual_length);
+ goto resubmit;
+ }
+
+ /* setup the message address */
+ m = (struct ucan_message_in *)
+ ((u8 *)urb->transfer_buffer + pos);
+ len = le16_to_cpu(m->len);
+
+ /* check sanity (length of content) */
+ if (urb->actual_length - pos < len) {
+ dev_warn(&up->udev->dev,
+ "%s: invalid input message al:%d pos:%d len:%d; too short (no data)\n",
+ __func__,
+ urb->actual_length, pos, len);
+ print_hex_dump(KERN_WARNING,
+ "raw data: ",
+ DUMP_PREFIX_ADDRESS,
+ 16,
+ 1,
+ urb->transfer_buffer,
+ urb->actual_length,
+ true);
+
+ goto resubmit;
+ }
+
+ switch (le16_to_cpu(m->type)) {
+ case UCAN_IN_RX:
+ ucan_rx_can_msg(up, m);
+ break;
+ case UCAN_IN_TX_COMPLETE:
+ ucan_tx_complete_msg(up, m);
+ break;
+ default:
+ dev_warn(&up->udev->dev,
+ "%s: invalid input message type\n",
+ __func__);
+ break;
+ }
+
+ /* proceed to next message */
+ pos += len;
+ /* align to 4 byte boundary */
+ pos = round_up(pos, 4);
+ }
+
+resubmit:
+ /* resubmit urb when done */
+ usb_fill_bulk_urb(urb, up->udev,
+ usb_rcvbulkpipe(up->udev,
+ up->in_ep->bEndpointAddress &
+ USB_ENDPOINT_NUMBER_MASK),
+ urb->transfer_buffer,
+ up->in_ep->wMaxPacketSize,
+ ucan_read_bulk_callback,
+ up);
+
+ usb_anchor_urb(urb, &up->rx_urbs);
+ ret = usb_submit_urb(urb, GFP_KERNEL);
+
+ if (ret < 0) {
+ dev_err(&up->udev->dev,
+ "%s: failed resubmitting read bulk urb: %d\n",
+ __func__,
+ ret);
+
+ usb_unanchor_urb(urb);
+ usb_free_coherent(up->udev, up->in_ep->wMaxPacketSize,
+ urb->transfer_buffer,
+ urb->transfer_dma);
+
+ if (ret == -ENODEV)
+ netif_device_detach(netdev);
+ }
+}
+
+/* callback after transmission of a USB message */
+static void ucan_write_bulk_callback(struct urb *urb)
+{
+ struct ucan_urb_context *context = urb->context;
+ struct ucan_priv *up;
+
+ /* get the urb context */
+ if (WARN_ON_ONCE(!context))
+ return;
+
+ /* free up our allocated buffer */
+ usb_free_coherent(urb->dev,
+ sizeof(struct ucan_message_out),
+ urb->transfer_buffer,
+ urb->transfer_dma);
+
+ up = context->up;
+ if (WARN_ON_ONCE(!up))
+ return;
+
+ /* sanity check */
+ if (!netif_device_present(up->netdev))
+ return;
+
+ /* transmission failed (USB - the device will not send a TX complete) */
+ if (urb->status) {
+ dev_warn_once(&up->udev->dev,
+ "%s: failed to transmit USB message to device: %d\n",
+ __func__,
+ urb->status);
+
+ /* update counters an cleanup */
+ can_free_echo_skb(up->netdev, context - up->context_array);
+
+ up->netdev->stats.tx_dropped++;
+
+ /* release context and restart the queue if necessary */
+ if (!ucan_release_context(up, context))
+ dev_err(&up->udev->dev,
+ "%s: urb failed, failed to release context\n",
+ __func__);
+ }
+}
+
+static void ucan_cleanup_rx_urbs(struct ucan_priv *up, struct urb **urbs)
+{
+ int i;
+
+ for (i = 0; i < UCAN_MAX_RX_URBS; i++) {
+ if (urbs[i]) {
+ usb_unanchor_urb(urbs[i]);
+ usb_free_coherent(up->udev, up->in_ep->wMaxPacketSize,
+ urbs[i]->transfer_buffer,
+ urbs[i]->transfer_dma);
+ usb_free_urb(urbs[i]);
+ }
+ }
+
+ memset(urbs, 0, sizeof(*urbs) * UCAN_MAX_RX_URBS);
+}
+
+static int ucan_prepare_and_anchor_rx_urbs(struct ucan_priv *up,
+ struct urb **urbs)
+{
+ int i;
+
+ memset(urbs, 0, sizeof(*urbs) * UCAN_MAX_RX_URBS);
+
+ for (i = 0; i < UCAN_MAX_RX_URBS; i++) {
+ void *buf;
+
+ urbs[i] = usb_alloc_urb(0, GFP_KERNEL);
+ if (!urbs[i])
+ goto err;
+
+ buf = usb_alloc_coherent(up->udev, up->in_ep->wMaxPacketSize,
+ GFP_KERNEL, &urbs[i]->transfer_dma);
+ if (!buf) {
+ /* cleanup this urb */
+ usb_free_urb(urbs[i]);
+ urbs[i] = NULL;
+ goto err;
+ }
+
+ usb_fill_bulk_urb(urbs[i], up->udev,
+ usb_rcvbulkpipe(up->udev,
+ up->in_ep->bEndpointAddress &
+ USB_ENDPOINT_NUMBER_MASK),
+ buf,
+ up->in_ep->wMaxPacketSize,
+ ucan_read_bulk_callback,
+ up);
+
+ urbs[i]->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+ usb_anchor_urb(urbs[i], &up->rx_urbs);
+ }
+ return 0;
+
+err:
+ /* cleanup other unsubmitted urbs */
+ ucan_cleanup_rx_urbs(up, urbs);
+ return -ENOMEM;
+}
+
+/* Submits rx urbs with the semantic: Either submit all, or cleanup
+ * everything. I case of errors submitted urbs are killed and all urbs in
+ * the array are freed. I case of no errors every entry in the urb
+ * array is set to NULL.
+ */
+static int ucan_submit_rx_urbs(struct ucan_priv *up, struct urb **urbs)
+{
+ int i, ret;
+
+ /* Iterate over all urbs to submit. On success remove the urb
+ * from the list.
+ */
+ for (i = 0; i < UCAN_MAX_RX_URBS; i++) {
+ ret = usb_submit_urb(urbs[i], GFP_KERNEL);
+ if (ret) {
+ dev_err(&up->udev->dev,
+ "%s: could not submit urb; code: %d\n",
+ __func__, ret);
+ goto err;
+ }
+
+ /* Anchor URB and drop reference, USB core will take
+ * care of freeing it
+ */
+ usb_free_urb(urbs[i]);
+ urbs[i] = NULL;
+ }
+ return 0;
+
+err:
+ /* Cleanup unsubmitted urbs */
+ ucan_cleanup_rx_urbs(up, urbs);
+
+ /* Kill urbs that are already submitted */
+ usb_kill_anchored_urbs(&up->rx_urbs);
+
+ return ret;
+}
+
+/* Open the network device */
+static int ucan_open(struct net_device *netdev)
+{
+ int ret, ret_cleanup;
+ u16 ctrlmode;
+ struct urb *urbs[UCAN_MAX_RX_URBS];
+ struct ucan_priv *up = netdev_priv(netdev);
+
+ ret = ucan_alloc_context_array(up);
+ if (ret)
+ goto err;
+
+ /* Allocate and prepare IN URBS - allocated and anchored
+ * urbs are stored in urbs[] for clean
+ */
+ ret = ucan_prepare_and_anchor_rx_urbs(up, urbs);
+ if (ret)
+ goto err_contexts;
+
+ /* Check the control mode */
+ ctrlmode = 0;
+ if (up->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
+ ctrlmode |= UCAN_MODE_LOOPBACK;
+ if (up->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
+ ctrlmode |= UCAN_MODE_SILENT;
+ if (up->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+ ctrlmode |= UCAN_MODE_3_SAMPLES;
+ if (up->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
+ ctrlmode |= UCAN_MODE_ONE_SHOT;
+
+ /* Enable this in any case - filtering is down within the
+ * receive path
+ */
+ ctrlmode |= UCAN_MODE_BERR_REPORT;
+ up->ctl_msg_buffer->cmd_start.mode = cpu_to_le16(ctrlmode);
+
+ /* Driver is ready to receive data - start the USB device */
+ ret = ucan_ctrl_command_out(up, UCAN_COMMAND_START, 0, 2);
+ if (ret < 0) {
+ dev_err(&up->udev->dev,
+ "%s: could not start UCAN device, code: %d\n",
+ __func__, ret);
+ goto err_reset;
+ }
+
+ /* Call CAN layer open */
+ ret = open_candev(netdev);
+ if (ret)
+ goto err_stop;
+
+ /* Driver is ready to receive data. Submit RX URBS */
+ ret = ucan_submit_rx_urbs(up, urbs);
+ if (ret)
+ goto err_stop;
+
+ up->can.state = CAN_STATE_ERROR_ACTIVE;
+
+ /* Start the network queue */
+ netif_start_queue(netdev);
+
+ return 0;
+
+err_stop:
+ /* The device have started already stop it */
+ ret_cleanup = ucan_ctrl_command_out(up, UCAN_COMMAND_STOP, 0, 0);
+ if (ret_cleanup < 0)
+ dev_err(&up->udev->dev,
+ "%s: could not stop UCAN device, code: %d\n",
+ __func__, ret_cleanup);
+
+err_reset:
+ /* The device might have received data, reset it for
+ * consistent state
+ */
+ ret_cleanup = ucan_ctrl_command_out(up, UCAN_COMMAND_RESET, 0, 0);
+ if (ret_cleanup < 0)
+ dev_err(&up->udev->dev,
+ "%s: could not reset UCAN device, code: %d\n",
+ __func__, ret_cleanup);
+
+ /* clean up unsubmitted urbs */
+ ucan_cleanup_rx_urbs(up, urbs);
+
+err_contexts:
+ ucan_release_context_array(up);
+
+err:
+ return ret;
+}
+
+static struct urb *ucan_prepare_tx_urb(struct ucan_priv *up,
+ struct ucan_urb_context *context,
+ struct can_frame *cf,
+ u8 echo_index)
+{
+ int mlen;
+ struct urb *urb;
+ struct ucan_message_out *m;
+
+ /* create a URB, and a buffer for it, and copy the data to the URB */
+ urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (!urb) {
+ netdev_err(up->netdev,
+ "%s: no memory left for URBs\n", __func__);
+ return NULL;
+ }
+
+ m = usb_alloc_coherent(up->udev,
+ sizeof(struct ucan_message_out),
+ GFP_ATOMIC,
+ &urb->transfer_dma);
+ if (!m) {
+ netdev_err(up->netdev,
+ "%s: no memory left for USB buffer\n", __func__);
+ usb_free_urb(urb);
+ return NULL;
+ }
+
+ /* build the USB message */
+ m->type = UCAN_OUT_TX;
+ m->msg.can_msg.id = cpu_to_le32(cf->can_id);
+
+ if (cf->can_id & CAN_RTR_FLAG) {
+ mlen = UCAN_OUT_HDR_SIZE +
+ offsetof(struct ucan_can_msg, dlc) +
+ sizeof(m->msg.can_msg.dlc);
+ m->msg.can_msg.dlc = cf->can_dlc;
+ } else {
+ mlen = UCAN_OUT_HDR_SIZE +
+ sizeof(m->msg.can_msg.id) + cf->can_dlc;
+ memcpy(m->msg.can_msg.data, cf->data, cf->can_dlc);
+ }
+ m->len = cpu_to_le16(mlen);
+
+ context->dlc = cf->can_dlc;
+
+ m->subtype = echo_index;
+
+ /* build the urb */
+ usb_fill_bulk_urb(urb, up->udev,
+ usb_sndbulkpipe(up->udev,
+ up->out_ep->bEndpointAddress &
+ USB_ENDPOINT_NUMBER_MASK),
+ m, mlen, ucan_write_bulk_callback, context);
+ urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+ return urb;
+}
+
+static void ucan_clean_up_tx_urb(struct ucan_priv *up, struct urb *urb)
+{
+ usb_free_coherent(up->udev, sizeof(struct ucan_message_out),
+ urb->transfer_buffer, urb->transfer_dma);
+ usb_free_urb(urb);
+}
+
+/* callback when Linux needs to send a can frame */
+static netdev_tx_t ucan_start_xmit(struct sk_buff *skb,
+ struct net_device *netdev)
+{
+ int ret;
+ u8 echo_index;
+ struct urb *urb;
+ struct ucan_urb_context *context;
+ struct ucan_priv *up = netdev_priv(netdev);
+ struct can_frame *cf = (struct can_frame *)skb->data;
+
+ /* check skb */
+ if (can_dropped_invalid_skb(netdev, skb))
+ return NETDEV_TX_OK;
+
+ /* allocate a context and slow down tx path, if fifo state is low */
+ context = ucan_alloc_context(up);
+ echo_index = context - up->context_array;
+
+ if (WARN_ON_ONCE(!context))
+ return NETDEV_TX_BUSY;
+
+ /* prepare urb for transmission */
+ urb = ucan_prepare_tx_urb(up, context, cf, echo_index);
+ if (!urb)
+ goto drop;
+
+ /* put the skb on can loopback stack */
+ can_put_echo_skb(skb, up->netdev, echo_index);
+
+ /* transmit it */
+ usb_anchor_urb(urb, &up->tx_urbs);
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
+
+ /* cleanup urb */
+ if (ret) {
+ /* on error, clean up */
+ usb_unanchor_urb(urb);
+ ucan_clean_up_tx_urb(up, urb);
+ if (!ucan_release_context(up, context))
+ dev_err(&up->udev->dev,
+ "%s: xmit submit err: failed to release context\n",
+ __func__);
+
+ /* remove the skb from the echo stack - this also
+ * frees the skb
+ */
+ can_free_echo_skb(up->netdev, echo_index);
+
+ if (ret == -ENODEV) {
+ netif_device_detach(up->netdev);
+ } else {
+ netdev_warn(up->netdev,
+ "%s: failed tx_urb %d\n", __func__, ret);
+ up->netdev->stats.tx_dropped++;
+ }
+ return NETDEV_TX_OK;
+ }
+
+ netif_trans_update(netdev);
+
+ /* release ref, as we do not need the urb anymore */
+ usb_free_urb(urb);
+
+ return NETDEV_TX_OK;
+
+drop:
+ if (!ucan_release_context(up, context))
+ dev_err(&up->udev->dev,
+ "%s: xmit drop: failed to release context\n", __func__);
+ dev_kfree_skb(skb);
+ up->netdev->stats.tx_dropped++;
+
+ return NETDEV_TX_OK;
+}
+
+/* Device goes down
+ *
+ * Clean up used resources
+ */
+static int ucan_close(struct net_device *netdev)
+{
+ int ret;
+ struct ucan_priv *up = netdev_priv(netdev);
+
+ up->can.state = CAN_STATE_STOPPED;
+
+ /* stop sending data */
+ usb_kill_anchored_urbs(&up->tx_urbs);
+
+ /* stop receiving data */
+ usb_kill_anchored_urbs(&up->rx_urbs);
+
+ /* stop and reset can device */
+ ret = ucan_ctrl_command_out(up, UCAN_COMMAND_STOP, 0, 0);
+ if (ret < 0)
+ dev_err(&up->udev->dev,
+ "%s: could not stop UCAN device, code: %d\n",
+ __func__, ret);
+
+ ret = ucan_ctrl_command_out(up, UCAN_COMMAND_RESET, 0, 0);
+ if (ret < 0)
+ dev_err(&up->udev->dev,
+ "%s: could not reset UCAN device, code: %d\n",
+ __func__, ret);
+
+ netif_stop_queue(netdev);
+
+ ucan_release_context_array(up);
+
+ close_candev(up->netdev);
+ return 0;
+}
+
+/* CAN driver callbacks */
+static const struct net_device_ops ucan_netdev_ops = {
+ .ndo_open = ucan_open,
+ .ndo_stop = ucan_close,
+ .ndo_start_xmit = ucan_start_xmit,
+ .ndo_change_mtu = can_change_mtu,
+};
+
+/* Request to set bittiming
+ *
+ * This function generates an USB set bittiming message and transmits
+ * it to the device
+ */
+static int ucan_set_bittiming(struct net_device *netdev)
+{
+ int ret;
+ struct ucan_priv *up = netdev_priv(netdev);
+ struct ucan_ctl_cmd_set_bittiming *cmd_set_bittiming;
+
+ cmd_set_bittiming = &up->ctl_msg_buffer->cmd_set_bittiming;
+ cmd_set_bittiming->tq = cpu_to_le32(up->can.bittiming.tq);
+ cmd_set_bittiming->brp = cpu_to_le16(up->can.bittiming.brp);
+ cmd_set_bittiming->sample_point =
+ cpu_to_le32(up->can.bittiming.sample_point);
+ cmd_set_bittiming->prop_seg = up->can.bittiming.prop_seg;
+ cmd_set_bittiming->phase_seg1 = up->can.bittiming.phase_seg1;
+ cmd_set_bittiming->phase_seg2 = up->can.bittiming.phase_seg2;
+ cmd_set_bittiming->sjw = up->can.bittiming.sjw;
+
+ dev_dbg(&up->udev->dev,
+ "Setup bittiming\n"
+ " bitrate: %d\n"
+ " sample-point: %d\n"
+ " tq: %d\n"
+ " prop_seg: %d\n"
+ " phase_seg1 %d\n"
+ " phase_seg2 %d\n"
+ " sjw %d\n"
+ " brp %d\n",
+ up->can.bittiming.bitrate, up->can.bittiming.sample_point,
+ up->can.bittiming.tq, up->can.bittiming.prop_seg,
+ up->can.bittiming.phase_seg1, up->can.bittiming.phase_seg2,
+ up->can.bittiming.sjw, up->can.bittiming.brp);
+
+ ret = ucan_ctrl_command_out(up, UCAN_COMMAND_SET_BITTIMING, 0,
+ sizeof(*cmd_set_bittiming));
+ return (ret < 0) ? ret : 0;
+}
+
+/* Restart the device to get it out of BUS-OFF state.
+ * Called when the user runs "ip link set can1 type can restart".
+ */
+static int ucan_set_mode(struct net_device *netdev, enum can_mode mode)
+{
+ int ret;
+ unsigned long flags;
+ struct ucan_priv *up = netdev_priv(netdev);
+
+ switch (mode) {
+ case CAN_MODE_START:
+ dev_dbg(&up->udev->dev,
+ "%s: Restarting device\n",
+ __func__);
+ ret = ucan_ctrl_command_out(up, UCAN_COMMAND_RESTART, 0, 0);
+ up->can.state = CAN_STATE_ERROR_ACTIVE;
+
+ /* check if queue can be restarted,
+ * up->available_tx_urbs must be protected by the
+ * lock
+ */
+ spin_lock_irqsave(&up->context_lock, flags);
+
+ if (up->available_tx_urbs > 0)
+ netif_wake_queue(up->netdev);
+
+ spin_unlock_irqrestore(&up->context_lock, flags);
+
+ return ret;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+/* Probe the device, reset it and gather general device information */
+static int ucan_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+ int ret;
+ int i;
+ u32 protocol_version;
+ struct usb_device *udev;
+ struct net_device *netdev;
+ struct usb_host_interface *iface_desc;
+ struct ucan_priv *up;
+ struct usb_endpoint_descriptor *ep;
+ struct usb_endpoint_descriptor *out_ep;
+ struct usb_endpoint_descriptor *in_ep;
+ union ucan_ctl_payload *ctl_msg_buffer;
+ char firmware_str[sizeof(union ucan_ctl_payload) + 1];
+
+ udev = interface_to_usbdev(intf);
+
+ /* Stage 1 - Interface Parsing
+ * ---------------------------
+ *
+ * Identifie the device USB interface descriptor and its
+ * endpoints. Probing is aborted on errors.
+ */
+
+ /* check if the interface is sane */
+ ret = -ENODEV;
+ iface_desc = intf->cur_altsetting;
+ if (!iface_desc)
+ goto err;
+
+ /* interface sanity check */
+ if (iface_desc->desc.bNumEndpoints != 2) {
+ dev_err(&udev->dev,
+ "%s: incompatible (possibly old) interface. It must have 2 endpoints but has %d\n",
+ __func__, iface_desc->desc.bNumEndpoints);
+ goto err;
+ }
+
+ dev_info(&udev->dev, "%s: found UCAN device on interface #%d\n",
+ __func__, iface_desc->desc.iInterface);
+
+ /* check interface endpoints */
+ in_ep = NULL;
+ out_ep = NULL;
+ for (i = 0; i < iface_desc->desc.bNumEndpoints; i++) {
+ ep = &iface_desc->endpoint[i].desc;
+
+ if (((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK) != 0) &&
+ ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
+ USB_ENDPOINT_XFER_BULK)) {
+ /* In Endpoint */
+ in_ep = ep;
+ } else if (((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK) ==
+ 0) &&
+ ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
+ USB_ENDPOINT_XFER_BULK)) {
+ /* Out Endpoint */
+ out_ep = ep;
+ }
+ }
+
+ /* check if interface is sane */
+ if (!in_ep || !out_ep) {
+ dev_err(&udev->dev, "%s: invalid endpoint configuration\n",
+ __func__);
+ goto err;
+ }
+ if (in_ep->wMaxPacketSize < sizeof(struct ucan_message_in)) {
+ dev_err(&udev->dev, "%s: invalid in_ep MaxPacketSize\n",
+ __func__);
+ goto err;
+ }
+ if (out_ep->wMaxPacketSize < sizeof(struct ucan_message_out)) {
+ dev_err(&udev->dev, "%s: invalid out_ep MaxPacketSize\n",
+ __func__);
+ goto err;
+ }
+
+ /* Stage 2 - Device Identification
+ * -------------------------------
+ *
+ * The device interface seems to be a ucan device. Do further
+ * compatibility checks. On error probing is aborted, on
+ * success this stage leaves the ctl_msg_buffer with the
+ * reported contents of a GET_INFO command (supported
+ * bittimings, tx_fifo depth). This information is used in
+ * Stage 3 for the final driver initialisation.
+ */
+
+ /* Prepare Memory for control transferes */
+ ctl_msg_buffer = devm_kzalloc(&udev->dev,
+ sizeof(union ucan_ctl_payload),
+ GFP_KERNEL);
+ if (!ctl_msg_buffer)
+ goto err;
+
+ /* get protocol version
+ *
+ * note: ucan_ctrl_command_* wrappers connot be used yet
+ * because `up` is initialised in Stage 3
+ */
+ ret = usb_control_msg(udev,
+ usb_rcvctrlpipe(udev, 0),
+ UCAN_COMMAND_GET,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_INTERFACE,
+ cpu_to_le16(UCAN_COMMAND_GET_PROTOCOL_VERSION),
+ iface_desc->desc.bInterfaceNumber,
+ ctl_msg_buffer,
+ sizeof(union ucan_ctl_payload),
+ UCAN_USB_CTL_PIPE_TIMEOUT);
+
+ /* older firmware version do not support this command - those
+ * are not supported by this drive
+ */
+ if (ret != 4) {
+ dev_err(&udev->dev,
+ "%s: could not read protocol version, ret=%d. The firmware on this device is too old, please update!\n",
+ __func__, ret);
+ if (ret >= 0)
+ ret = -EINVAL;
+ goto err;
+ }
+
+ /* this driver currently supports protocol version 3 only */
+ protocol_version =
+ le32_to_cpu(ctl_msg_buffer->cmd_get_protocol_version.version);
+ if (protocol_version < UCAN_PROTOCOL_VERSION_MIN ||
+ protocol_version > UCAN_PROTOCOL_VERSION_MAX) {
+ dev_err(&udev->dev, "%s: device protocol version %d is not supported\n",
+ __func__, protocol_version);
+ ret = -EINVAL;
+ goto err;
+ }
+ dev_info(&udev->dev, "%s: device protocol version %d ok\n", __func__,
+ protocol_version);
+
+ /* request the device information and store it in ctl_msg_buffer
+ *
+ * note: ucan_ctrl_command_* wrappers connot be used yet
+ * because `up` is initialised in Stage 3
+ */
+ ret = usb_control_msg(udev,
+ usb_rcvctrlpipe(udev, 0),
+ UCAN_COMMAND_GET,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_INTERFACE,
+ cpu_to_le16(UCAN_COMMAND_GET_INFO),
+ iface_desc->desc.bInterfaceNumber,
+ ctl_msg_buffer,
+ sizeof(ctl_msg_buffer->cmd_get_device_info),
+ UCAN_USB_CTL_PIPE_TIMEOUT);
+
+ if (ret < 0) {
+ dev_err(&udev->dev, "%s: failed to retrieve device info\n",
+ __func__);
+ goto err;
+ }
+ if (ret < sizeof(ctl_msg_buffer->cmd_get_device_info)) {
+ dev_err(&udev->dev, "%s: device reported invalid device info\n",
+ __func__);
+ ret = -EINVAL;
+ goto err;
+ }
+ if (ctl_msg_buffer->cmd_get_device_info.tx_fifo == 0) {
+ dev_err(&udev->dev, "%s: device reported invalid tx-fifo size\n",
+ __func__);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ /* Stage 3 - Driver Initialisation
+ * -------------------------------
+ *
+ * Register device to Linux, prepare private structures and
+ * reset the device.
+ */
+
+ /* allocate driver resources */
+ netdev = alloc_candev(sizeof(struct ucan_priv),
+ ctl_msg_buffer->cmd_get_device_info.tx_fifo);
+ if (!netdev) {
+ dev_err(&udev->dev, "%s: cannot allocate candev\n", __func__);
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ up = netdev_priv(netdev);
+
+ /* initialze data */
+ up->udev = udev;
+ up->intf = intf;
+ up->netdev = netdev;
+ up->intf_index = iface_desc->desc.bInterfaceNumber;
+ up->in_ep = in_ep;
+ up->out_ep = out_ep;
+ up->ctl_msg_buffer = ctl_msg_buffer;
+ up->context_array = NULL;
+ up->available_tx_urbs = 0;
+
+ up->can.state = CAN_STATE_STOPPED;
+ up->can.bittiming_const = &up->device_info.bittiming_const;
+ up->can.do_set_bittiming = ucan_set_bittiming;
+ up->can.do_set_mode = &ucan_set_mode;
+ spin_lock_init(&up->context_lock);
+ netdev->netdev_ops = &ucan_netdev_ops;
+
+ usb_set_intfdata(intf, up);
+ SET_NETDEV_DEV(netdev, &intf->dev);
+
+ /* parse device information
+ * the data retrieved in Stage 2 is still available in
+ * up->ctl_msg_buffer
+ */
+ ucan_parse_device_info(up, &ctl_msg_buffer->cmd_get_device_info);
+
+ /* just print some device information - if available */
+ ret = ucan_device_request_in(up, UCAN_DEVICE_GET_FW_STRING, 0,
+ sizeof(union ucan_ctl_payload));
+ if (ret > 0) {
+ /* copy string while ensuring zero terminiation */
+ strncpy(firmware_str, up->ctl_msg_buffer->raw,
+ sizeof(union ucan_ctl_payload));
+ firmware_str[sizeof(union ucan_ctl_payload)] = '\0';
+ } else {
+ strcpy(firmware_str, "unknown firmware");
+ }
+
+ /* device is compatible, reset it */
+ ret = ucan_ctrl_command_out(up, UCAN_COMMAND_RESET, 0, 0);
+ if (ret < 0)
+ goto err_free_candev;
+
+ init_usb_anchor(&up->rx_urbs);
+ init_usb_anchor(&up->tx_urbs);
+
+ up->can.state = CAN_STATE_STOPPED;
+
+ /* register the device */
+ ret = register_candev(netdev);
+ if (ret)
+ goto err_free_candev;
+
+ /* initialisation complete, log device info */
+ dev_info(&up->udev->dev,
+ "%s: device reports:\n"
+ " Clock frequency [Hz] : %d\n"
+ " TX FIFO length : %d\n"
+ " Time segment 1 [min-max] : %d-%d\n"
+ " Time segment 2 [min-max] : %d-%d\n"
+ " SWJ [max] : %d\n"
+ " Prescaler [min-max,step] : %d-%d,%d\n"
+ " Supported modes : %s%s%s%s%s = 0x%x\n"
+ " Firmware : %s\n",
+ __func__,
+ up->can.clock.freq, up->device_info.tx_fifo,
+ up->can.bittiming_const->tseg1_min,
+ up->can.bittiming_const->tseg1_max,
+ up->can.bittiming_const->tseg2_min,
+ up->can.bittiming_const->tseg2_max,
+ up->can.bittiming_const->sjw_max,
+ up->can.bittiming_const->brp_min,
+ up->can.bittiming_const->brp_max,
+ up->can.bittiming_const->brp_inc,
+ (up->can.ctrlmode_supported & CAN_CTRLMODE_LOOPBACK) ?
+ "LOOPBACK" : "",
+ (up->can.ctrlmode_supported & CAN_CTRLMODE_LISTENONLY) ?
+ " | LISTENONLY" : "",
+ (up->can.ctrlmode_supported & CAN_CTRLMODE_3_SAMPLES) ?
+ " | 3_SAMPLES" : "",
+ (up->can.ctrlmode_supported & CAN_CTRLMODE_ONE_SHOT) ?
+ " | ONE_SHOT" : "",
+ (up->can.ctrlmode_supported & CAN_CTRLMODE_BERR_REPORTING) ?
+ " | BERR_REPORTING" : "",
+ up->can.ctrlmode_supported,
+ firmware_str);
+
+ dev_info(&udev->dev, "%s: registered UCAN device at %s\n",
+ __func__, netdev->name);
+
+ /* success */
+ return 0;
+
+err_free_candev:
+ free_candev(netdev);
+err:
+ return ret;
+}
+
+/* disconnect the device */
+static void ucan_disconnect(struct usb_interface *intf)
+{
+ struct usb_device *udev;
+ struct ucan_priv *up = usb_get_intfdata(intf);
+
+ udev = interface_to_usbdev(intf);
+
+ usb_set_intfdata(intf, NULL);
+
+ if (up) {
+ unregister_netdev(up->netdev);
+ free_candev(up->netdev);
+ }
+}
+
+static struct usb_device_id ucan_table[] = {
+ /* Mule (soldered onto compute modules) */
+ {USB_DEVICE_INTERFACE_NUMBER(0x2294, 0x425a, 0)},
+ /* Seal (standalone USB stick) */
+ {USB_DEVICE_INTERFACE_NUMBER(0x2294, 0x425b, 0)},
+ {} /* Terminating entry */
+};
+
+MODULE_DEVICE_TABLE(usb, ucan_table);
+/* driver callbacks */
+static struct usb_driver ucan_driver = {
+ .name = "ucan",
+ .probe = ucan_probe,
+ .disconnect = ucan_disconnect,
+ .id_table = ucan_table,
+};
+
+module_usb_driver(ucan_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Martin Elshuber <martin.elshuber@theobroma-systems.com>");
+MODULE_AUTHOR("Jakob Unterwurzacher <jakob.unterwurzacher@theobroma-systems.com>");
+MODULE_DESCRIPTION("Driver for Theobroma Systems UCAN devices");
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-22 13:53 [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices Jakob Unterwurzacher
2018-03-22 13:53 ` [PATCH v3 1/1] " Jakob Unterwurzacher
@ 2018-03-23 8:32 ` Wolfgang Grandegger
2018-03-23 9:40 ` Jakob Unterwurzacher
1 sibling, 1 reply; 11+ messages in thread
From: Wolfgang Grandegger @ 2018-03-23 8:32 UTC (permalink / raw)
To: Jakob Unterwurzacher
Cc: Martin Elshuber, Philipp Tomsich, Marc Kleine-Budde, linux-can,
linux-kernel
Hello Jacob,
Am 22.03.2018 um 14:53 schrieb Jakob Unterwurzacher:
> This is v3 of the Theobroma Systems CAN/USB adapter driver
> upstreaming effort.
>
> Featured v2 -> v3 changes:
> * count error frames as data packets
> * use canid_t for all can ids
> * use BIT(x) instead of (1 << x)
> * use __le16 / __le32 for little-endian fields
> * add spinlock around context allocation (fixes a possible race)
> * fix comment style
> * use WARN_ON return value
> * fix state logic bug that did not allow return to ERROR_ACTIVE
> * drop echo_index from context_array (not needed)
> * rename "tx_contexts" -> "context_array" to prevent confusion
> * add __func__ to all errors and warnings, and to info where it made sense
The final output messages in the driver should especially be useful for
the end user... and not the developer! This is also true for the
function names. You already use more "__func__" than all other CAN
drivers together. Just my opinion!
>
> Jakob Unterwurzacher (1):
> can: ucan: add driver for Theobroma Systems UCAN devices
>
> Documentation/networking/can_ucan_protocol.rst | 315 +++++
> Documentation/networking/index.rst | 1 +
> drivers/net/can/usb/Kconfig | 10 +
> drivers/net/can/usb/Makefile | 1 +
> drivers/net/can/usb/ucan.c | 1628 ++++++++++++++++++++++++
> 5 files changed, 1955 insertions(+)
> create mode 100644 Documentation/networking/can_ucan_protocol.rst
> create mode 100644 drivers/net/can/usb/ucan.c
Wolfgang.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-22 13:53 ` [PATCH v3 1/1] " Jakob Unterwurzacher
@ 2018-03-23 8:42 ` Wolfgang Grandegger
2018-03-24 11:43 ` kbuild test robot
1 sibling, 0 replies; 11+ messages in thread
From: Wolfgang Grandegger @ 2018-03-23 8:42 UTC (permalink / raw)
To: Jakob Unterwurzacher
Cc: Martin Elshuber, Philipp Tomsich, Marc Kleine-Budde, linux-can,
linux-kernel
Am 22.03.2018 um 14:53 schrieb Jakob Unterwurzacher:
> The UCAN driver supports the microcontroller-based USB/CAN
> adapters from Theobroma Systems. There are two form-factors
> that run essentially the same firmware:
>
> * Seal: standalone USB stick ( https://www.theobroma-systems.com/seal )
>
> * Mule: integrated on the PCB of various System-on-Modules from
> Theobroma Systems like the A31-µQ7 and the RK3399-Q7
> ( https://www.theobroma-systems.com/rk3399-q7 )
>
> The USB wire protocol has been designed to be as generic and
> hardware-indendent as possible in the hope of being useful for
> implementation on other microcontrollers.
>
> Signed-off-by: Martin Elshuber <martin.elshuber@theobroma-systems.com>
> Signed-off-by: Jakob Unterwurzacher <jakob.unterwurzacher@theobroma-systems.com>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> Documentation/networking/can_ucan_protocol.rst | 315 +++++
> Documentation/networking/index.rst | 1 +
> drivers/net/can/usb/Kconfig | 10 +
> drivers/net/can/usb/Makefile | 1 +
> drivers/net/can/usb/ucan.c | 1628 ++++++++++++++++++++++++
> 5 files changed, 1955 insertions(+)
> create mode 100644 Documentation/networking/can_ucan_protocol.rst
> create mode 100644 drivers/net/can/usb/ucan.c
>
[...snip...]
> diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
> new file mode 100644
> index 000000000000..5a98ef5d95da
> --- /dev/null
> +++ b/drivers/net/can/usb/ucan.c
[...snip...]
> +static void ucan_rx_can_msg(struct ucan_priv *up, struct ucan_message_in *m)
> +{
> + int len;
> + canid_t canid, canid_mask;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct net_device_stats *stats = &up->netdev->stats;
> +
> + /* get the contents of the length field */
> + len = le16_to_cpu(m->len);
> +
> + /* check sanity */
> + if (len < UCAN_IN_HDR_SIZE + sizeof(m->msg.can_msg.id)) {
> + dev_warn(&up->udev->dev,
> + "%s: invalid input message len: %d\n", __func__, len);
> + return;
> + }
> +
> + /* handle error frames */
> + canid = le32_to_cpu(m->msg.can_msg.id);
> + if (canid & CAN_ERR_FLAG) {
> + bool busstate_changed = ucan_handle_error_frame(up, m, canid);
> + /* if berr-reporting is off only state changes get through */
> + if (!(up->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> + !busstate_changed)
> + return;
> + }
> +
> + /* allocate skb */
> + skb = alloc_can_skb(up->netdev, &cf);
> + if (!skb)
> + return;
> +
> + /* compute the mask for canid */
> + canid_mask = CAN_RTR_FLAG | CAN_ERR_FLAG;
> + if (canid & CAN_EFF_FLAG)
> + canid_mask |= CAN_EFF_MASK | CAN_EFF_FLAG;
> + else
> + canid_mask |= CAN_SFF_MASK;
You do not want that for error frames... or have I missed something?
> + /* fill the can frame */
> + cf->can_id = canid & canid_mask;
> +
> + /* compute DLC taking RTR_FLAG into account */
> + cf->can_dlc = ucan_get_can_dlc(&m->msg.can_msg, len);
> +
> + /* copy the payload of non RTR frames */
> + if (!(cf->can_id & CAN_RTR_FLAG))
> + memcpy(cf->data, m->msg.can_msg.data, cf->can_dlc);
Ditto.
> + /* don't count error frames as real packets */
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +
> + /* pass it to Linux */
> + netif_rx(skb);
> +}
> +
> +/* callback indicating completed transmission */
> +static void ucan_tx_complete_msg(struct ucan_priv *up,
> + struct ucan_message_in *m)
> +{
> + u16 count, i;
> + u8 echo_index, dlc;
> + u16 len = le16_to_cpu(m->len);
> +
> + struct ucan_urb_context *context;
> +
> + if (len < UCAN_IN_HDR_SIZE || (len % 2 != 0)) {
> + dev_err(&up->udev->dev,
> + "%s: invalid tx complete length\n",
> + __func__);
> + return;
> + }
> +
> + count = (len - UCAN_IN_HDR_SIZE) / 2;
> + for (i = 0; i < count; i++) {
> + /* we did not submit such echo ids */
> + echo_index = m->msg.can_tx_complete_msg[i].echo_index;
> + if (echo_index >= up->device_info.tx_fifo) {
> + up->netdev->stats.tx_errors++;
> + dev_err(&up->udev->dev,
> + "%s: device answered with invalid echo_index\n",
> + __func__);
> + continue;
> + }
> +
> + /* gather information from the context */
> + context = &up->context_array[echo_index];
> + dlc = READ_ONCE(context->dlc);
> +
> + /* Release context and restart queue if necessary.
> + * Also check if the context was allocated
> + */
> + if (ucan_release_context(up, context))
> + continue;
> +
> + if (m->msg.can_tx_complete_msg[i].flags &
> + UCAN_TX_COMPLETE_SUCCESS) {
> + /* update statistics */
> + up->netdev->stats.tx_packets++;
> + up->netdev->stats.tx_bytes += dlc;
> + can_get_echo_skb(up->netdev, echo_index);
> + } else {
> + up->netdev->stats.tx_dropped++;
> + can_free_echo_skb(up->netdev, echo_index);
> + }
> + }
> +}
> +
> +/* callback on reception of a USB message */
> +static void ucan_read_bulk_callback(struct urb *urb)
> +{
> + int ret;
> + int pos;
> + struct ucan_priv *up = urb->context;
> + struct net_device *netdev = up->netdev;
> + struct ucan_message_in *m;
> +
> + /* the device is not up and the driver should not receive any
> + * data on the bulk in pipe
> + */
> + if (WARN_ON(!up->context_array)) {
> + usb_free_coherent(up->udev,
> + up->in_ep->wMaxPacketSize,
> + urb->transfer_buffer,
> + urb->transfer_dma);
> + return;
> + }
> +
> + /* check URB status */
> + switch (urb->status) {
> + case 0:
> + break;
> + case -ENOENT:
> + case -EPIPE:
> + case -EPROTO:
> + case -ESHUTDOWN:
> + case -ETIME:
> + /* urb is not resubmitted -> free dma data */
> + usb_free_coherent(up->udev,
> + up->in_ep->wMaxPacketSize,
> + urb->transfer_buffer,
> + urb->transfer_dma);
> + dev_dbg(&up->udev->dev,
> + "%s: ENOENT|EPIPE|EPROTO|ESHUTDOWN|ETIME\n",
Some space would make that message more readable, I think!
> + __func__);
> + return;
> + default:
> + goto resubmit;
> + }
> +
> + /* sanity check */
> + if (!netif_device_present(netdev))
> + return;
> +
> + /* iterate over input */
> + pos = 0;
> + while (pos < urb->actual_length) {
> + int len;
> +
> + /* check sanity (length of header) */
> + if ((urb->actual_length - pos) < UCAN_IN_HDR_SIZE) {
> + dev_warn(&up->udev->dev,
> + "%s: invalid input message %d; too short (no header)\n",
> + __func__,
> + urb->actual_length);
> + goto resubmit;
> + }
> +
> + /* setup the message address */
> + m = (struct ucan_message_in *)
> + ((u8 *)urb->transfer_buffer + pos);
> + len = le16_to_cpu(m->len);
> +
> + /* check sanity (length of content) */
> + if (urb->actual_length - pos < len) {
> + dev_warn(&up->udev->dev,
> + "%s: invalid input message al:%d pos:%d len:%d; too short (no data)\n",
> + __func__,
> + urb->actual_length, pos, len);
> + print_hex_dump(KERN_WARNING,
> + "raw data: ",
> + DUMP_PREFIX_ADDRESS,
> + 16,
> + 1,
> + urb->transfer_buffer,
> + urb->actual_length,
> + true);
> +
> + goto resubmit;
> + }
> +
> + switch (le16_to_cpu(m->type)) {
> + case UCAN_IN_RX:
> + ucan_rx_can_msg(up, m);
> + break;
> + case UCAN_IN_TX_COMPLETE:
> + ucan_tx_complete_msg(up, m);
> + break;
> + default:
> + dev_warn(&up->udev->dev,
> + "%s: invalid input message type\n",
> + __func__);
> + break;
> + }
> +
> + /* proceed to next message */
> + pos += len;
> + /* align to 4 byte boundary */
> + pos = round_up(pos, 4);
> + }
> +
> +resubmit:
> + /* resubmit urb when done */
> + usb_fill_bulk_urb(urb, up->udev,
> + usb_rcvbulkpipe(up->udev,
> + up->in_ep->bEndpointAddress &
> + USB_ENDPOINT_NUMBER_MASK),
> + urb->transfer_buffer,
> + up->in_ep->wMaxPacketSize,
> + ucan_read_bulk_callback,
> + up);
> +
> + usb_anchor_urb(urb, &up->rx_urbs);
> + ret = usb_submit_urb(urb, GFP_KERNEL);
> +
> + if (ret < 0) {
> + dev_err(&up->udev->dev,
> + "%s: failed resubmitting read bulk urb: %d\n",
> + __func__,
> + ret);
> +
> + usb_unanchor_urb(urb);
> + usb_free_coherent(up->udev, up->in_ep->wMaxPacketSize,
> + urb->transfer_buffer,
> + urb->transfer_dma);
> +
> + if (ret == -ENODEV)
> + netif_device_detach(netdev);
> + }
> +}
> +static int ucan_set_bittiming(struct net_device *netdev)
> +{
> + int ret;
> + struct ucan_priv *up = netdev_priv(netdev);
> + struct ucan_ctl_cmd_set_bittiming *cmd_set_bittiming;
> +
> + cmd_set_bittiming = &up->ctl_msg_buffer->cmd_set_bittiming;
> + cmd_set_bittiming->tq = cpu_to_le32(up->can.bittiming.tq);
> + cmd_set_bittiming->brp = cpu_to_le16(up->can.bittiming.brp);
> + cmd_set_bittiming->sample_point =
> + cpu_to_le32(up->can.bittiming.sample_point);
> + cmd_set_bittiming->prop_seg = up->can.bittiming.prop_seg;
> + cmd_set_bittiming->phase_seg1 = up->can.bittiming.phase_seg1;
> + cmd_set_bittiming->phase_seg2 = up->can.bittiming.phase_seg2;
> + cmd_set_bittiming->sjw = up->can.bittiming.sjw;
> +
> + dev_dbg(&up->udev->dev,
> + "Setup bittiming\n"
> + " bitrate: %d\n"
> + " sample-point: %d\n"
> + " tq: %d\n"
> + " prop_seg: %d\n"
> + " phase_seg1 %d\n"
> + " phase_seg2 %d\n"
> + " sjw %d\n"
> + " brp %d\n",
> + up->can.bittiming.bitrate, up->can.bittiming.sample_point,
> + up->can.bittiming.tq, up->can.bittiming.prop_seg,
> + up->can.bittiming.phase_seg1, up->can.bittiming.phase_seg2,
> + up->can.bittiming.sjw, up->can.bittiming.brp);
Do we need that! Is it useful for the end user? The information is also
available via "ip -d -s link show canX".
> +
> + ret = ucan_ctrl_command_out(up, UCAN_COMMAND_SET_BITTIMING, 0,
> + sizeof(*cmd_set_bittiming));
> + return (ret < 0) ? ret : 0;
> +}
> +
> +/* Restart the device to get it out of BUS-OFF state.
> + * Called when the user runs "ip link set can1 type can restart".
> + */
> +static int ucan_set_mode(struct net_device *netdev, enum can_mode mode)
> +{
> + int ret;
> + unsigned long flags;
> + struct ucan_priv *up = netdev_priv(netdev);
> +
> + switch (mode) {
> + case CAN_MODE_START:
> + dev_dbg(&up->udev->dev,
> + "%s: Restarting device\n",
> + __func__);
> + ret = ucan_ctrl_command_out(up, UCAN_COMMAND_RESTART, 0, 0);
> + up->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + /* check if queue can be restarted,
> + * up->available_tx_urbs must be protected by the
> + * lock
> + */
> + spin_lock_irqsave(&up->context_lock, flags);
> +
> + if (up->available_tx_urbs > 0)
> + netif_wake_queue(up->netdev);
> +
> + spin_unlock_irqrestore(&up->context_lock, flags);
> +
> + return ret;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +/* Probe the device, reset it and gather general device information */
> +static int ucan_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + int ret;
> + int i;
> + u32 protocol_version;
> + struct usb_device *udev;
> + struct net_device *netdev;
> + struct usb_host_interface *iface_desc;
> + struct ucan_priv *up;
> + struct usb_endpoint_descriptor *ep;
> + struct usb_endpoint_descriptor *out_ep;
> + struct usb_endpoint_descriptor *in_ep;
> + union ucan_ctl_payload *ctl_msg_buffer;
> + char firmware_str[sizeof(union ucan_ctl_payload) + 1];
> +
> + udev = interface_to_usbdev(intf);
> +
> + /* Stage 1 - Interface Parsing
> + * ---------------------------
> + *
> + * Identifie the device USB interface descriptor and its
> + * endpoints. Probing is aborted on errors.
> + */
> +
> + /* check if the interface is sane */
> + ret = -ENODEV;
> + iface_desc = intf->cur_altsetting;
> + if (!iface_desc)
> + goto err;
> +
> + /* interface sanity check */
> + if (iface_desc->desc.bNumEndpoints != 2) {
> + dev_err(&udev->dev,
> + "%s: incompatible (possibly old) interface. It must have 2 endpoints but has %d\n",
> + __func__, iface_desc->desc.bNumEndpoints);
> + goto err;
> + }
> +
> + dev_info(&udev->dev, "%s: found UCAN device on interface #%d\n",
> + __func__, iface_desc->desc.iInterface);
> +
> + /* check interface endpoints */
> + in_ep = NULL;
> + out_ep = NULL;
> + for (i = 0; i < iface_desc->desc.bNumEndpoints; i++) {
> + ep = &iface_desc->endpoint[i].desc;
> +
> + if (((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK) != 0) &&
> + ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
> + USB_ENDPOINT_XFER_BULK)) {
> + /* In Endpoint */
> + in_ep = ep;
> + } else if (((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK) ==
> + 0) &&
> + ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
> + USB_ENDPOINT_XFER_BULK)) {
> + /* Out Endpoint */
> + out_ep = ep;
> + }
> + }
> +
> + /* check if interface is sane */
> + if (!in_ep || !out_ep) {
> + dev_err(&udev->dev, "%s: invalid endpoint configuration\n",
> + __func__);
> + goto err;
> + }
> + if (in_ep->wMaxPacketSize < sizeof(struct ucan_message_in)) {
> + dev_err(&udev->dev, "%s: invalid in_ep MaxPacketSize\n",
> + __func__);
> + goto err;
> + }
> + if (out_ep->wMaxPacketSize < sizeof(struct ucan_message_out)) {
> + dev_err(&udev->dev, "%s: invalid out_ep MaxPacketSize\n",
> + __func__);
> + goto err;
> + }
> +
> + /* Stage 2 - Device Identification
> + * -------------------------------
> + *
> + * The device interface seems to be a ucan device. Do further
> + * compatibility checks. On error probing is aborted, on
> + * success this stage leaves the ctl_msg_buffer with the
> + * reported contents of a GET_INFO command (supported
> + * bittimings, tx_fifo depth). This information is used in
> + * Stage 3 for the final driver initialisation.
> + */
> +
> + /* Prepare Memory for control transferes */
> + ctl_msg_buffer = devm_kzalloc(&udev->dev,
> + sizeof(union ucan_ctl_payload),
> + GFP_KERNEL);
> + if (!ctl_msg_buffer)
> + goto err;
> +
> + /* get protocol version
> + *
> + * note: ucan_ctrl_command_* wrappers connot be used yet
> + * because `up` is initialised in Stage 3
> + */
> + ret = usb_control_msg(udev,
> + usb_rcvctrlpipe(udev, 0),
> + UCAN_COMMAND_GET,
> + USB_DIR_IN | USB_TYPE_VENDOR |
> + USB_RECIP_INTERFACE,
> + cpu_to_le16(UCAN_COMMAND_GET_PROTOCOL_VERSION),
> + iface_desc->desc.bInterfaceNumber,
> + ctl_msg_buffer,
> + sizeof(union ucan_ctl_payload),
> + UCAN_USB_CTL_PIPE_TIMEOUT);
> +
> + /* older firmware version do not support this command - those
> + * are not supported by this drive
> + */
> + if (ret != 4) {
> + dev_err(&udev->dev,
> + "%s: could not read protocol version, ret=%d. The firmware on this device is too old, please update!\n",
> + __func__, ret);
> + if (ret >= 0)
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + /* this driver currently supports protocol version 3 only */
> + protocol_version =
> + le32_to_cpu(ctl_msg_buffer->cmd_get_protocol_version.version);
> + if (protocol_version < UCAN_PROTOCOL_VERSION_MIN ||
> + protocol_version > UCAN_PROTOCOL_VERSION_MAX) {
> + dev_err(&udev->dev, "%s: device protocol version %d is not supported\n",
> + __func__, protocol_version);
> + ret = -EINVAL;
> + goto err;
> + }
> + dev_info(&udev->dev, "%s: device protocol version %d ok\n", __func__,
> + protocol_version);
> +
> + /* request the device information and store it in ctl_msg_buffer
> + *
> + * note: ucan_ctrl_command_* wrappers connot be used yet
> + * because `up` is initialised in Stage 3
> + */
> + ret = usb_control_msg(udev,
> + usb_rcvctrlpipe(udev, 0),
> + UCAN_COMMAND_GET,
> + USB_DIR_IN | USB_TYPE_VENDOR |
> + USB_RECIP_INTERFACE,
> + cpu_to_le16(UCAN_COMMAND_GET_INFO),
> + iface_desc->desc.bInterfaceNumber,
> + ctl_msg_buffer,
> + sizeof(ctl_msg_buffer->cmd_get_device_info),
> + UCAN_USB_CTL_PIPE_TIMEOUT);
> +
> + if (ret < 0) {
> + dev_err(&udev->dev, "%s: failed to retrieve device info\n",
> + __func__);
> + goto err;
> + }
> + if (ret < sizeof(ctl_msg_buffer->cmd_get_device_info)) {
> + dev_err(&udev->dev, "%s: device reported invalid device info\n",
> + __func__);
> + ret = -EINVAL;
> + goto err;
> + }
> + if (ctl_msg_buffer->cmd_get_device_info.tx_fifo == 0) {
> + dev_err(&udev->dev, "%s: device reported invalid tx-fifo size\n",
> + __func__);
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + /* Stage 3 - Driver Initialisation
> + * -------------------------------
> + *
> + * Register device to Linux, prepare private structures and
> + * reset the device.
> + */
> +
> + /* allocate driver resources */
> + netdev = alloc_candev(sizeof(struct ucan_priv),
> + ctl_msg_buffer->cmd_get_device_info.tx_fifo);
> + if (!netdev) {
> + dev_err(&udev->dev, "%s: cannot allocate candev\n", __func__);
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + up = netdev_priv(netdev);
> +
> + /* initialze data */
> + up->udev = udev;
> + up->intf = intf;
> + up->netdev = netdev;
> + up->intf_index = iface_desc->desc.bInterfaceNumber;
> + up->in_ep = in_ep;
> + up->out_ep = out_ep;
> + up->ctl_msg_buffer = ctl_msg_buffer;
> + up->context_array = NULL;
> + up->available_tx_urbs = 0;
> +
> + up->can.state = CAN_STATE_STOPPED;
> + up->can.bittiming_const = &up->device_info.bittiming_const;
> + up->can.do_set_bittiming = ucan_set_bittiming;
> + up->can.do_set_mode = &ucan_set_mode;
> + spin_lock_init(&up->context_lock);
> + netdev->netdev_ops = &ucan_netdev_ops;
> +
> + usb_set_intfdata(intf, up);
> + SET_NETDEV_DEV(netdev, &intf->dev);
> +
> + /* parse device information
> + * the data retrieved in Stage 2 is still available in
> + * up->ctl_msg_buffer
> + */
> + ucan_parse_device_info(up, &ctl_msg_buffer->cmd_get_device_info);
> +
> + /* just print some device information - if available */
> + ret = ucan_device_request_in(up, UCAN_DEVICE_GET_FW_STRING, 0,
> + sizeof(union ucan_ctl_payload));
> + if (ret > 0) {
> + /* copy string while ensuring zero terminiation */
> + strncpy(firmware_str, up->ctl_msg_buffer->raw,
> + sizeof(union ucan_ctl_payload));
> + firmware_str[sizeof(union ucan_ctl_payload)] = '\0';
> + } else {
> + strcpy(firmware_str, "unknown firmware");
> + }
> +
> + /* device is compatible, reset it */
> + ret = ucan_ctrl_command_out(up, UCAN_COMMAND_RESET, 0, 0);
> + if (ret < 0)
> + goto err_free_candev;
> +
> + init_usb_anchor(&up->rx_urbs);
> + init_usb_anchor(&up->tx_urbs);
> +
> + up->can.state = CAN_STATE_STOPPED;
> +
> + /* register the device */
> + ret = register_candev(netdev);
> + if (ret)
> + goto err_free_candev;
> +
> + /* initialisation complete, log device info */
> + dev_info(&up->udev->dev,
> + "%s: device reports:\n"
> + " Clock frequency [Hz] : %d\n"
> + " TX FIFO length : %d\n"
> + " Time segment 1 [min-max] : %d-%d\n"
> + " Time segment 2 [min-max] : %d-%d\n"
> + " SWJ [max] : %d\n"
> + " Prescaler [min-max,step] : %d-%d,%d\n"
> + " Supported modes : %s%s%s%s%s = 0x%x\n"
> + " Firmware : %s\n",
> + __func__,
> + up->can.clock.freq, up->device_info.tx_fifo,
> + up->can.bittiming_const->tseg1_min,
> + up->can.bittiming_const->tseg1_max,
> + up->can.bittiming_const->tseg2_min,
> + up->can.bittiming_const->tseg2_max,
> + up->can.bittiming_const->sjw_max,
> + up->can.bittiming_const->brp_min,
> + up->can.bittiming_const->brp_max,
> + up->can.bittiming_const->brp_inc,
> + (up->can.ctrlmode_supported & CAN_CTRLMODE_LOOPBACK) ?
> + "LOOPBACK" : "",
> + (up->can.ctrlmode_supported & CAN_CTRLMODE_LISTENONLY) ?
> + " | LISTENONLY" : "",
> + (up->can.ctrlmode_supported & CAN_CTRLMODE_3_SAMPLES) ?
> + " | 3_SAMPLES" : "",
> + (up->can.ctrlmode_supported & CAN_CTRLMODE_ONE_SHOT) ?
> + " | ONE_SHOT" : "",
> + (up->can.ctrlmode_supported & CAN_CTRLMODE_BERR_REPORTING) ?
> + " | BERR_REPORTING" : "",
> + up->can.ctrlmode_supported,
> + firmware_str);
Ditto!
> + dev_info(&udev->dev, "%s: registered UCAN device at %s\n",
> + __func__, netdev->name);
> +
> + /* success */
> + return 0;
> +
> +err_free_candev:
> + free_candev(netdev);
> +err:
> + return ret;
> +}
> +
> +/* disconnect the device */
> +static void ucan_disconnect(struct usb_interface *intf)
> +{
> + struct usb_device *udev;
> + struct ucan_priv *up = usb_get_intfdata(intf);
> +
> + udev = interface_to_usbdev(intf);
> +
> + usb_set_intfdata(intf, NULL);
> +
> + if (up) {
> + unregister_netdev(up->netdev);
> + free_candev(up->netdev);
> + }
> +}
> +
> +static struct usb_device_id ucan_table[] = {
> + /* Mule (soldered onto compute modules) */
> + {USB_DEVICE_INTERFACE_NUMBER(0x2294, 0x425a, 0)},
> + /* Seal (standalone USB stick) */
> + {USB_DEVICE_INTERFACE_NUMBER(0x2294, 0x425b, 0)},
> + {} /* Terminating entry */
> +};
> +
> +MODULE_DEVICE_TABLE(usb, ucan_table);
> +/* driver callbacks */
> +static struct usb_driver ucan_driver = {
> + .name = "ucan",
> + .probe = ucan_probe,
> + .disconnect = ucan_disconnect,
> + .id_table = ucan_table,
> +};
> +
> +module_usb_driver(ucan_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Martin Elshuber <martin.elshuber@theobroma-systems.com>");
> +MODULE_AUTHOR("Jakob Unterwurzacher <jakob.unterwurzacher@theobroma-systems.com>");
> +MODULE_DESCRIPTION("Driver for Theobroma Systems UCAN devices");
I will have a closer look later today.
Thanks,
Wolfgang.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-23 8:32 ` [PATCH v3 0/1] " Wolfgang Grandegger
@ 2018-03-23 9:40 ` Jakob Unterwurzacher
2018-03-23 10:04 ` Wolfgang Grandegger
0 siblings, 1 reply; 11+ messages in thread
From: Jakob Unterwurzacher @ 2018-03-23 9:40 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Martin Elshuber, Philipp Tomsich, Marc Kleine-Budde, linux-can,
linux-kernel
On 23.03.18 09:32, Wolfgang Grandegger wrote:
>> * add __func__ to all errors and warnings, and to info where it made sense
>
> The final output messages in the driver should especially be useful for
> the end user... and not the developer! This is also true for the
> function names. You already use more "__func__" than all other CAN
> drivers together. Just my opinion!
The idea was to make it clear which driver printed the message. In my
opinion, this is a problem:
> drivers/net/can/usb$ git grep "No memory left for USB buffer"
> ems_usb.c: netdev_err(netdev, "No memory left for USB buffer\n");
> ems_usb.c: netdev_err(netdev, "No memory left for USB buffer\n");
> esd_usb2.c: "No memory left for USB buffer\n");
> esd_usb2.c: netdev_err(netdev, "No memory left for USB buffer\n");
> gs_usb.c: netdev_err(netdev, "No memory left for USB buffer\n");
> gs_usb.c: "No memory left for USB buffer\n");
> kvaser_usb.c: "No memory left for USB buffer\n");
> mcba_usb.c: netdev_err(netdev, "No memory left for USB buffer\n");
> usb_8dev.c: netdev_err(netdev, "No memory left for USB buffer\n");
> usb_8dev.c: netdev_err(netdev, "No memory left for USB buffer\n");
But I'm open to other suggestions (use a fixed "ucan: " prefix?) or to
drop it entirely if you think it is not worth it.
Thanks for the feedback,
Jakob
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-23 9:40 ` Jakob Unterwurzacher
@ 2018-03-23 10:04 ` Wolfgang Grandegger
2018-03-23 10:12 ` Jakob Unterwurzacher
0 siblings, 1 reply; 11+ messages in thread
From: Wolfgang Grandegger @ 2018-03-23 10:04 UTC (permalink / raw)
To: Jakob Unterwurzacher
Cc: Martin Elshuber, Philipp Tomsich, Marc Kleine-Budde, linux-can,
linux-kernel
Am 23.03.2018 um 10:40 schrieb Jakob Unterwurzacher:
> On 23.03.18 09:32, Wolfgang Grandegger wrote:
>>> * add __func__ to all errors and warnings, and to info where it made
>>> sense
>>
>> The final output messages in the driver should especially be useful for
>> the end user... and not the developer! This is also true for the
>> function names. You already use more "__func__" than all other CAN
>> drivers together. Just my opinion!
>
> The idea was to make it clear which driver printed the message. In my
> opinion, this is a problem:
>
>> drivers/net/can/usb$ git grep "No memory left for USB buffer"
>> ems_usb.c: netdev_err(netdev, "No memory left for
>> USB buffer\n");
>> ems_usb.c: netdev_err(netdev, "No memory left for USB
>> buffer\n");
>> esd_usb2.c: "No memory left for USB
>> buffer\n");
>> esd_usb2.c: netdev_err(netdev, "No memory left for USB
>> buffer\n");
>> gs_usb.c: netdev_err(netdev, "No memory left for USB
>> buffer\n");
>> gs_usb.c: "No memory left for
>> USB buffer\n");
>> kvaser_usb.c: "No memory left for USB
>> buffer\n");
>> mcba_usb.c: netdev_err(netdev, "No memory left for
>> USB buffer\n");
>> usb_8dev.c: netdev_err(netdev, "No memory left for USB
>> buffer\n");
>> usb_8dev.c: netdev_err(netdev, "No memory left for
>> USB buffer\n");
> But I'm open to other suggestions (use a fixed "ucan: " prefix?) or to
> drop it entirely if you think it is not worth it.
But there is already a device prefix, e.g.:
peak_usb 1-6:1.0: PEAK-System PCAN-USB adapter hwrev 28 serial FFFFFFFF (1 channel)
peak_usb 1-6:1.0 can0: attached to PCAN-USB channel 0 (device 255)
^^^^^^^^
No need to add another one!
Wolfgang.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-23 10:04 ` Wolfgang Grandegger
@ 2018-03-23 10:12 ` Jakob Unterwurzacher
2018-03-27 10:19 ` Martin Elshuber
0 siblings, 1 reply; 11+ messages in thread
From: Jakob Unterwurzacher @ 2018-03-23 10:12 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Martin Elshuber, Philipp Tomsich, Marc Kleine-Budde, linux-can,
linux-kernel
On 23.03.18 11:04, Wolfgang Grandegger wrote:
>
>> But I'm open to other suggestions (use a fixed "ucan: " prefix?) or to
>> drop it entirely if you think it is not worth it.
>
> But there is already a device prefix, e.g.:
>
> peak_usb 1-6:1.0: PEAK-System PCAN-USB adapter hwrev 28 serial FFFFFFFF (1 channel)
> peak_usb 1-6:1.0 can0: attached to PCAN-USB channel 0 (device 255)
> ^^^^^^^^
Interesting. Looks like the UCAN driver is missing something to make
this work:
[ 67.792947] usb 5-1.4: ucan_probe: registered UCAN device at can0
I'll take a closer look.
Thanks,
Jakob
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-22 13:53 ` [PATCH v3 1/1] " Jakob Unterwurzacher
@ 2018-03-24 11:43 ` kbuild test robot
2018-03-24 11:43 ` kbuild test robot
1 sibling, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-03-24 11:43 UTC (permalink / raw)
Cc: kbuild-all, jakob.unterwurzacher, Martin Elshuber,
Philipp Tomsich, Wolfgang Grandegger, Marc Kleine-Budde,
linux-can, linux-kernel
Hi Jakob,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on v4.16-rc6]
[cannot apply to next-20180323]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jakob-Unterwurzacher/can-ucan-add-driver-for-Theobroma-Systems-UCAN-devices/20180324-164143
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/net/can/usb/ucan.c:301:16: sparse: restricted __le32 degrades to integer
drivers/net/can/usb/ucan.c:414:32: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
drivers/net/can/usb/ucan.c:414:32: expected unsigned short [unsigned] [usertype] value
drivers/net/can/usb/ucan.c:414:32: got restricted __le16 [usertype] <noident>
drivers/net/can/usb/ucan.c:428:32: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
drivers/net/can/usb/ucan.c:428:32: expected unsigned short [unsigned] [usertype] value
drivers/net/can/usb/ucan.c:428:32: got restricted __le16 [usertype] <noident>
drivers/net/can/usb/ucan.c:683:44: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:683:44: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:683:44: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:700:44: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:700:44: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:700:44: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:752:25: sparse: cast to restricted __le16
drivers/net/can/usb/ucan.c:779:36: sparse: incorrect type in argument 5 (different base types) @@ expected int [signed] buffer_length @@ got restricted __le1int [signed] buffer_length @@
drivers/net/can/usb/ucan.c:779:36: expected int [signed] buffer_length
drivers/net/can/usb/ucan.c:779:36: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:793:54: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:793:54: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:793:54: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:853:62: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:853:62: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:853:62: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:877:61: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:877:61: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:877:61: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:891:44: sparse: incorrect type in argument 5 (different base types) @@ expected int [signed] buffer_length @@ got restricted __le1int [signed] buffer_length @@
drivers/net/can/usb/ucan.c:891:44: expected int [signed] buffer_length
drivers/net/can/usb/ucan.c:891:44: got restricted __le16 [usertype] wMaxPacketSize
>> drivers/net/can/usb/ucan.c:1238:41: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] sample_point @@ got restricted __le16 [usertype] sample_point @@
drivers/net/can/usb/ucan.c:1238:41: expected restricted __le16 [usertype] sample_point
drivers/net/can/usb/ucan.c:1238:41: got restricted __le32 [usertype] <noident>
drivers/net/can/usb/ucan.c:1368:18: sparse: restricted __le16 degrades to integer
drivers/net/can/usb/ucan.c:1373:19: sparse: restricted __le16 degrades to integer
drivers/net/can/usb/ucan.c:1407:31: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
drivers/net/can/usb/ucan.c:1407:31: expected unsigned short [unsigned] [usertype] value
drivers/net/can/usb/ucan.c:1407:31: got restricted __le16 [usertype] <noident>
drivers/net/can/usb/ucan.c:1448:31: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
drivers/net/can/usb/ucan.c:1448:31: expected unsigned short [unsigned] [usertype] value
drivers/net/can/usb/ucan.c:1448:31: got restricted __le16 [usertype] <noident>
vim +301 drivers/net/can/usb/ucan.c
298
299 static u8 ucan_get_can_dlc(struct ucan_can_msg *msg, u16 len)
300 {
> 301 if (msg->id & CAN_RTR_FLAG)
302 return get_can_dlc(msg->dlc);
303 else
304 return get_can_dlc(len - (UCAN_IN_HDR_SIZE + sizeof(msg->id)));
305 }
306
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices
@ 2018-03-24 11:43 ` kbuild test robot
0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-03-24 11:43 UTC (permalink / raw)
To: Jakob Unterwurzacher
Cc: kbuild-all, jakob.unterwurzacher, Martin Elshuber,
Philipp Tomsich, Wolfgang Grandegger, Marc Kleine-Budde,
linux-can, linux-kernel
Hi Jakob,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on v4.16-rc6]
[cannot apply to next-20180323]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jakob-Unterwurzacher/can-ucan-add-driver-for-Theobroma-Systems-UCAN-devices/20180324-164143
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/net/can/usb/ucan.c:301:16: sparse: restricted __le32 degrades to integer
drivers/net/can/usb/ucan.c:414:32: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
drivers/net/can/usb/ucan.c:414:32: expected unsigned short [unsigned] [usertype] value
drivers/net/can/usb/ucan.c:414:32: got restricted __le16 [usertype] <noident>
drivers/net/can/usb/ucan.c:428:32: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
drivers/net/can/usb/ucan.c:428:32: expected unsigned short [unsigned] [usertype] value
drivers/net/can/usb/ucan.c:428:32: got restricted __le16 [usertype] <noident>
drivers/net/can/usb/ucan.c:683:44: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:683:44: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:683:44: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:700:44: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:700:44: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:700:44: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:752:25: sparse: cast to restricted __le16
drivers/net/can/usb/ucan.c:779:36: sparse: incorrect type in argument 5 (different base types) @@ expected int [signed] buffer_length @@ got restricted __le1int [signed] buffer_length @@
drivers/net/can/usb/ucan.c:779:36: expected int [signed] buffer_length
drivers/net/can/usb/ucan.c:779:36: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:793:54: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:793:54: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:793:54: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:853:62: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:853:62: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:853:62: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:877:61: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
drivers/net/can/usb/ucan.c:877:61: expected unsigned long [unsigned] [usertype] size
drivers/net/can/usb/ucan.c:877:61: got restricted __le16 [usertype] wMaxPacketSize
drivers/net/can/usb/ucan.c:891:44: sparse: incorrect type in argument 5 (different base types) @@ expected int [signed] buffer_length @@ got restricted __le1int [signed] buffer_length @@
drivers/net/can/usb/ucan.c:891:44: expected int [signed] buffer_length
drivers/net/can/usb/ucan.c:891:44: got restricted __le16 [usertype] wMaxPacketSize
>> drivers/net/can/usb/ucan.c:1238:41: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] sample_point @@ got restricted __le16 [usertype] sample_point @@
drivers/net/can/usb/ucan.c:1238:41: expected restricted __le16 [usertype] sample_point
drivers/net/can/usb/ucan.c:1238:41: got restricted __le32 [usertype] <noident>
drivers/net/can/usb/ucan.c:1368:18: sparse: restricted __le16 degrades to integer
drivers/net/can/usb/ucan.c:1373:19: sparse: restricted __le16 degrades to integer
drivers/net/can/usb/ucan.c:1407:31: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
drivers/net/can/usb/ucan.c:1407:31: expected unsigned short [unsigned] [usertype] value
drivers/net/can/usb/ucan.c:1407:31: got restricted __le16 [usertype] <noident>
drivers/net/can/usb/ucan.c:1448:31: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
drivers/net/can/usb/ucan.c:1448:31: expected unsigned short [unsigned] [usertype] value
drivers/net/can/usb/ucan.c:1448:31: got restricted __le16 [usertype] <noident>
vim +301 drivers/net/can/usb/ucan.c
298
299 static u8 ucan_get_can_dlc(struct ucan_can_msg *msg, u16 len)
300 {
> 301 if (msg->id & CAN_RTR_FLAG)
302 return get_can_dlc(msg->dlc);
303 else
304 return get_can_dlc(len - (UCAN_IN_HDR_SIZE + sizeof(msg->id)));
305 }
306
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-23 10:12 ` Jakob Unterwurzacher
@ 2018-03-27 10:19 ` Martin Elshuber
0 siblings, 0 replies; 11+ messages in thread
From: Martin Elshuber @ 2018-03-27 10:19 UTC (permalink / raw)
To: Jakob Unterwurzacher, Wolfgang Grandegger
Cc: Philipp Tomsich, Marc Kleine-Budde, linux-can, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1186 bytes --]
Where possible we changed dev_XXX to netdev_XXX. and removed all __func__.
netdev_XXX prints the necessary information on the device, bus and netif
The remaining dev_XXX (within probe) are extended such that they print
the string "ucan" (__func__ are also removed)
Am 23.03.18 um 11:12 schrieb Jakob Unterwurzacher:
> On 23.03.18 11:04, Wolfgang Grandegger wrote:
>>
>>> But I'm open to other suggestions (use a fixed "ucan: " prefix?) or to
>>> drop it entirely if you think it is not worth it.
>>
>> But there is already a device prefix, e.g.:
>>
>> peak_usb 1-6:1.0: PEAK-System PCAN-USB adapter hwrev 28 serial
>> FFFFFFFF (1 channel)
>> peak_usb 1-6:1.0 can0: attached to PCAN-USB channel 0 (device 255)
>> ^^^^^^^^
>
> Interesting. Looks like the UCAN driver is missing something to make
> this work:
>
> [ 67.792947] usb 5-1.4: ucan_probe: registered UCAN device at can0
>
> I'll take a closer look.
>
> Thanks,
> Jakob
--
Martin Elshuber
Theobroma Systems Design und Consulting GmbH
Seestadtstraße 27 (Aspern IQ), 1220 Wien, Austria
Phone: +43 1 236 98 93-405, Fax: +43 1 236 98 93-9
http://www.theobroma-systems.com
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 842 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices
2018-03-24 11:43 ` kbuild test robot
(?)
@ 2018-03-27 10:20 ` Martin Elshuber
-1 siblings, 0 replies; 11+ messages in thread
From: Martin Elshuber @ 2018-03-27 10:20 UTC (permalink / raw)
To: kbuild test robot, Jakob Unterwurzacher
Cc: kbuild-all, Philipp Tomsich, Wolfgang Grandegger,
Marc Kleine-Budde, linux-can, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 6441 bytes --]
fixed and rechecked again with sparse
Am 24.03.18 um 12:43 schrieb kbuild test robot:
> Hi Jakob,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on net-next/master]
> [also build test WARNING on v4.16-rc6]
> [cannot apply to next-20180323]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Jakob-Unterwurzacher/can-ucan-add-driver-for-Theobroma-Systems-UCAN-devices/20180324-164143
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
>>> drivers/net/can/usb/ucan.c:301:16: sparse: restricted __le32 degrades to integer
> drivers/net/can/usb/ucan.c:414:32: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
> drivers/net/can/usb/ucan.c:414:32: expected unsigned short [unsigned] [usertype] value
> drivers/net/can/usb/ucan.c:414:32: got restricted __le16 [usertype] <noident>
> drivers/net/can/usb/ucan.c:428:32: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
> drivers/net/can/usb/ucan.c:428:32: expected unsigned short [unsigned] [usertype] value
> drivers/net/can/usb/ucan.c:428:32: got restricted __le16 [usertype] <noident>
> drivers/net/can/usb/ucan.c:683:44: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
> drivers/net/can/usb/ucan.c:683:44: expected unsigned long [unsigned] [usertype] size
> drivers/net/can/usb/ucan.c:683:44: got restricted __le16 [usertype] wMaxPacketSize
> drivers/net/can/usb/ucan.c:700:44: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
> drivers/net/can/usb/ucan.c:700:44: expected unsigned long [unsigned] [usertype] size
> drivers/net/can/usb/ucan.c:700:44: got restricted __le16 [usertype] wMaxPacketSize
> drivers/net/can/usb/ucan.c:752:25: sparse: cast to restricted __le16
> drivers/net/can/usb/ucan.c:779:36: sparse: incorrect type in argument 5 (different base types) @@ expected int [signed] buffer_length @@ got restricted __le1int [signed] buffer_length @@
> drivers/net/can/usb/ucan.c:779:36: expected int [signed] buffer_length
> drivers/net/can/usb/ucan.c:779:36: got restricted __le16 [usertype] wMaxPacketSize
> drivers/net/can/usb/ucan.c:793:54: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
> drivers/net/can/usb/ucan.c:793:54: expected unsigned long [unsigned] [usertype] size
> drivers/net/can/usb/ucan.c:793:54: got restricted __le16 [usertype] wMaxPacketSize
> drivers/net/can/usb/ucan.c:853:62: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
> drivers/net/can/usb/ucan.c:853:62: expected unsigned long [unsigned] [usertype] size
> drivers/net/can/usb/ucan.c:853:62: got restricted __le16 [usertype] wMaxPacketSize
> drivers/net/can/usb/ucan.c:877:61: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long [unsigned] [usertype] size @@ got d long [unsigned] [usertype] size @@
> drivers/net/can/usb/ucan.c:877:61: expected unsigned long [unsigned] [usertype] size
> drivers/net/can/usb/ucan.c:877:61: got restricted __le16 [usertype] wMaxPacketSize
> drivers/net/can/usb/ucan.c:891:44: sparse: incorrect type in argument 5 (different base types) @@ expected int [signed] buffer_length @@ got restricted __le1int [signed] buffer_length @@
> drivers/net/can/usb/ucan.c:891:44: expected int [signed] buffer_length
> drivers/net/can/usb/ucan.c:891:44: got restricted __le16 [usertype] wMaxPacketSize
>>> drivers/net/can/usb/ucan.c:1238:41: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] sample_point @@ got restricted __le16 [usertype] sample_point @@
> drivers/net/can/usb/ucan.c:1238:41: expected restricted __le16 [usertype] sample_point
> drivers/net/can/usb/ucan.c:1238:41: got restricted __le32 [usertype] <noident>
> drivers/net/can/usb/ucan.c:1368:18: sparse: restricted __le16 degrades to integer
> drivers/net/can/usb/ucan.c:1373:19: sparse: restricted __le16 degrades to integer
> drivers/net/can/usb/ucan.c:1407:31: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
> drivers/net/can/usb/ucan.c:1407:31: expected unsigned short [unsigned] [usertype] value
> drivers/net/can/usb/ucan.c:1407:31: got restricted __le16 [usertype] <noident>
> drivers/net/can/usb/ucan.c:1448:31: sparse: incorrect type in argument 5 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
> drivers/net/can/usb/ucan.c:1448:31: expected unsigned short [unsigned] [usertype] value
> drivers/net/can/usb/ucan.c:1448:31: got restricted __le16 [usertype] <noident>
>
> vim +301 drivers/net/can/usb/ucan.c
>
> 298
> 299 static u8 ucan_get_can_dlc(struct ucan_can_msg *msg, u16 len)
> 300 {
> > 301 if (msg->id & CAN_RTR_FLAG)
> 302 return get_can_dlc(msg->dlc);
> 303 else
> 304 return get_can_dlc(len - (UCAN_IN_HDR_SIZE + sizeof(msg->id)));
> 305 }
> 306
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
--
Martin Elshuber
Theobroma Systems Design und Consulting GmbH
Seestadtstraße 27 (Aspern IQ), 1220 Wien, Austria
Phone: +43 1 236 98 93-405, Fax: +43 1 236 98 93-9
http://www.theobroma-systems.com
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 842 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-03-27 10:20 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 13:53 [PATCH v3 0/1] can: ucan: add driver for Theobroma Systems UCAN devices Jakob Unterwurzacher
2018-03-22 13:53 ` [PATCH v3 1/1] " Jakob Unterwurzacher
2018-03-23 8:42 ` Wolfgang Grandegger
2018-03-24 11:43 ` kbuild test robot
2018-03-24 11:43 ` kbuild test robot
2018-03-27 10:20 ` Martin Elshuber
2018-03-23 8:32 ` [PATCH v3 0/1] " Wolfgang Grandegger
2018-03-23 9:40 ` Jakob Unterwurzacher
2018-03-23 10:04 ` Wolfgang Grandegger
2018-03-23 10:12 ` Jakob Unterwurzacher
2018-03-27 10:19 ` Martin Elshuber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.