[PATCH net-next 0/15] pull-request: can-next 2022-07-03

[PATCH net-next 0/15] pull-request: can-next 2022-07-03

[Marc Kleine-Budde]

Hello Jakub, hello David,

this is a pull request of 15 patches for net-next/master.

The first 2 patches are by Max Staudt and add the can327 serial CAN
driver along with a new line discipline ID.

The next patch is by me an fixes a typo in the ctucanfd driver.

The last 12 patches are by Dario Binacchi and integrate slcan CAN
serial driver better into the existing CAN driver API.

regards,
Marc

---
The following changes since commit 0fcae3c8b1b32d79cb4bbf841023757358fb0413:

  ipmr: fix a lockdep splat in ipmr_rtm_dumplink() (2022-06-27 12:01:01 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-5.20-20220703

for you to fetch changes up to 0ebd5529d2ddab76a46681991d350b82c62ef13e:

  Merge branch 'can-slcan-extend-supported-features' (2022-07-03 11:37:05 +0200)

----------------------------------------------------------------
linux-can-next-for-5.20-20220703

----------------------------------------------------------------
Dario Binacchi (12):
      can: slcan: use the BIT() helper
      can: slcan: use netdev helpers to print out messages
      can: slcan: use the alloc_can_skb() helper
      can: netlink: dump bitrate 0 if can_priv::bittiming.bitrate is -1U
      can: slcan: use CAN network device driver API
      can: slcan: allow to send commands to the adapter
      can: slcan: set bitrate by CAN device driver API
      can: slcan: send the open/close commands to the adapter
      can: slcan: move driver into separate sub directory
      can: slcan: add ethtool support to reset adapter errors
      can: slcan: extend the protocol with error info
      can: slcan: extend the protocol with CAN state info

Marc Kleine-Budde (3):
      Merge branch 'can327-CAN-ldisc-driver-for-ELM327-based-OBD-II-adapters'
      can: ctucanfd: ctucan_interrupt(): fix typo
      Merge branch 'can-slcan-extend-supported-features'

Max Staudt (2):
      tty: Add N_CAN327 line discipline ID for ELM327 based CAN driver
      can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters

 .../networking/device_drivers/can/can327.rst       |  331 ++++++
 .../networking/device_drivers/can/index.rst        |    1 +
 MAINTAINERS                                        |    7 +
 drivers/net/can/Kconfig                            |   58 +-
 drivers/net/can/Makefile                           |    3 +-
 drivers/net/can/can327.c                           | 1137 ++++++++++++++++++++
 drivers/net/can/ctucanfd/ctucanfd_base.c           |    2 +-
 drivers/net/can/dev/netlink.c                      |    3 +-
 drivers/net/can/slcan/Makefile                     |    7 +
 drivers/net/can/{slcan.c => slcan/slcan-core.c}    |  504 +++++++--
 drivers/net/can/slcan/slcan-ethtool.c              |   65 ++
 drivers/net/can/slcan/slcan.h                      |   18 +
 include/linux/can/bittiming.h                      |    2 +
 include/uapi/linux/tty.h                           |    3 +-
 14 files changed, 2034 insertions(+), 107 deletions(-)
 create mode 100644 Documentation/networking/device_drivers/can/can327.rst
 create mode 100644 drivers/net/can/can327.c
 create mode 100644 drivers/net/can/slcan/Makefile
 rename drivers/net/can/{slcan.c => slcan/slcan-core.c} (65%)
 create mode 100644 drivers/net/can/slcan/slcan-ethtool.c
 create mode 100644 drivers/net/can/slcan/slcan.h

[PATCH net-next 01/15] tty: Add N_CAN327 line discipline ID for ELM327 based CAN driver

[Marc Kleine-Budde]

From: Max Staudt <max@enpas.org>

The actual driver will be added via the CAN tree.

Link: https://lore.kernel.org/all/20220618180134.9890-1-max@enpas.org
Link: https://lore.kernel.org/all/Yrm9Ezlw1dLmIxyS@kroah.com
Signed-off-by: Max Staudt <max@enpas.org>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 include/uapi/linux/tty.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/tty.h b/include/uapi/linux/tty.h
index 9d0f06bfbac3..68aeae2addec 100644
--- a/include/uapi/linux/tty.h
+++ b/include/uapi/linux/tty.h
@@ -38,8 +38,9 @@
 #define N_NULL		27	/* Null ldisc used for error handling */
 #define N_MCTP		28	/* MCTP-over-serial */
 #define N_DEVELOPMENT	29	/* Manual out-of-tree testing */
+#define N_CAN327	30	/* ELM327 based OBD-II interfaces */
 
 /* Always the newest line discipline + 1 */
-#define NR_LDISCS	30
+#define NR_LDISCS	31
 
 #endif /* _UAPI_LINUX_TTY_H */

base-commit: 0fcae3c8b1b32d79cb4bbf841023757358fb0413
-- 
2.35.1

[PATCH net-next 02/15] can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters

[Marc Kleine-Budde]

From: Max Staudt <max@enpas.org>

This is the can327 driver. It does a surprisingly good job at turning
ELM327 based OBD-II interfaces into cheap CAN interfaces for simple
homebrew projects.

Please see the included documentation for details and limitations:
Documentation/networking/device_drivers/can/can327.rst

Link: https://lore.kernel.org/all/20220618195031.10975-1-max@enpas.org
Signed-off-by: Max Staudt <max@enpas.org>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
[mkl: minor coding style improvements]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 .../networking/device_drivers/can/can327.rst  |  331 +++++
 .../networking/device_drivers/can/index.rst   |    1 +
 MAINTAINERS                                   |    7 +
 drivers/net/can/Kconfig                       |   18 +
 drivers/net/can/Makefile                      |    1 +
 drivers/net/can/can327.c                      | 1137 +++++++++++++++++
 6 files changed, 1495 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/can/can327.rst
 create mode 100644 drivers/net/can/can327.c

diff --git a/Documentation/networking/device_drivers/can/can327.rst b/Documentation/networking/device_drivers/can/can327.rst
new file mode 100644
index 000000000000..b87bfbe5d51c
--- /dev/null
+++ b/Documentation/networking/device_drivers/can/can327.rst
@@ -0,0 +1,331 @@
+.. SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+
+can327: ELM327 driver for Linux SocketCAN
+==========================================
+
+Authors
+--------
+
+Max Staudt <max@enpas.org>
+
+
+
+Motivation
+-----------
+
+This driver aims to lower the initial cost for hackers interested in
+working with CAN buses.
+
+CAN adapters are expensive, few, and far between.
+ELM327 interfaces are cheap and plentiful.
+Let's use ELM327s as CAN adapters.
+
+
+
+Introduction
+-------------
+
+This driver is an effort to turn abundant ELM327 based OBD interfaces
+into full fledged (as far as possible) CAN interfaces.
+
+Since the ELM327 was never meant to be a stand alone CAN controller,
+the driver has to switch between its modes as quickly as possible in
+order to fake full-duplex operation.
+
+As such, can327 is a best effort driver. However, this is more than
+enough to implement simple request-response protocols (such as OBD II),
+and to monitor broadcast messages on a bus (such as in a vehicle).
+
+Most ELM327s come as nondescript serial devices, attached via USB or
+Bluetooth. The driver cannot recognize them by itself, and as such it
+is up to the user to attach it in form of a TTY line discipline
+(similar to PPP, SLIP, slcan, ...).
+
+This driver is meant for ELM327 versions 1.4b and up, see below for
+known limitations in older controllers and clones.
+
+
+
+Data sheet
+-----------
+
+The official data sheets can be found at ELM electronics' home page:
+
+  https://www.elmelectronics.com/
+
+
+
+How to attach the line discipline
+----------------------------------
+
+Every ELM327 chip is factory programmed to operate at a serial setting
+of 38400 baud/s, 8 data bits, no parity, 1 stopbit.
+
+If you have kept this default configuration, the line discipline can
+be attached on a command prompt as follows::
+
+    sudo ldattach \
+           --debug \
+           --speed 38400 \
+           --eightbits \
+           --noparity \
+           --onestopbit \
+           --iflag -ICRNL,INLCR,-IXOFF \
+           30 \
+           /dev/ttyUSB0
+
+To change the ELM327's serial settings, please refer to its data
+sheet. This needs to be done before attaching the line discipline.
+
+Once the ldisc is attached, the CAN interface starts out unconfigured.
+Set the speed before starting it::
+
+    # The interface needs to be down to change parameters
+    sudo ip link set can0 down
+    sudo ip link set can0 type can bitrate 500000
+    sudo ip link set can0 up
+
+500000 bit/s is a common rate for OBD-II diagnostics.
+If you're connecting straight to a car's OBD port, this is the speed
+that most cars (but not all!) expect.
+
+After this, you can set out as usual with candump, cansniffer, etc.
+
+
+
+How to check the controller version
+------------------------------------
+
+Use a terminal program to attach to the controller.
+
+After issuing the "``AT WS``" command, the controller will respond with
+its version::
+
+    >AT WS
+
+
+    ELM327 v1.4b
+
+    >
+
+Note that clones may claim to be any version they like.
+It is not indicative of their actual feature set.
+
+
+
+
+Communication example
+----------------------
+
+This is a short and incomplete introduction on how to talk to an ELM327.
+It is here to guide understanding of the controller's and the driver's
+limitation (listed below) as well as manual testing.
+
+
+The ELM327 has two modes:
+
+- Command mode
+- Reception mode
+
+In command mode, it expects one command per line, terminated by CR.
+By default, the prompt is a "``>``", after which a command can be
+entered::
+
+    >ATE1
+    OK
+    >
+
+The init script in the driver switches off several configuration options
+that are only meaningful in the original OBD scenario the chip is meant
+for, and are actually a hindrance for can327.
+
+
+When a command is not recognized, such as by an older version of the
+ELM327, a question mark is printed as a response instead of OK::
+
+    >ATUNKNOWN
+    ?
+    >
+
+At present, can327 does not evaluate this response. See the section
+below on known limitations for details.
+
+
+When a CAN frame is to be sent, the target address is configured, after
+which the frame is sent as a command that consists of the data's hex
+dump::
+
+    >ATSH123
+    OK
+    >DEADBEEF12345678
+    OK
+    >
+
+The above interaction sends the SFF frame "``DE AD BE EF 12 34 56 78``"
+with (11 bit) CAN ID ``0x123``.
+For this to function, the controller must be configured for SFF sending
+mode (using "``AT PB``", see code or datasheet).
+
+
+Once a frame has been sent and wait-for-reply mode is on (``ATR1``,
+configured on ``listen-only=off``), or when the reply timeout expires
+and the driver sets the controller into monitoring mode (``ATMA``),
+the ELM327 will send one line for each received CAN frame, consisting
+of CAN ID, DLC, and data::
+
+    123 8 DEADBEEF12345678
+
+For EFF (29 bit) CAN frames, the address format is slightly different,
+which can327 uses to tell the two apart::
+
+    12 34 56 78 8 DEADBEEF12345678
+
+The ELM327 will receive both SFF and EFF frames - the current CAN
+config (``ATPB``) does not matter.
+
+
+If the ELM327's internal UART sending buffer runs full, it will abort
+the monitoring mode, print "BUFFER FULL" and drop back into command
+mode. Note that in this case, unlike with other error messages, the
+error message may appear on the same line as the last (usually
+incomplete) data frame::
+
+    12 34 56 78 8 DEADBEEF123 BUFFER FULL
+
+
+
+Known limitations of the controller
+------------------------------------
+
+- Clone devices ("v1.5" and others)
+
+  Sending RTR frames is not supported and will be dropped silently.
+
+  Receiving RTR with DLC 8 will appear to be a regular frame with
+  the last received frame's DLC and payload.
+
+  "``AT CSM``" (CAN Silent Monitoring, i.e. don't send CAN ACKs) is
+  not supported, and is hard coded to ON. Thus, frames are not ACKed
+  while listening: "``AT MA``" (Monitor All) will always be "silent".
+  However, immediately after sending a frame, the ELM327 will be in
+  "receive reply" mode, in which it *does* ACK any received frames.
+  Once the bus goes silent, or an error occurs (such as BUFFER FULL),
+  or the receive reply timeout runs out, the ELM327 will end reply
+  reception mode on its own and can327 will fall back to "``AT MA``"
+  in order to keep monitoring the bus.
+
+  Other limitations may apply, depending on the clone and the quality
+  of its firmware.
+
+
+- All versions
+
+  No full duplex operation is supported. The driver will switch
+  between input/output mode as quickly as possible.
+
+  The length of outgoing RTR frames cannot be set. In fact, some
+  clones (tested with one identifying as "``v1.5``") are unable to
+  send RTR frames at all.
+
+  We don't have a way to get real-time notifications on CAN errors.
+  While there is a command (``AT CS``) to retrieve some basic stats,
+  we don't poll it as it would force us to interrupt reception mode.
+
+
+- Versions prior to 1.4b
+
+  These versions do not send CAN ACKs when in monitoring mode (AT MA).
+  However, they do send ACKs while waiting for a reply immediately
+  after sending a frame. The driver maximizes this time to make the
+  controller as useful as possible.
+
+  Starting with version 1.4b, the ELM327 supports the "``AT CSM``"
+  command, and the "listen-only" CAN option will take effect.
+
+
+- Versions prior to 1.4
+
+  These chips do not support the "``AT PB``" command, and thus cannot
+  change bitrate or SFF/EFF mode on-the-fly. This will have to be
+  programmed by the user before attaching the line discipline. See the
+  data sheet for details.
+
+
+- Versions prior to 1.3
+
+  These chips cannot be used at all with can327. They do not support
+  the "``AT D1``" command, which is necessary to avoid parsing conflicts
+  on incoming data, as well as distinction of RTR frame lengths.
+
+  Specifically, this allows for easy distinction of SFF and EFF
+  frames, and to check whether frames are complete. While it is possible
+  to deduce the type and length from the length of the line the ELM327
+  sends us, this method fails when the ELM327's UART output buffer
+  overruns. It may abort sending in the middle of the line, which will
+  then be mistaken for something else.
+
+
+
+Known limitations of the driver
+--------------------------------
+
+- No 8/7 timing.
+
+  ELM327 can only set CAN bitrates that are of the form 500000/n, where
+  n is an integer divisor.
+  However there is an exception: With a separate flag, it may set the
+  speed to be 8/7 of the speed indicated by the divisor.
+  This mode is not currently implemented.
+
+- No evaluation of command responses.
+
+  The ELM327 will reply with OK when a command is understood, and with ?
+  when it is not. The driver does not currently check this, and simply
+  assumes that the chip understands every command.
+  The driver is built such that functionality degrades gracefully
+  nevertheless. See the section on known limitations of the controller.
+
+- No use of hardware CAN ID filtering
+
+  An ELM327's UART sending buffer will easily overflow on heavy CAN bus
+  load, resulting in the "``BUFFER FULL``" message. Using the hardware
+  filters available through "``AT CF xxx``" and "``AT CM xxx``" would be
+  helpful here, however SocketCAN does not currently provide a facility
+  to make use of such hardware features.
+
+
+
+Rationale behind the chosen configuration
+------------------------------------------
+
+``AT E1``
+  Echo on
+
+  We need this to be able to get a prompt reliably.
+
+``AT S1``
+  Spaces on
+
+  We need this to distinguish 11/29 bit CAN addresses received.
+
+  Note:
+  We can usually do this using the line length (odd/even),
+  but this fails if the line is not transmitted fully to
+  the host (BUFFER FULL).
+
+``AT D1``
+  DLC on
+
+  We need this to tell the "length" of RTR frames.
+
+
+
+A note on CAN bus termination
+------------------------------
+
+Your adapter may have resistors soldered in which are meant to terminate
+the bus. This is correct when it is plugged into a OBD-II socket, but
+not helpful when trying to tap into the middle of an existing CAN bus.
+
+If communications don't work with the adapter connected, check for the
+termination resistors on its PCB and try removing them.
diff --git a/Documentation/networking/device_drivers/can/index.rst b/Documentation/networking/device_drivers/can/index.rst
index 0c3cc6633559..6a8a4f74fa26 100644
--- a/Documentation/networking/device_drivers/can/index.rst
+++ b/Documentation/networking/device_drivers/can/index.rst
@@ -10,6 +10,7 @@ Contents:
 .. toctree::
    :maxdepth: 2
 
+   can327
    ctu/ctucanfd-driver
    freescale/flexcan
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 36f0a205c54a..e3c0641c60cb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7315,6 +7315,13 @@ L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/ethernet/ibm/ehea/
 
+ELM327 CAN NETWORK DRIVER
+M:	Max Staudt <max@enpas.org>
+L:	linux-can@vger.kernel.org
+S:	Maintained
+F:	Documentation/networking/device_drivers/can/can327.rst
+F:	drivers/net/can/can327.c
+
 EM28XX VIDEO4LINUX DRIVER
 M:	Mauro Carvalho Chehab <mchehab@kernel.org>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 806f15146f69..4078d0775572 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -113,6 +113,24 @@ config CAN_AT91
 	  This is a driver for the SoC CAN controller in Atmel's AT91SAM9263
 	  and AT91SAM9X5 processors.
 
+config CAN_CAN327
+	tristate "Serial / USB serial ELM327 based OBD-II Interfaces (can327)"
+	depends on TTY
+	select CAN_RX_OFFLOAD
+	help
+	  CAN driver for several 'low cost' OBD-II interfaces based on the
+	  ELM327 OBD-II interpreter chip.
+
+	  This is a best effort driver - the ELM327 interface was never
+	  designed to be used as a standalone CAN interface. However, it can
+	  still be used for simple request-response protocols (such as OBD II),
+	  and to monitor broadcast messages on a bus (such as in a vehicle).
+
+	  Please refer to the documentation for information on how to use it:
+	  Documentation/networking/device_drivers/can/can327.rst
+
+	  If this driver is built as a module, it will be called can327.
+
 config CAN_FLEXCAN
 	tristate "Support for Freescale FLEXCAN based chips"
 	depends on OF || COLDFIRE || COMPILE_TEST
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 0af85983634c..cafa0e0a3dfe 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -14,6 +14,7 @@ obj-y				+= usb/
 obj-y				+= softing/
 
 obj-$(CONFIG_CAN_AT91)		+= at91_can.o
+obj-$(CONFIG_CAN_CAN327)	+= can327.o
 obj-$(CONFIG_CAN_CC770)		+= cc770/
 obj-$(CONFIG_CAN_C_CAN)		+= c_can/
 obj-$(CONFIG_CAN_CTUCANFD)	+= ctucanfd/
diff --git a/drivers/net/can/can327.c b/drivers/net/can/can327.c
new file mode 100644
index 000000000000..5da7778d92dc
--- /dev/null
+++ b/drivers/net/can/can327.c
@@ -0,0 +1,1137 @@
+// SPDX-License-Identifier: GPL-2.0
+/* ELM327 based CAN interface driver (tty line discipline)
+ *
+ * This driver started as a derivative of linux/drivers/net/can/slcan.c
+ * and my thanks go to the original authors for their inspiration.
+ *
+ * can327.c Author : Max Staudt <max-linux@enpas.org>
+ * slcan.c Author  : Oliver Hartkopp <socketcan@hartkopp.net>
+ * slip.c Authors  : Laurence Culhane <loz@holmes.demon.co.uk>
+ *                   Fred N. van Kempen <waltje@uwalt.nl.mugnet.org>
+ */
+
+#define pr_fmt(fmt) "can327: " fmt
+
+#include <linux/init.h>
+#include <linux/module.h>
+
+#include <linux/bitops.h>
+#include <linux/ctype.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/lockdep.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/tty.h>
+#include <linux/tty_ldisc.h>
+#include <linux/workqueue.h>
+
+#include <uapi/linux/tty.h>
+
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+#include <linux/can/rx-offload.h>
+
+#define CAN327_NAPI_WEIGHT 4
+
+#define CAN327_SIZE_TXBUF 32
+#define CAN327_SIZE_RXBUF 1024
+
+#define CAN327_CAN_CONFIG_SEND_SFF 0x8000
+#define CAN327_CAN_CONFIG_VARIABLE_DLC 0x4000
+#define CAN327_CAN_CONFIG_RECV_BOTH_SFF_EFF 0x2000
+#define CAN327_CAN_CONFIG_BAUDRATE_MULT_8_7 0x1000
+
+#define CAN327_DUMMY_CHAR 'y'
+#define CAN327_DUMMY_STRING "y"
+#define CAN327_READY_CHAR '>'
+
+/* Bits in elm->cmds_todo */
+enum can327_tx_do {
+	CAN327_TX_DO_CAN_DATA = 0,
+	CAN327_TX_DO_CANID_11BIT,
+	CAN327_TX_DO_CANID_29BIT_LOW,
+	CAN327_TX_DO_CANID_29BIT_HIGH,
+	CAN327_TX_DO_CAN_CONFIG_PART2,
+	CAN327_TX_DO_CAN_CONFIG,
+	CAN327_TX_DO_RESPONSES,
+	CAN327_TX_DO_SILENT_MONITOR,
+	CAN327_TX_DO_INIT,
+};
+
+struct can327 {
+	/* This must be the first member when using alloc_candev() */
+	struct can_priv can;
+
+	struct can_rx_offload offload;
+
+	/* TTY buffers */
+	u8 txbuf[CAN327_SIZE_TXBUF];
+	u8 rxbuf[CAN327_SIZE_RXBUF];
+
+	/* Per-channel lock */
+	spinlock_t lock;
+
+	/* TTY and netdev devices that we're bridging */
+	struct tty_struct *tty;
+	struct net_device *dev;
+
+	/* TTY buffer accounting */
+	struct work_struct tx_work;	/* Flushes TTY TX buffer */
+	u8 *txhead;			/* Next TX byte */
+	size_t txleft;			/* Bytes left to TX */
+	int rxfill;			/* Bytes already RX'd in buffer */
+
+	/* State machine */
+	enum {
+		CAN327_STATE_NOTINIT = 0,
+		CAN327_STATE_GETDUMMYCHAR,
+		CAN327_STATE_GETPROMPT,
+		CAN327_STATE_RECEIVING,
+	} state;
+
+	/* Things we have yet to send */
+	char **next_init_cmd;
+	unsigned long cmds_todo;
+
+	/* The CAN frame and config the ELM327 is sending/using,
+	 * or will send/use after finishing all cmds_todo
+	 */
+	struct can_frame can_frame_to_send;
+	u16 can_config;
+	u8 can_bitrate_divisor;
+
+	/* Parser state */
+	bool drop_next_line;
+
+	/* Stop the channel on UART side hardware failure, e.g. stray
+	 * characters or neverending lines. This may be caused by bad
+	 * UART wiring, a bad ELM327, a bad UART bridge...
+	 * Once this is true, nothing will be sent to the TTY.
+	 */
+	bool uart_side_failure;
+};
+
+static inline void can327_uart_side_failure(struct can327 *elm);
+
+static void can327_send(struct can327 *elm, const void *buf, size_t len)
+{
+	int written;
+
+	lockdep_assert_held(&elm->lock);
+
+	if (elm->uart_side_failure)
+		return;
+
+	memcpy(elm->txbuf, buf, len);
+
+	/* Order of next two lines is *very* important.
+	 * When we are sending a little amount of data,
+	 * the transfer may be completed inside the ops->write()
+	 * routine, because it's running with interrupts enabled.
+	 * In this case we *never* got WRITE_WAKEUP event,
+	 * if we did not request it before write operation.
+	 *       14 Oct 1994  Dmitry Gorodchanin.
+	 */
+	set_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
+	written = elm->tty->ops->write(elm->tty, elm->txbuf, len);
+	if (written < 0) {
+		netdev_err(elm->dev, "Failed to write to tty %s.\n",
+			   elm->tty->name);
+		can327_uart_side_failure(elm);
+		return;
+	}
+
+	elm->txleft = len - written;
+	elm->txhead = elm->txbuf + written;
+}
+
+/* Take the ELM327 out of almost any state and back into command mode.
+ * We send CAN327_DUMMY_CHAR which will either abort any running
+ * operation, or be echoed back to us in case we're already in command
+ * mode.
+ */
+static void can327_kick_into_cmd_mode(struct can327 *elm)
+{
+	lockdep_assert_held(&elm->lock);
+
+	if (elm->state != CAN327_STATE_GETDUMMYCHAR &&
+	    elm->state != CAN327_STATE_GETPROMPT) {
+		can327_send(elm, CAN327_DUMMY_STRING, 1);
+
+		elm->state = CAN327_STATE_GETDUMMYCHAR;
+	}
+}
+
+/* Schedule a CAN frame and necessary config changes to be sent to the TTY. */
+static void can327_send_frame(struct can327 *elm, struct can_frame *frame)
+{
+	lockdep_assert_held(&elm->lock);
+
+	/* Schedule any necessary changes in ELM327's CAN configuration */
+	if (elm->can_frame_to_send.can_id != frame->can_id) {
+		/* Set the new CAN ID for transmission. */
+		if ((frame->can_id ^ elm->can_frame_to_send.can_id)
+		    & CAN_EFF_FLAG) {
+			elm->can_config =
+				(frame->can_id & CAN_EFF_FLAG ? 0 : CAN327_CAN_CONFIG_SEND_SFF) |
+				CAN327_CAN_CONFIG_VARIABLE_DLC |
+				CAN327_CAN_CONFIG_RECV_BOTH_SFF_EFF |
+				elm->can_bitrate_divisor;
+
+			set_bit(CAN327_TX_DO_CAN_CONFIG, &elm->cmds_todo);
+		}
+
+		if (frame->can_id & CAN_EFF_FLAG) {
+			clear_bit(CAN327_TX_DO_CANID_11BIT, &elm->cmds_todo);
+			set_bit(CAN327_TX_DO_CANID_29BIT_LOW, &elm->cmds_todo);
+			set_bit(CAN327_TX_DO_CANID_29BIT_HIGH, &elm->cmds_todo);
+		} else {
+			set_bit(CAN327_TX_DO_CANID_11BIT, &elm->cmds_todo);
+			clear_bit(CAN327_TX_DO_CANID_29BIT_LOW,
+				  &elm->cmds_todo);
+			clear_bit(CAN327_TX_DO_CANID_29BIT_HIGH,
+				  &elm->cmds_todo);
+		}
+	}
+
+	/* Schedule the CAN frame itself. */
+	elm->can_frame_to_send = *frame;
+	set_bit(CAN327_TX_DO_CAN_DATA, &elm->cmds_todo);
+
+	can327_kick_into_cmd_mode(elm);
+}
+
+/* ELM327 initialisation sequence.
+ * The line length is limited by the buffer in can327_handle_prompt().
+ */
+static char *can327_init_script[] = {
+	"AT WS\r",        /* v1.0: Warm Start */
+	"AT PP FF OFF\r", /* v1.0: All Programmable Parameters Off */
+	"AT M0\r",        /* v1.0: Memory Off */
+	"AT AL\r",        /* v1.0: Allow Long messages */
+	"AT BI\r",        /* v1.0: Bypass Initialisation */
+	"AT CAF0\r",      /* v1.0: CAN Auto Formatting Off */
+	"AT CFC0\r",      /* v1.0: CAN Flow Control Off */
+	"AT CF 000\r",    /* v1.0: Reset CAN ID Filter */
+	"AT CM 000\r",    /* v1.0: Reset CAN ID Mask */
+	"AT E1\r",        /* v1.0: Echo On */
+	"AT H1\r",        /* v1.0: Headers On */
+	"AT L0\r",        /* v1.0: Linefeeds Off */
+	"AT SH 7DF\r",    /* v1.0: Set CAN sending ID to 0x7df */
+	"AT ST FF\r",     /* v1.0: Set maximum Timeout for response after TX */
+	"AT AT0\r",       /* v1.2: Adaptive Timing Off */
+	"AT D1\r",        /* v1.3: Print DLC On */
+	"AT S1\r",        /* v1.3: Spaces On */
+	"AT TP B\r",      /* v1.0: Try Protocol B */
+	NULL
+};
+
+static void can327_init_device(struct can327 *elm)
+{
+	lockdep_assert_held(&elm->lock);
+
+	elm->state = CAN327_STATE_NOTINIT;
+	elm->can_frame_to_send.can_id = 0x7df; /* ELM327 HW default */
+	elm->rxfill = 0;
+	elm->drop_next_line = 0;
+
+	/* We can only set the bitrate as a fraction of 500000.
+	 * The bitrates listed in can327_bitrate_const will
+	 * limit the user to the right values.
+	 */
+	elm->can_bitrate_divisor = 500000 / elm->can.bittiming.bitrate;
+	elm->can_config =
+		CAN327_CAN_CONFIG_SEND_SFF | CAN327_CAN_CONFIG_VARIABLE_DLC |
+		CAN327_CAN_CONFIG_RECV_BOTH_SFF_EFF | elm->can_bitrate_divisor;
+
+	/* Configure ELM327 and then start monitoring */
+	elm->next_init_cmd = &can327_init_script[0];
+	set_bit(CAN327_TX_DO_INIT, &elm->cmds_todo);
+	set_bit(CAN327_TX_DO_SILENT_MONITOR, &elm->cmds_todo);
+	set_bit(CAN327_TX_DO_RESPONSES, &elm->cmds_todo);
+	set_bit(CAN327_TX_DO_CAN_CONFIG, &elm->cmds_todo);
+
+	can327_kick_into_cmd_mode(elm);
+}
+
+static void can327_feed_frame_to_netdev(struct can327 *elm, struct sk_buff *skb)
+{
+	lockdep_assert_held(&elm->lock);
+
+	if (!netif_running(elm->dev))
+		return;
+
+	/* Queue for NAPI pickup.
+	 * rx-offload will update stats and LEDs for us.
+	 */
+	if (can_rx_offload_queue_tail(&elm->offload, skb))
+		elm->dev->stats.rx_fifo_errors++;
+
+	/* Wake NAPI */
+	can_rx_offload_irq_finish(&elm->offload);
+}
+
+/* Called when we're out of ideas and just want it all to end. */
+static inline void can327_uart_side_failure(struct can327 *elm)
+{
+	struct can_frame *frame;
+	struct sk_buff *skb;
+
+	lockdep_assert_held(&elm->lock);
+
+	elm->uart_side_failure = true;
+
+	clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
+
+	elm->can.can_stats.bus_off++;
+	netif_stop_queue(elm->dev);
+	elm->can.state = CAN_STATE_BUS_OFF;
+	can_bus_off(elm->dev);
+
+	netdev_err(elm->dev,
+		   "ELM327 misbehaved. Blocking further communication.\n");
+
+	skb = alloc_can_err_skb(elm->dev, &frame);
+	if (!skb)
+		return;
+
+	frame->can_id |= CAN_ERR_BUSOFF;
+	can327_feed_frame_to_netdev(elm, skb);
+}
+
+/* Compares a byte buffer (non-NUL terminated) to the payload part of
+ * a string, and returns true iff the buffer (content *and* length) is
+ * exactly that string, without the terminating NUL byte.
+ *
+ * Example: If reference is "BUS ERROR", then this returns true iff nbytes == 9
+ *          and !memcmp(buf, "BUS ERROR", 9).
+ *
+ * The reason to use strings is so we can easily include them in the C
+ * code, and to avoid hardcoding lengths.
+ */
+static inline bool can327_rxbuf_cmp(const u8 *buf, size_t nbytes,
+				    const char *reference)
+{
+	size_t ref_len = strlen(reference);
+
+	return (nbytes == ref_len) && !memcmp(buf, reference, ref_len);
+}
+
+static void can327_parse_error(struct can327 *elm, size_t len)
+{
+	struct can_frame *frame;
+	struct sk_buff *skb;
+
+	lockdep_assert_held(&elm->lock);
+
+	skb = alloc_can_err_skb(elm->dev, &frame);
+	if (!skb)
+		/* It's okay to return here:
+		 * The outer parsing loop will drop this UART buffer.
+		 */
+		return;
+
+	/* Filter possible error messages based on length of RX'd line */
+	if (can327_rxbuf_cmp(elm->rxbuf, len, "UNABLE TO CONNECT")) {
+		netdev_err(elm->dev,
+			   "ELM327 reported UNABLE TO CONNECT. Please check your setup.\n");
+	} else if (can327_rxbuf_cmp(elm->rxbuf, len, "BUFFER FULL")) {
+		/* This will only happen if the last data line was complete.
+		 * Otherwise, can327_parse_frame() will heuristically
+		 * emit this kind of error frame instead.
+		 */
+		frame->can_id |= CAN_ERR_CRTL;
+		frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+	} else if (can327_rxbuf_cmp(elm->rxbuf, len, "BUS ERROR")) {
+		frame->can_id |= CAN_ERR_BUSERROR;
+	} else if (can327_rxbuf_cmp(elm->rxbuf, len, "CAN ERROR")) {
+		frame->can_id |= CAN_ERR_PROT;
+	} else if (can327_rxbuf_cmp(elm->rxbuf, len, "<RX ERROR")) {
+		frame->can_id |= CAN_ERR_PROT;
+	} else if (can327_rxbuf_cmp(elm->rxbuf, len, "BUS BUSY")) {
+		frame->can_id |= CAN_ERR_PROT;
+		frame->data[2] = CAN_ERR_PROT_OVERLOAD;
+	} else if (can327_rxbuf_cmp(elm->rxbuf, len, "FB ERROR")) {
+		frame->can_id |= CAN_ERR_PROT;
+		frame->data[2] = CAN_ERR_PROT_TX;
+	} else if (len == 5 && !memcmp(elm->rxbuf, "ERR", 3)) {
+		/* ERR is followed by two digits, hence line length 5 */
+		netdev_err(elm->dev, "ELM327 reported an ERR%c%c. Please power it off and on again.\n",
+			   elm->rxbuf[3], elm->rxbuf[4]);
+		frame->can_id |= CAN_ERR_CRTL;
+	} else {
+		/* Something else has happened.
+		 * Maybe garbage on the UART line.
+		 * Emit a generic error frame.
+		 */
+	}
+
+	can327_feed_frame_to_netdev(elm, skb);
+}
+
+/* Parse CAN frames coming as ASCII from ELM327.
+ * They can be of various formats:
+ *
+ * 29-bit ID (EFF):  12 34 56 78 D PL PL PL PL PL PL PL PL
+ * 11-bit ID (!EFF): 123 D PL PL PL PL PL PL PL PL
+ *
+ * where D = DLC, PL = payload byte
+ *
+ * Instead of a payload, RTR indicates a remote request.
+ *
+ * We will use the spaces and line length to guess the format.
+ */
+static int can327_parse_frame(struct can327 *elm, size_t len)
+{
+	struct can_frame *frame;
+	struct sk_buff *skb;
+	int hexlen;
+	int datastart;
+	int i;
+
+	lockdep_assert_held(&elm->lock);
+
+	skb = alloc_can_skb(elm->dev, &frame);
+	if (!skb)
+		return -ENOMEM;
+
+	/* Find first non-hex and non-space character:
+	 *  - In the simplest case, there is none.
+	 *  - For RTR frames, 'R' is the first non-hex character.
+	 *  - An error message may replace the end of the data line.
+	 */
+	for (hexlen = 0; hexlen <= len; hexlen++) {
+		if (hex_to_bin(elm->rxbuf[hexlen]) < 0 &&
+		    elm->rxbuf[hexlen] != ' ') {
+			break;
+		}
+	}
+
+	/* Sanity check whether the line is really a clean hexdump,
+	 * or terminated by an error message, or contains garbage.
+	 */
+	if (hexlen < len && !isdigit(elm->rxbuf[hexlen]) &&
+	    !isupper(elm->rxbuf[hexlen]) && '<' != elm->rxbuf[hexlen] &&
+	    ' ' != elm->rxbuf[hexlen]) {
+		/* The line is likely garbled anyway, so bail.
+		 * The main code will restart listening.
+		 */
+		kfree_skb(skb);
+		return -ENODATA;
+	}
+
+	/* Use spaces in CAN ID to distinguish 29 or 11 bit address length.
+	 * No out-of-bounds access:
+	 * We use the fact that we can always read from elm->rxbuf.
+	 */
+	if (elm->rxbuf[2] == ' ' && elm->rxbuf[5] == ' ' &&
+	    elm->rxbuf[8] == ' ' && elm->rxbuf[11] == ' ' &&
+	    elm->rxbuf[13] == ' ') {
+		frame->can_id = CAN_EFF_FLAG;
+		datastart = 14;
+	} else if (elm->rxbuf[3] == ' ' && elm->rxbuf[5] == ' ') {
+		datastart = 6;
+	} else {
+		/* This is not a well-formatted data line.
+		 * Assume it's an error message.
+		 */
+		kfree_skb(skb);
+		return -ENODATA;
+	}
+
+	if (hexlen < datastart) {
+		/* The line is too short to be a valid frame hex dump.
+		 * Something interrupted the hex dump or it is invalid.
+		 */
+		kfree_skb(skb);
+		return -ENODATA;
+	}
+
+	/* From here on all chars up to buf[hexlen] are hex or spaces,
+	 * at well-defined offsets.
+	 */
+
+	/* Read CAN data length */
+	frame->len = (hex_to_bin(elm->rxbuf[datastart - 2]) << 0);
+
+	/* Read CAN ID */
+	if (frame->can_id & CAN_EFF_FLAG) {
+		frame->can_id |= (hex_to_bin(elm->rxbuf[0]) << 28) |
+				 (hex_to_bin(elm->rxbuf[1]) << 24) |
+				 (hex_to_bin(elm->rxbuf[3]) << 20) |
+				 (hex_to_bin(elm->rxbuf[4]) << 16) |
+				 (hex_to_bin(elm->rxbuf[6]) << 12) |
+				 (hex_to_bin(elm->rxbuf[7]) << 8) |
+				 (hex_to_bin(elm->rxbuf[9]) << 4) |
+				 (hex_to_bin(elm->rxbuf[10]) << 0);
+	} else {
+		frame->can_id |= (hex_to_bin(elm->rxbuf[0]) << 8) |
+				 (hex_to_bin(elm->rxbuf[1]) << 4) |
+				 (hex_to_bin(elm->rxbuf[2]) << 0);
+	}
+
+	/* Check for RTR frame */
+	if (elm->rxfill >= hexlen + 3 &&
+	    !memcmp(&elm->rxbuf[hexlen], "RTR", 3)) {
+		frame->can_id |= CAN_RTR_FLAG;
+	}
+
+	/* Is the line long enough to hold the advertised payload?
+	 * Note: RTR frames have a DLC, but no actual payload.
+	 */
+	if (!(frame->can_id & CAN_RTR_FLAG) &&
+	    (hexlen < frame->len * 3 + datastart)) {
+		/* Incomplete frame.
+		 * Probably the ELM327's RS232 TX buffer was full.
+		 * Emit an error frame and exit.
+		 */
+		frame->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL;
+		frame->len = CAN_ERR_DLC;
+		frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+		can327_feed_frame_to_netdev(elm, skb);
+
+		/* Signal failure to parse.
+		 * The line will be re-parsed as an error line, which will fail.
+		 * However, this will correctly drop the state machine back into
+		 * command mode.
+		 */
+		return -ENODATA;
+	}
+
+	/* Parse the data nibbles. */
+	for (i = 0; i < frame->len; i++) {
+		frame->data[i] =
+			(hex_to_bin(elm->rxbuf[datastart + 3 * i]) << 4) |
+			(hex_to_bin(elm->rxbuf[datastart + 3 * i + 1]));
+	}
+
+	/* Feed the frame to the network layer. */
+	can327_feed_frame_to_netdev(elm, skb);
+
+	return 0;
+}
+
+static void can327_parse_line(struct can327 *elm, size_t len)
+{
+	lockdep_assert_held(&elm->lock);
+
+	/* Skip empty lines */
+	if (!len)
+		return;
+
+	/* Skip echo lines */
+	if (elm->drop_next_line) {
+		elm->drop_next_line = 0;
+		return;
+	} else if (!memcmp(elm->rxbuf, "AT", 2)) {
+		return;
+	}
+
+	/* Regular parsing */
+	if (elm->state == CAN327_STATE_RECEIVING &&
+	    can327_parse_frame(elm, len)) {
+		/* Parse an error line. */
+		can327_parse_error(elm, len);
+
+		/* Start afresh. */
+		can327_kick_into_cmd_mode(elm);
+	}
+}
+
+static void can327_handle_prompt(struct can327 *elm)
+{
+	struct can_frame *frame = &elm->can_frame_to_send;
+	/* Size this buffer for the largest ELM327 line we may generate,
+	 * which is currently an 8 byte CAN frame's payload hexdump.
+	 * Items in can327_init_script must fit here, too!
+	 */
+	char local_txbuf[sizeof("0102030405060708\r")];
+
+	lockdep_assert_held(&elm->lock);
+
+	if (!elm->cmds_todo) {
+		/* Enter CAN monitor mode */
+		can327_send(elm, "ATMA\r", 5);
+		elm->state = CAN327_STATE_RECEIVING;
+
+		/* We will be in the default state once this command is
+		 * sent, so enable the TX packet queue.
+		 */
+		netif_wake_queue(elm->dev);
+
+		return;
+	}
+
+	/* Reconfigure ELM327 step by step as indicated by elm->cmds_todo */
+	if (test_bit(CAN327_TX_DO_INIT, &elm->cmds_todo)) {
+		snprintf(local_txbuf, sizeof(local_txbuf), "%s",
+			 *elm->next_init_cmd);
+
+		elm->next_init_cmd++;
+		if (!(*elm->next_init_cmd)) {
+			clear_bit(CAN327_TX_DO_INIT, &elm->cmds_todo);
+			/* Init finished. */
+		}
+
+	} else if (test_and_clear_bit(CAN327_TX_DO_SILENT_MONITOR, &elm->cmds_todo)) {
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			 "ATCSM%i\r",
+			 !!(elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY));
+
+	} else if (test_and_clear_bit(CAN327_TX_DO_RESPONSES, &elm->cmds_todo)) {
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			 "ATR%i\r",
+			 !(elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY));
+
+	} else if (test_and_clear_bit(CAN327_TX_DO_CAN_CONFIG, &elm->cmds_todo)) {
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			 "ATPC\r");
+		set_bit(CAN327_TX_DO_CAN_CONFIG_PART2, &elm->cmds_todo);
+
+	} else if (test_and_clear_bit(CAN327_TX_DO_CAN_CONFIG_PART2, &elm->cmds_todo)) {
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			 "ATPB%04X\r",
+			 elm->can_config);
+
+	} else if (test_and_clear_bit(CAN327_TX_DO_CANID_29BIT_HIGH, &elm->cmds_todo)) {
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			 "ATCP%02X\r",
+			 (frame->can_id & CAN_EFF_MASK) >> 24);
+
+	} else if (test_and_clear_bit(CAN327_TX_DO_CANID_29BIT_LOW, &elm->cmds_todo)) {
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			 "ATSH%06X\r",
+			 frame->can_id & CAN_EFF_MASK & ((1 << 24) - 1));
+
+	} else if (test_and_clear_bit(CAN327_TX_DO_CANID_11BIT, &elm->cmds_todo)) {
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			 "ATSH%03X\r",
+			 frame->can_id & CAN_SFF_MASK);
+
+	} else if (test_and_clear_bit(CAN327_TX_DO_CAN_DATA, &elm->cmds_todo)) {
+		if (frame->can_id & CAN_RTR_FLAG) {
+			/* Send an RTR frame. Their DLC is fixed.
+			 * Some chips don't send them at all.
+			 */
+			snprintf(local_txbuf, sizeof(local_txbuf), "ATRTR\r");
+		} else {
+			/* Send a regular CAN data frame */
+			int i;
+
+			for (i = 0; i < frame->len; i++) {
+				snprintf(&local_txbuf[2 * i],
+					 sizeof(local_txbuf), "%02X",
+					 frame->data[i]);
+			}
+
+			snprintf(&local_txbuf[2 * i], sizeof(local_txbuf),
+				 "\r");
+		}
+
+		elm->drop_next_line = 1;
+		elm->state = CAN327_STATE_RECEIVING;
+
+		/* We will be in the default state once this command is
+		 * sent, so enable the TX packet queue.
+		 */
+		netif_wake_queue(elm->dev);
+	}
+
+	can327_send(elm, local_txbuf, strlen(local_txbuf));
+}
+
+static bool can327_is_ready_char(char c)
+{
+	/* Bits 0xc0 are sometimes set (randomly), hence the mask.
+	 * Probably bad hardware.
+	 */
+	return (c & 0x3f) == CAN327_READY_CHAR;
+}
+
+static void can327_drop_bytes(struct can327 *elm, size_t i)
+{
+	lockdep_assert_held(&elm->lock);
+
+	memmove(&elm->rxbuf[0], &elm->rxbuf[i], CAN327_SIZE_RXBUF - i);
+	elm->rxfill -= i;
+}
+
+static void can327_parse_rxbuf(struct can327 *elm, size_t first_new_char_idx)
+{
+	size_t len, pos;
+
+	lockdep_assert_held(&elm->lock);
+
+	switch (elm->state) {
+	case CAN327_STATE_NOTINIT:
+		elm->rxfill = 0;
+		break;
+
+	case CAN327_STATE_GETDUMMYCHAR:
+		/* Wait for 'y' or '>' */
+		for (pos = 0; pos < elm->rxfill; pos++) {
+			if (elm->rxbuf[pos] == CAN327_DUMMY_CHAR) {
+				can327_send(elm, "\r", 1);
+				elm->state = CAN327_STATE_GETPROMPT;
+				pos++;
+				break;
+			} else if (can327_is_ready_char(elm->rxbuf[pos])) {
+				can327_send(elm, CAN327_DUMMY_STRING, 1);
+				pos++;
+				break;
+			}
+		}
+
+		can327_drop_bytes(elm, pos);
+		break;
+
+	case CAN327_STATE_GETPROMPT:
+		/* Wait for '>' */
+		if (can327_is_ready_char(elm->rxbuf[elm->rxfill - 1]))
+			can327_handle_prompt(elm);
+
+		elm->rxfill = 0;
+		break;
+
+	case CAN327_STATE_RECEIVING:
+		/* Find <CR> delimiting feedback lines. */
+		len = first_new_char_idx;
+		while (len < elm->rxfill && elm->rxbuf[len] != '\r')
+			len++;
+
+		if (len == CAN327_SIZE_RXBUF) {
+			/* Assume the buffer ran full with garbage.
+			 * Did we even connect at the right baud rate?
+			 */
+			netdev_err(elm->dev,
+				   "RX buffer overflow. Faulty ELM327 or UART?\n");
+			can327_uart_side_failure(elm);
+		} else if (len == elm->rxfill) {
+			if (can327_is_ready_char(elm->rxbuf[elm->rxfill - 1])) {
+				/* The ELM327's AT ST response timeout ran out,
+				 * so we got a prompt.
+				 * Clear RX buffer and restart listening.
+				 */
+				elm->rxfill = 0;
+
+				can327_handle_prompt(elm);
+			}
+
+			/* No <CR> found - we haven't received a full line yet.
+			 * Wait for more data.
+			 */
+		} else {
+			/* We have a full line to parse. */
+			can327_parse_line(elm, len);
+
+			/* Remove parsed data from RX buffer. */
+			can327_drop_bytes(elm, len + 1);
+
+			/* More data to parse? */
+			if (elm->rxfill)
+				can327_parse_rxbuf(elm, 0);
+		}
+	}
+}
+
+static int can327_netdev_open(struct net_device *dev)
+{
+	struct can327 *elm = netdev_priv(dev);
+	int err;
+
+	spin_lock_bh(&elm->lock);
+
+	if (!elm->tty) {
+		spin_unlock_bh(&elm->lock);
+		return -ENODEV;
+	}
+
+	if (elm->uart_side_failure)
+		netdev_warn(elm->dev,
+			    "Reopening netdev after a UART side fault has been detected.\n");
+
+	/* Clear TTY buffers */
+	elm->rxfill = 0;
+	elm->txleft = 0;
+
+	/* open_candev() checks for elm->can.bittiming.bitrate != 0 */
+	err = open_candev(dev);
+	if (err) {
+		spin_unlock_bh(&elm->lock);
+		return err;
+	}
+
+	can327_init_device(elm);
+	spin_unlock_bh(&elm->lock);
+
+	err = can_rx_offload_add_manual(dev, &elm->offload, CAN327_NAPI_WEIGHT);
+	if (err) {
+		close_candev(dev);
+		return err;
+	}
+
+	can_rx_offload_enable(&elm->offload);
+
+	elm->can.state = CAN_STATE_ERROR_ACTIVE;
+	netif_start_queue(dev);
+
+	return 0;
+}
+
+static int can327_netdev_close(struct net_device *dev)
+{
+	struct can327 *elm = netdev_priv(dev);
+
+	/* Interrupt whatever the ELM327 is doing right now */
+	spin_lock_bh(&elm->lock);
+	can327_send(elm, CAN327_DUMMY_STRING, 1);
+	spin_unlock_bh(&elm->lock);
+
+	netif_stop_queue(dev);
+
+	/* Give UART one final chance to flush. */
+	clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
+	flush_work(&elm->tx_work);
+
+	can_rx_offload_disable(&elm->offload);
+	elm->can.state = CAN_STATE_STOPPED;
+	can_rx_offload_del(&elm->offload);
+	close_candev(dev);
+
+	return 0;
+}
+
+/* Send a can_frame to a TTY. */
+static netdev_tx_t can327_netdev_start_xmit(struct sk_buff *skb,
+					    struct net_device *dev)
+{
+	struct can327 *elm = netdev_priv(dev);
+	struct can_frame *frame = (struct can_frame *)skb->data;
+
+	if (can_dropped_invalid_skb(dev, skb))
+		return NETDEV_TX_OK;
+
+	/* We shouldn't get here after a hardware fault:
+	 * can_bus_off() calls netif_carrier_off()
+	 */
+	if (elm->uart_side_failure) {
+		WARN_ON_ONCE(elm->uart_side_failure);
+		goto out;
+	}
+
+	netif_stop_queue(dev);
+
+	/* BHs are already disabled, so no spin_lock_bh().
+	 * See Documentation/networking/netdevices.txt
+	 */
+	spin_lock(&elm->lock);
+	can327_send_frame(elm, frame);
+	spin_unlock(&elm->lock);
+
+	dev->stats.tx_packets++;
+	dev->stats.tx_bytes += frame->can_id & CAN_RTR_FLAG ? 0 : frame->len;
+
+out:
+	kfree_skb(skb);
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops can327_netdev_ops = {
+	.ndo_open = can327_netdev_open,
+	.ndo_stop = can327_netdev_close,
+	.ndo_start_xmit = can327_netdev_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
+};
+
+static bool can327_is_valid_rx_char(u8 c)
+{
+	static const bool lut_char_is_valid['z'] = {
+		['\r'] = true,
+		[' '] = true,
+		['.'] = true,
+		['0'] = true, true, true, true, true,
+		['5'] = true, true, true, true, true,
+		['<'] = true,
+		[CAN327_READY_CHAR] = true,
+		['?'] = true,
+		['A'] = true, true, true, true, true, true, true,
+		['H'] = true, true, true, true, true, true, true,
+		['O'] = true, true, true, true, true, true, true,
+		['V'] = true, true, true, true, true,
+		['a'] = true,
+		['b'] = true,
+		['v'] = true,
+		[CAN327_DUMMY_CHAR] = true,
+	};
+	BUILD_BUG_ON(CAN327_DUMMY_CHAR >= 'z');
+
+	return (c < ARRAY_SIZE(lut_char_is_valid) && lut_char_is_valid[c]);
+}
+
+/* Handle incoming ELM327 ASCII data.
+ * This will not be re-entered while running, but other ldisc
+ * functions may be called in parallel.
+ */
+static void can327_ldisc_rx(struct tty_struct *tty, const unsigned char *cp,
+			    const char *fp, int count)
+{
+	struct can327 *elm = (struct can327 *)tty->disc_data;
+	size_t first_new_char_idx;
+
+	if (elm->uart_side_failure)
+		return;
+
+	spin_lock_bh(&elm->lock);
+
+	/* Store old rxfill, so can327_parse_rxbuf() will have
+	 * the option of skipping already checked characters.
+	 */
+	first_new_char_idx = elm->rxfill;
+
+	while (count-- && elm->rxfill < CAN327_SIZE_RXBUF) {
+		if (fp && *fp++) {
+			netdev_err(elm->dev,
+				   "Error in received character stream. Check your wiring.");
+
+			can327_uart_side_failure(elm);
+
+			spin_unlock_bh(&elm->lock);
+			return;
+		}
+
+		/* Ignore NUL characters, which the PIC microcontroller may
+		 * inadvertently insert due to a known hardware bug.
+		 * See ELM327 documentation, which refers to a Microchip PIC
+		 * bug description.
+		 */
+		if (*cp) {
+			/* Check for stray characters on the UART line.
+			 * Likely caused by bad hardware.
+			 */
+			if (!can327_is_valid_rx_char(*cp)) {
+				netdev_err(elm->dev,
+					   "Received illegal character %02x.\n",
+					   *cp);
+				can327_uart_side_failure(elm);
+
+				spin_unlock_bh(&elm->lock);
+				return;
+			}
+
+			elm->rxbuf[elm->rxfill++] = *cp;
+		}
+
+		cp++;
+	}
+
+	if (count >= 0) {
+		netdev_err(elm->dev,
+			   "Receive buffer overflowed. Bad chip or wiring? count = %i",
+			   count);
+
+		can327_uart_side_failure(elm);
+
+		spin_unlock_bh(&elm->lock);
+		return;
+	}
+
+	can327_parse_rxbuf(elm, first_new_char_idx);
+	spin_unlock_bh(&elm->lock);
+}
+
+/* Write out remaining transmit buffer.
+ * Scheduled when TTY is writable.
+ */
+static void can327_ldisc_tx_worker(struct work_struct *work)
+{
+	struct can327 *elm = container_of(work, struct can327, tx_work);
+	ssize_t written;
+
+	if (elm->uart_side_failure)
+		return;
+
+	spin_lock_bh(&elm->lock);
+
+	if (elm->txleft) {
+		written = elm->tty->ops->write(elm->tty, elm->txhead,
+					       elm->txleft);
+		if (written < 0) {
+			netdev_err(elm->dev, "Failed to write to tty %s.\n",
+				   elm->tty->name);
+			can327_uart_side_failure(elm);
+
+			spin_unlock_bh(&elm->lock);
+			return;
+		}
+
+		elm->txleft -= written;
+		elm->txhead += written;
+	}
+
+	if (!elm->txleft)
+		clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
+
+	spin_unlock_bh(&elm->lock);
+}
+
+/* Called by the driver when there's room for more data. */
+static void can327_ldisc_tx_wakeup(struct tty_struct *tty)
+{
+	struct can327 *elm = (struct can327 *)tty->disc_data;
+
+	schedule_work(&elm->tx_work);
+}
+
+/* ELM327 can only handle bitrates that are integer divisors of 500 kHz,
+ * or 7/8 of that. Divisors are 1 to 64.
+ * Currently we don't implement support for 7/8 rates.
+ */
+static const u32 can327_bitrate_const[] = {
+	7812,  7936,  8064,  8196,   8333,   8474,   8620,   8771,
+	8928,  9090,  9259,  9433,   9615,   9803,   10000,  10204,
+	10416, 10638, 10869, 11111,  11363,  11627,  11904,  12195,
+	12500, 12820, 13157, 13513,  13888,  14285,  14705,  15151,
+	15625, 16129, 16666, 17241,  17857,  18518,  19230,  20000,
+	20833, 21739, 22727, 23809,  25000,  26315,  27777,  29411,
+	31250, 33333, 35714, 38461,  41666,  45454,  50000,  55555,
+	62500, 71428, 83333, 100000, 125000, 166666, 250000, 500000
+};
+
+static int can327_ldisc_open(struct tty_struct *tty)
+{
+	struct net_device *dev;
+	struct can327 *elm;
+	int err;
+
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
+	if (!tty->ops->write)
+		return -EOPNOTSUPP;
+
+	dev = alloc_candev(sizeof(struct can327), 0);
+	if (!dev)
+		return -ENFILE;
+	elm = netdev_priv(dev);
+
+	/* Configure TTY interface */
+	tty->receive_room = 65536; /* We don't flow control */
+	spin_lock_init(&elm->lock);
+	INIT_WORK(&elm->tx_work, can327_ldisc_tx_worker);
+
+	/* Configure CAN metadata */
+	elm->can.bitrate_const = can327_bitrate_const;
+	elm->can.bitrate_const_cnt = ARRAY_SIZE(can327_bitrate_const);
+	elm->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY;
+
+	/* Configure netdev interface */
+	elm->dev = dev;
+	dev->netdev_ops = &can327_netdev_ops;
+
+	/* Mark ldisc channel as alive */
+	elm->tty = tty;
+	tty->disc_data = elm;
+
+	/* Let 'er rip */
+	err = register_candev(elm->dev);
+	if (err) {
+		free_candev(elm->dev);
+		return err;
+	}
+
+	netdev_info(elm->dev, "can327 on %s.\n", tty->name);
+
+	return 0;
+}
+
+/* Close down a can327 channel.
+ * This means flushing out any pending queues, and then returning.
+ * This call is serialized against other ldisc functions:
+ * Once this is called, no other ldisc function of ours is entered.
+ *
+ * We also use this function for a hangup event.
+ */
+static void can327_ldisc_close(struct tty_struct *tty)
+{
+	struct can327 *elm = (struct can327 *)tty->disc_data;
+
+	/* unregister_netdev() calls .ndo_stop() so we don't have to.
+	 * Our .ndo_stop() also flushes the TTY write wakeup handler,
+	 * so we can safely set elm->tty = NULL after this.
+	 */
+	unregister_candev(elm->dev);
+
+	/* Mark channel as dead */
+	spin_lock_bh(&elm->lock);
+	tty->disc_data = NULL;
+	elm->tty = NULL;
+	spin_unlock_bh(&elm->lock);
+
+	netdev_info(elm->dev, "can327 off %s.\n", tty->name);
+
+	free_candev(elm->dev);
+}
+
+static int can327_ldisc_ioctl(struct tty_struct *tty, unsigned int cmd,
+			      unsigned long arg)
+{
+	struct can327 *elm = (struct can327 *)tty->disc_data;
+	unsigned int tmp;
+
+	switch (cmd) {
+	case SIOCGIFNAME:
+		tmp = strnlen(elm->dev->name, IFNAMSIZ - 1) + 1;
+		if (copy_to_user((void __user *)arg, elm->dev->name, tmp))
+			return -EFAULT;
+		return 0;
+
+	case SIOCSIFHWADDR:
+		return -EINVAL;
+
+	default:
+		return tty_mode_ioctl(tty, cmd, arg);
+	}
+}
+
+static struct tty_ldisc_ops can327_ldisc = {
+	.owner = THIS_MODULE,
+	.name = "can327",
+	.num = N_CAN327,
+	.receive_buf = can327_ldisc_rx,
+	.write_wakeup = can327_ldisc_tx_wakeup,
+	.open = can327_ldisc_open,
+	.close = can327_ldisc_close,
+	.ioctl = can327_ldisc_ioctl,
+};
+
+static int __init can327_init(void)
+{
+	int status;
+
+	status = tty_register_ldisc(&can327_ldisc);
+	if (status)
+		pr_err("Can't register line discipline\n");
+
+	return status;
+}
+
+static void __exit can327_exit(void)
+{
+	/* This will only be called when all channels have been closed by
+	 * userspace - tty_ldisc.c takes care of the module's refcount.
+	 */
+	tty_unregister_ldisc(&can327_ldisc);
+}
+
+module_init(can327_init);
+module_exit(can327_exit);
+
+MODULE_ALIAS_LDISC(N_CAN327);
+MODULE_DESCRIPTION("ELM327 based CAN interface");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Max Staudt <max@enpas.org>");
-- 
2.35.1

[PATCH net-next 03/15] can: ctucanfd: ctucan_interrupt(): fix typo

[Marc Kleine-Budde]

This patch fixes the typo "poniter" -> "pointer" in the kerneldoc of
ctucan_interrupt().

Link: https://lore.kernel.org/all/20220628083204.881311-1-mkl@pengutronix.de
Fixes: 2dcb8e8782d8 ("can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part.")
Cc: Ondrej Ille <ondrej.ille@gmail.com>
Cc: Martin Jerabek <martin.jerabek01@gmail.com>
Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/ctucanfd/ctucanfd_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
index 64990bf20fdc..14ac7c0ee04c 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -1087,7 +1087,7 @@ static void ctucan_tx_interrupt(struct net_device *ndev)
 /**
  * ctucan_interrupt() - CAN Isr
  * @irq:	irq number
- * @dev_id:	device id poniter
+ * @dev_id:	device id pointer
  *
  * This is the CTU CAN FD ISR. It checks for the type of interrupt
  * and invokes the corresponding ISR.
-- 
2.35.1

[PATCH net-next 04/15] can: slcan: use the BIT() helper

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

Use the BIT() helper instead of an explicit shift.

Link: https://lore.kernel.org/all/20220628163137.413025-2-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index 64a3aee8a7da..b37d35c2a23a 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -413,7 +413,7 @@ static int slc_open(struct net_device *dev)
 	if (sl->tty == NULL)
 		return -ENODEV;
 
-	sl->flags &= (1 << SLF_INUSE);
+	sl->flags &= BIT(SLF_INUSE);
 	netif_start_queue(dev);
 	return 0;
 }
-- 
2.35.1

[PATCH net-next 05/15] can: slcan: use netdev helpers to print out messages

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

Replace printk() calls with corresponding netdev helpers.

Link: https://lore.kernel.org/all/20220628163137.413025-3-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index b37d35c2a23a..6162a9c21672 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -365,7 +365,7 @@ static netdev_tx_t slc_xmit(struct sk_buff *skb, struct net_device *dev)
 	spin_lock(&sl->lock);
 	if (!netif_running(dev))  {
 		spin_unlock(&sl->lock);
-		printk(KERN_WARNING "%s: xmit: iface is down\n", dev->name);
+		netdev_warn(dev, "xmit: iface is down\n");
 		goto out;
 	}
 	if (sl->tty == NULL) {
@@ -776,8 +776,7 @@ static void __exit slcan_exit(void)
 
 		sl = netdev_priv(dev);
 		if (sl->tty) {
-			printk(KERN_ERR "%s: tty discipline still running\n",
-			       dev->name);
+			netdev_err(dev, "tty discipline still running\n");
 		}
 
 		unregister_netdev(dev);
-- 
2.35.1

[PATCH net-next 06/15] can: slcan: use the alloc_can_skb() helper

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

It is used successfully by most (if not all) CAN device drivers. It
allows to remove replicated code.

Link: https://lore.kernel.org/all/20220628163137.413025-4-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan.c | 70 +++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 37 deletions(-)

diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index 6162a9c21672..c39580b142e0 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -54,6 +54,7 @@
 #include <linux/kernel.h>
 #include <linux/workqueue.h>
 #include <linux/can.h>
+#include <linux/can/dev.h>
 #include <linux/can/skb.h>
 #include <linux/can/can-ml.h>
 
@@ -143,85 +144,80 @@ static struct net_device **slcan_devs;
 static void slc_bump(struct slcan *sl)
 {
 	struct sk_buff *skb;
-	struct can_frame cf;
+	struct can_frame *cf;
 	int i, tmp;
 	u32 tmpid;
 	char *cmd = sl->rbuff;
 
-	memset(&cf, 0, sizeof(cf));
+	skb = alloc_can_skb(sl->dev, &cf);
+	if (unlikely(!skb)) {
+		sl->dev->stats.rx_dropped++;
+		return;
+	}
 
 	switch (*cmd) {
 	case 'r':
-		cf.can_id = CAN_RTR_FLAG;
+		cf->can_id = CAN_RTR_FLAG;
 		fallthrough;
 	case 't':
 		/* store dlc ASCII value and terminate SFF CAN ID string */
-		cf.len = sl->rbuff[SLC_CMD_LEN + SLC_SFF_ID_LEN];
+		cf->len = sl->rbuff[SLC_CMD_LEN + SLC_SFF_ID_LEN];
 		sl->rbuff[SLC_CMD_LEN + SLC_SFF_ID_LEN] = 0;
 		/* point to payload data behind the dlc */
 		cmd += SLC_CMD_LEN + SLC_SFF_ID_LEN + 1;
 		break;
 	case 'R':
-		cf.can_id = CAN_RTR_FLAG;
+		cf->can_id = CAN_RTR_FLAG;
 		fallthrough;
 	case 'T':
-		cf.can_id |= CAN_EFF_FLAG;
+		cf->can_id |= CAN_EFF_FLAG;
 		/* store dlc ASCII value and terminate EFF CAN ID string */
-		cf.len = sl->rbuff[SLC_CMD_LEN + SLC_EFF_ID_LEN];
+		cf->len = sl->rbuff[SLC_CMD_LEN + SLC_EFF_ID_LEN];
 		sl->rbuff[SLC_CMD_LEN + SLC_EFF_ID_LEN] = 0;
 		/* point to payload data behind the dlc */
 		cmd += SLC_CMD_LEN + SLC_EFF_ID_LEN + 1;
 		break;
 	default:
-		return;
+		goto decode_failed;
 	}
 
 	if (kstrtou32(sl->rbuff + SLC_CMD_LEN, 16, &tmpid))
-		return;
+		goto decode_failed;
 
-	cf.can_id |= tmpid;
+	cf->can_id |= tmpid;
 
 	/* get len from sanitized ASCII value */
-	if (cf.len >= '0' && cf.len < '9')
-		cf.len -= '0';
+	if (cf->len >= '0' && cf->len < '9')
+		cf->len -= '0';
 	else
-		return;
+		goto decode_failed;
 
 	/* RTR frames may have a dlc > 0 but they never have any data bytes */
-	if (!(cf.can_id & CAN_RTR_FLAG)) {
-		for (i = 0; i < cf.len; i++) {
+	if (!(cf->can_id & CAN_RTR_FLAG)) {
+		for (i = 0; i < cf->len; i++) {
 			tmp = hex_to_bin(*cmd++);
 			if (tmp < 0)
-				return;
-			cf.data[i] = (tmp << 4);
+				goto decode_failed;
+
+			cf->data[i] = (tmp << 4);
 			tmp = hex_to_bin(*cmd++);
 			if (tmp < 0)
-				return;
-			cf.data[i] |= tmp;
+				goto decode_failed;
+
+			cf->data[i] |= tmp;
 		}
 	}
 
-	skb = dev_alloc_skb(sizeof(struct can_frame) +
-			    sizeof(struct can_skb_priv));
-	if (!skb)
-		return;
-
-	skb->dev = sl->dev;
-	skb->protocol = htons(ETH_P_CAN);
-	skb->pkt_type = PACKET_BROADCAST;
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
-
-	can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = sl->dev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
-
-	skb_put_data(skb, &cf, sizeof(struct can_frame));
-
 	sl->dev->stats.rx_packets++;
-	if (!(cf.can_id & CAN_RTR_FLAG))
-		sl->dev->stats.rx_bytes += cf.len;
+	if (!(cf->can_id & CAN_RTR_FLAG))
+		sl->dev->stats.rx_bytes += cf->len;
 
 	netif_rx(skb);
+	return;
+
+decode_failed:
+	sl->dev->stats.rx_errors++;
+	dev_kfree_skb(skb);
 }
 
 /* parse tty input stream */
-- 
2.35.1

[PATCH net-next 07/15] can: netlink: dump bitrate 0 if can_priv::bittiming.bitrate is -1U

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

Upcoming changes on slcan driver will require you to specify a bitrate
of value -1 to prevent the open_candev() from failing but at the same
time highlighting that it is a fake value. In this case the command
`ip --details -s -s link show' would print 4294967295 as the bitrate
value. The patch change this value in 0.

Link: https://lore.kernel.org/all/20220628163137.413025-5-dario.binacchi@amarulasolutions.com
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev/netlink.c | 3 ++-
 include/linux/can/bittiming.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 037824011266..8efa22d9f214 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -511,7 +511,8 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
 	if (priv->do_get_state)
 		priv->do_get_state(dev, &state);
 
-	if ((priv->bittiming.bitrate &&
+	if ((priv->bittiming.bitrate != CAN_BITRATE_UNSET &&
+	     priv->bittiming.bitrate != CAN_BITRATE_UNKNOWN &&
 	     nla_put(skb, IFLA_CAN_BITTIMING,
 		     sizeof(priv->bittiming), &priv->bittiming)) ||
 
diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h
index 7ae21c0f7f23..ef0a77173e3c 100644
--- a/include/linux/can/bittiming.h
+++ b/include/linux/can/bittiming.h
@@ -11,6 +11,8 @@
 
 #define CAN_SYNC_SEG 1
 
+#define CAN_BITRATE_UNSET 0
+#define CAN_BITRATE_UNKNOWN (-1U)
 
 #define CAN_CTRLMODE_TDC_MASK					\
 	(CAN_CTRLMODE_TDC_AUTO | CAN_CTRLMODE_TDC_MANUAL)
-- 
2.35.1

[PATCH net-next 08/15] can: slcan: use CAN network device driver API

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

As suggested by commit [1], now the driver uses the functions and the
data structures provided by the CAN network device driver interface.

Currently the driver doesn't implement a way to set bitrate for SLCAN
based devices via ip tool, so you'll have to do this by slcand or
slcan_attach invocation through the -sX parameter:

- slcan_attach -f -s6 -o /dev/ttyACM0
- slcand -f -s8 -o /dev/ttyUSB0

where -s6 in will set adapter's bitrate to 500 Kbit/s and -s8 to
1Mbit/s.
See the table below for further CAN bitrates:
- s0 ->   10 Kbit/s
- s1 ->   20 Kbit/s
- s2 ->   50 Kbit/s
- s3 ->  100 Kbit/s
- s4 ->  125 Kbit/s
- s5 ->  250 Kbit/s
- s6 ->  500 Kbit/s
- s7 ->  800 Kbit/s
- s8 -> 1000 Kbit/s

In doing so, the struct can_priv::bittiming.bitrate of the driver is not
set and since the open_candev() checks that the bitrate has been set, it
must be a non-zero value, the bitrate is set to a fake value (-1U)
before it is called.

Using the rtnl_lock()/rtnl_unlock() functions has become a bit more
tricky as the register_candev() function indirectly calls rtnl_lock()
via register_netdev(). To avoid a deadlock it is therefore necessary to
call rtnl_unlock() before calling register_candev(). The same goes for
the unregister_candev() function.

[1] commit 39549eef3587f ("can: CAN Network device driver and Netlink interface")

Link: https://lore.kernel.org/all/20220628163137.413025-6-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/Kconfig | 40 ++++++++++----------
 drivers/net/can/slcan.c | 82 ++++++++++++++++++++---------------------
 2 files changed, 60 insertions(+), 62 deletions(-)

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 4078d0775572..3048ad77edb3 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -49,26 +49,6 @@ config CAN_VXCAN
 	  This driver can also be built as a module.  If so, the module
 	  will be called vxcan.
 
-config CAN_SLCAN
-	tristate "Serial / USB serial CAN Adaptors (slcan)"
-	depends on TTY
-	help
-	  CAN driver for several 'low cost' CAN interfaces that are attached
-	  via serial lines or via USB-to-serial adapters using the LAWICEL
-	  ASCII protocol. The driver implements the tty linediscipline N_SLCAN.
-
-	  As only the sending and receiving of CAN frames is implemented, this
-	  driver should work with the (serial/USB) CAN hardware from:
-	  www.canusb.com / www.can232.com / www.mictronics.de / www.canhack.de
-
-	  Userspace tools to attach the SLCAN line discipline (slcan_attach,
-	  slcand) can be found in the can-utils at the linux-can project, see
-	  https://github.com/linux-can/can-utils for details.
-
-	  The slcan driver supports up to 10 CAN netdevices by default which
-	  can be changed by the 'maxdev=xx' module option. This driver can
-	  also be built as a module. If so, the module will be called slcan.
-
 config CAN_NETLINK
 	bool "CAN device drivers with Netlink support"
 	default y
@@ -172,6 +152,26 @@ config CAN_KVASER_PCIEFD
 	    Kvaser Mini PCI Express HS v2
 	    Kvaser Mini PCI Express 2xHS v2
 
+config CAN_SLCAN
+	tristate "Serial / USB serial CAN Adaptors (slcan)"
+	depends on TTY
+	help
+	  CAN driver for several 'low cost' CAN interfaces that are attached
+	  via serial lines or via USB-to-serial adapters using the LAWICEL
+	  ASCII protocol. The driver implements the tty linediscipline N_SLCAN.
+
+	  As only the sending and receiving of CAN frames is implemented, this
+	  driver should work with the (serial/USB) CAN hardware from:
+	  www.canusb.com / www.can232.com / www.mictronics.de / www.canhack.de
+
+	  Userspace tools to attach the SLCAN line discipline (slcan_attach,
+	  slcand) can be found in the can-utils at the linux-can project, see
+	  https://github.com/linux-can/can-utils for details.
+
+	  The slcan driver supports up to 10 CAN netdevices by default which
+	  can be changed by the 'maxdev=xx' module option. This driver can
+	  also be built as a module. If so, the module will be called slcan.
+
 config CAN_SUN4I
 	tristate "Allwinner A10 CAN controller"
 	depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index c39580b142e0..bf84698f1a81 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -56,7 +56,6 @@
 #include <linux/can.h>
 #include <linux/can/dev.h>
 #include <linux/can/skb.h>
-#include <linux/can/can-ml.h>
 
 MODULE_ALIAS_LDISC(N_SLCAN);
 MODULE_DESCRIPTION("serial line CAN interface");
@@ -79,6 +78,7 @@ MODULE_PARM_DESC(maxdev, "Maximum number of slcan interfaces");
 #define SLC_EFF_ID_LEN 8
 
 struct slcan {
+	struct can_priv         can;
 	int			magic;
 
 	/* Various fields. */
@@ -394,6 +394,8 @@ static int slc_close(struct net_device *dev)
 		clear_bit(TTY_DO_WRITE_WAKEUP, &sl->tty->flags);
 	}
 	netif_stop_queue(dev);
+	close_candev(dev);
+	sl->can.state = CAN_STATE_STOPPED;
 	sl->rcount   = 0;
 	sl->xleft    = 0;
 	spin_unlock_bh(&sl->lock);
@@ -405,20 +407,34 @@ static int slc_close(struct net_device *dev)
 static int slc_open(struct net_device *dev)
 {
 	struct slcan *sl = netdev_priv(dev);
+	int err;
 
 	if (sl->tty == NULL)
 		return -ENODEV;
 
+	/* The baud rate is not set with the command
+	 * `ip link set <iface> type can bitrate <baud>' and therefore
+	 * can.bittiming.bitrate is CAN_BITRATE_UNSET (0), causing
+	 * open_candev() to fail. So let's set to a fake value.
+	 */
+	sl->can.bittiming.bitrate = CAN_BITRATE_UNKNOWN;
+	err = open_candev(dev);
+	if (err) {
+		netdev_err(dev, "failed to open can device\n");
+		return err;
+	}
+
+	sl->can.state = CAN_STATE_ERROR_ACTIVE;
 	sl->flags &= BIT(SLF_INUSE);
 	netif_start_queue(dev);
 	return 0;
 }
 
-/* Hook the destructor so we can free slcan devs at the right point in time */
-static void slc_free_netdev(struct net_device *dev)
+static void slc_dealloc(struct slcan *sl)
 {
-	int i = dev->base_addr;
+	int i = sl->dev->base_addr;
 
+	free_candev(sl->dev);
 	slcan_devs[i] = NULL;
 }
 
@@ -434,24 +450,6 @@ static const struct net_device_ops slc_netdev_ops = {
 	.ndo_change_mtu         = slcan_change_mtu,
 };
 
-static void slc_setup(struct net_device *dev)
-{
-	dev->netdev_ops		= &slc_netdev_ops;
-	dev->needs_free_netdev	= true;
-	dev->priv_destructor	= slc_free_netdev;
-
-	dev->hard_header_len	= 0;
-	dev->addr_len		= 0;
-	dev->tx_queue_len	= 10;
-
-	dev->mtu		= CAN_MTU;
-	dev->type		= ARPHRD_CAN;
-
-	/* New-style flags. */
-	dev->flags		= IFF_NOARP;
-	dev->features           = NETIF_F_HW_CSUM;
-}
-
 /******************************************
   Routines looking at TTY side.
  ******************************************/
@@ -514,11 +512,8 @@ static void slc_sync(void)
 static struct slcan *slc_alloc(void)
 {
 	int i;
-	char name[IFNAMSIZ];
 	struct net_device *dev = NULL;
-	struct can_ml_priv *can_ml;
 	struct slcan       *sl;
-	int size;
 
 	for (i = 0; i < maxdev; i++) {
 		dev = slcan_devs[i];
@@ -531,16 +526,14 @@ static struct slcan *slc_alloc(void)
 	if (i >= maxdev)
 		return NULL;
 
-	sprintf(name, "slcan%d", i);
-	size = ALIGN(sizeof(*sl), NETDEV_ALIGN) + sizeof(struct can_ml_priv);
-	dev = alloc_netdev(size, name, NET_NAME_UNKNOWN, slc_setup);
+	dev = alloc_candev(sizeof(*sl), 1);
 	if (!dev)
 		return NULL;
 
+	snprintf(dev->name, sizeof(dev->name), "slcan%d", i);
+	dev->netdev_ops = &slc_netdev_ops;
 	dev->base_addr  = i;
 	sl = netdev_priv(dev);
-	can_ml = (void *)sl + ALIGN(sizeof(*sl), NETDEV_ALIGN);
-	can_set_ml_priv(dev, can_ml);
 
 	/* Initialize channel control data */
 	sl->magic = SLCAN_MAGIC;
@@ -605,26 +598,28 @@ static int slcan_open(struct tty_struct *tty)
 
 		set_bit(SLF_INUSE, &sl->flags);
 
-		err = register_netdevice(sl->dev);
-		if (err)
+		rtnl_unlock();
+		err = register_candev(sl->dev);
+		if (err) {
+			pr_err("slcan: can't register candev\n");
 			goto err_free_chan;
+		}
+	} else {
+		rtnl_unlock();
 	}
 
-	/* Done.  We have linked the TTY line to a channel. */
-	rtnl_unlock();
 	tty->receive_room = 65536;	/* We don't flow control */
 
 	/* TTY layer expects 0 on success */
 	return 0;
 
 err_free_chan:
+	rtnl_lock();
 	sl->tty = NULL;
 	tty->disc_data = NULL;
 	clear_bit(SLF_INUSE, &sl->flags);
-	slc_free_netdev(sl->dev);
-	/* do not call free_netdev before rtnl_unlock */
+	slc_dealloc(sl);
 	rtnl_unlock();
-	free_netdev(sl->dev);
 	return err;
 
 err_exit:
@@ -658,9 +653,11 @@ static void slcan_close(struct tty_struct *tty)
 	synchronize_rcu();
 	flush_work(&sl->tx_work);
 
-	/* Flush network side */
-	unregister_netdev(sl->dev);
-	/* This will complete via sl_free_netdev */
+	slc_close(sl->dev);
+	unregister_candev(sl->dev);
+	rtnl_lock();
+	slc_dealloc(sl);
+	rtnl_unlock();
 }
 
 static void slcan_hangup(struct tty_struct *tty)
@@ -768,14 +765,15 @@ static void __exit slcan_exit(void)
 		dev = slcan_devs[i];
 		if (!dev)
 			continue;
-		slcan_devs[i] = NULL;
 
 		sl = netdev_priv(dev);
 		if (sl->tty) {
 			netdev_err(dev, "tty discipline still running\n");
 		}
 
-		unregister_netdev(dev);
+		slc_close(dev);
+		unregister_candev(dev);
+		slc_dealloc(sl);
 	}
 
 	kfree(slcan_devs);
-- 
2.35.1

[PATCH net-next 09/15] can: slcan: allow to send commands to the adapter

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

This is a preparation patch for the upcoming support to change the
bitrate via ip tool, reset the adapter error states via the ethtool API
and, more generally, send commands to the adapter.

Since the close command (i. e. "C\r") will be sent in the ndo_stop()
where netif_running() returns false, a new flag bit (i. e. SLF_XCMD) for
serial transmission has to be added.

Link: https://lore.kernel.org/all/20220628163137.413025-7-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan.c | 46 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index bf84698f1a81..dfccf8d6c9a5 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -97,6 +97,9 @@ struct slcan {
 	unsigned long		flags;		/* Flag values/ mode etc     */
 #define SLF_INUSE		0		/* Channel in use            */
 #define SLF_ERROR		1               /* Parity, etc. error        */
+#define SLF_XCMD		2               /* Command transmission      */
+	wait_queue_head_t       xcmd_wait;      /* Wait queue for commands   */
+						/* transmission              */
 };
 
 static struct net_device **slcan_devs;
@@ -314,12 +317,22 @@ static void slcan_transmit(struct work_struct *work)
 
 	spin_lock_bh(&sl->lock);
 	/* First make sure we're connected. */
-	if (!sl->tty || sl->magic != SLCAN_MAGIC || !netif_running(sl->dev)) {
+	if (!sl->tty || sl->magic != SLCAN_MAGIC ||
+	    (unlikely(!netif_running(sl->dev)) &&
+	     likely(!test_bit(SLF_XCMD, &sl->flags)))) {
 		spin_unlock_bh(&sl->lock);
 		return;
 	}
 
 	if (sl->xleft <= 0)  {
+		if (unlikely(test_bit(SLF_XCMD, &sl->flags))) {
+			clear_bit(SLF_XCMD, &sl->flags);
+			clear_bit(TTY_DO_WRITE_WAKEUP, &sl->tty->flags);
+			spin_unlock_bh(&sl->lock);
+			wake_up(&sl->xcmd_wait);
+			return;
+		}
+
 		/* Now serial buffer is almost free & we can start
 		 * transmission of another packet */
 		sl->dev->stats.tx_packets++;
@@ -383,6 +396,36 @@ static netdev_tx_t slc_xmit(struct sk_buff *skb, struct net_device *dev)
  *   Routines looking at netdevice side.
  ******************************************/
 
+static int slcan_transmit_cmd(struct slcan *sl, const unsigned char *cmd)
+{
+	int ret, actual, n;
+
+	spin_lock(&sl->lock);
+	if (!sl->tty) {
+		spin_unlock(&sl->lock);
+		return -ENODEV;
+	}
+
+	n = snprintf(sl->xbuff, sizeof(sl->xbuff), "%s", cmd);
+	set_bit(TTY_DO_WRITE_WAKEUP, &sl->tty->flags);
+	actual = sl->tty->ops->write(sl->tty, sl->xbuff, n);
+	sl->xleft = n - actual;
+	sl->xhead = sl->xbuff + actual;
+	set_bit(SLF_XCMD, &sl->flags);
+	spin_unlock(&sl->lock);
+	ret = wait_event_interruptible_timeout(sl->xcmd_wait,
+					       !test_bit(SLF_XCMD, &sl->flags),
+					       HZ);
+	clear_bit(SLF_XCMD, &sl->flags);
+	if (ret == -ERESTARTSYS)
+		return ret;
+
+	if (ret == 0)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
 /* Netdevice UP -> DOWN routine */
 static int slc_close(struct net_device *dev)
 {
@@ -540,6 +583,7 @@ static struct slcan *slc_alloc(void)
 	sl->dev	= dev;
 	spin_lock_init(&sl->lock);
 	INIT_WORK(&sl->tx_work, slcan_transmit);
+	init_waitqueue_head(&sl->xcmd_wait);
 	slcan_devs[i] = dev;
 
 	return sl;
-- 
2.35.1

[PATCH net-next 10/15] can: slcan: set bitrate by CAN device driver API

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

It allows to set the bitrate via ip tool, as it happens for the other
CAN device drivers. It still remains possible to set the bitrate via
slcand or slcan_attach utilities. In case the ip tool is used, the
driver will send the serial command to the adapter.

Link: https://lore.kernel.org/all/20220628163137.413025-8-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan.c | 41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index dfccf8d6c9a5..74033e2d7097 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -104,6 +104,11 @@ struct slcan {
 
 static struct net_device **slcan_devs;
 
+static const u32 slcan_bitrate_const[] = {
+	10000, 20000, 50000, 100000, 125000,
+	250000, 500000, 800000, 1000000
+};
+
  /************************************************************************
   *			SLCAN ENCAPSULATION FORMAT			 *
   ************************************************************************/
@@ -439,6 +444,9 @@ static int slc_close(struct net_device *dev)
 	netif_stop_queue(dev);
 	close_candev(dev);
 	sl->can.state = CAN_STATE_STOPPED;
+	if (sl->can.bittiming.bitrate == CAN_BITRATE_UNKNOWN)
+		sl->can.bittiming.bitrate = CAN_BITRATE_UNSET;
+
 	sl->rcount   = 0;
 	sl->xleft    = 0;
 	spin_unlock_bh(&sl->lock);
@@ -450,7 +458,8 @@ static int slc_close(struct net_device *dev)
 static int slc_open(struct net_device *dev)
 {
 	struct slcan *sl = netdev_priv(dev);
-	int err;
+	unsigned char cmd[SLC_MTU];
+	int err, s;
 
 	if (sl->tty == NULL)
 		return -ENODEV;
@@ -460,15 +469,39 @@ static int slc_open(struct net_device *dev)
 	 * can.bittiming.bitrate is CAN_BITRATE_UNSET (0), causing
 	 * open_candev() to fail. So let's set to a fake value.
 	 */
-	sl->can.bittiming.bitrate = CAN_BITRATE_UNKNOWN;
+	if (sl->can.bittiming.bitrate == CAN_BITRATE_UNSET)
+		sl->can.bittiming.bitrate = CAN_BITRATE_UNKNOWN;
+
 	err = open_candev(dev);
 	if (err) {
 		netdev_err(dev, "failed to open can device\n");
 		return err;
 	}
 
-	sl->can.state = CAN_STATE_ERROR_ACTIVE;
 	sl->flags &= BIT(SLF_INUSE);
+
+	if (sl->can.bittiming.bitrate != CAN_BITRATE_UNKNOWN) {
+		for (s = 0; s < ARRAY_SIZE(slcan_bitrate_const); s++) {
+			if (sl->can.bittiming.bitrate == slcan_bitrate_const[s])
+				break;
+		}
+
+		/* The CAN framework has already validate the bitrate value,
+		 * so we can avoid to check if `s' has been properly set.
+		 */
+
+		snprintf(cmd, sizeof(cmd), "C\rS%d\r", s);
+		err = slcan_transmit_cmd(sl, cmd);
+		if (err) {
+			netdev_err(dev,
+				   "failed to send bitrate command 'C\\rS%d\\r'\n",
+				   s);
+			close_candev(dev);
+			return err;
+		}
+	}
+
+	sl->can.state = CAN_STATE_ERROR_ACTIVE;
 	netif_start_queue(dev);
 	return 0;
 }
@@ -581,6 +614,8 @@ static struct slcan *slc_alloc(void)
 	/* Initialize channel control data */
 	sl->magic = SLCAN_MAGIC;
 	sl->dev	= dev;
+	sl->can.bitrate_const = slcan_bitrate_const;
+	sl->can.bitrate_const_cnt = ARRAY_SIZE(slcan_bitrate_const);
 	spin_lock_init(&sl->lock);
 	INIT_WORK(&sl->tx_work, slcan_transmit);
 	init_waitqueue_head(&sl->xcmd_wait);
-- 
2.35.1

[PATCH net-next 11/15] can: slcan: send the open/close commands to the adapter

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

In case the bitrate has been set via ip tool, this patch changes the
driver to send the open ("O\r") and close ("C\r) commands to the
adapter.

Link: https://lore.kernel.org/all/20220628163137.413025-9-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index 74033e2d7097..249b5ade06fc 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -435,9 +435,20 @@ static int slcan_transmit_cmd(struct slcan *sl, const unsigned char *cmd)
 static int slc_close(struct net_device *dev)
 {
 	struct slcan *sl = netdev_priv(dev);
+	int err;
 
 	spin_lock_bh(&sl->lock);
 	if (sl->tty) {
+		if (sl->can.bittiming.bitrate &&
+		    sl->can.bittiming.bitrate != CAN_BITRATE_UNKNOWN) {
+			spin_unlock_bh(&sl->lock);
+			err = slcan_transmit_cmd(sl, "C\r");
+			spin_lock_bh(&sl->lock);
+			if (err)
+				netdev_warn(dev,
+					    "failed to send close command 'C\\r'\n");
+		}
+
 		/* TTY discipline is running. */
 		clear_bit(TTY_DO_WRITE_WAKEUP, &sl->tty->flags);
 	}
@@ -496,14 +507,23 @@ static int slc_open(struct net_device *dev)
 			netdev_err(dev,
 				   "failed to send bitrate command 'C\\rS%d\\r'\n",
 				   s);
-			close_candev(dev);
-			return err;
+			goto cmd_transmit_failed;
+		}
+
+		err = slcan_transmit_cmd(sl, "O\r");
+		if (err) {
+			netdev_err(dev, "failed to send open command 'O\\r'\n");
+			goto cmd_transmit_failed;
 		}
 	}
 
 	sl->can.state = CAN_STATE_ERROR_ACTIVE;
 	netif_start_queue(dev);
 	return 0;
+
+cmd_transmit_failed:
+	close_candev(dev);
+	return err;
 }
 
 static void slc_dealloc(struct slcan *sl)
-- 
2.35.1

[PATCH net-next 12/15] can: slcan: move driver into separate sub directory

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

This patch moves the slcan driver into a separate directory, a later
patch will add more files.

Link: https://lore.kernel.org/all/20220628163137.413025-10-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/Makefile                        | 2 +-
 drivers/net/can/slcan/Makefile                  | 6 ++++++
 drivers/net/can/{slcan.c => slcan/slcan-core.c} | 0
 3 files changed, 7 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/can/slcan/Makefile
 rename drivers/net/can/{slcan.c => slcan/slcan-core.c} (100%)

diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index cafa0e0a3dfe..61c75ce9d500 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -5,7 +5,7 @@
 
 obj-$(CONFIG_CAN_VCAN)		+= vcan.o
 obj-$(CONFIG_CAN_VXCAN)		+= vxcan.o
-obj-$(CONFIG_CAN_SLCAN)		+= slcan.o
+obj-$(CONFIG_CAN_SLCAN)		+= slcan/
 
 obj-y				+= dev/
 obj-y				+= rcar/
diff --git a/drivers/net/can/slcan/Makefile b/drivers/net/can/slcan/Makefile
new file mode 100644
index 000000000000..2e84f7bf7617
--- /dev/null
+++ b/drivers/net/can/slcan/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_CAN_SLCAN) += slcan.o
+
+slcan-objs :=
+slcan-objs += slcan-core.o
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan/slcan-core.c
similarity index 100%
rename from drivers/net/can/slcan.c
rename to drivers/net/can/slcan/slcan-core.c
-- 
2.35.1

[PATCH net-next 13/15] can: slcan: add ethtool support to reset adapter errors

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

This patch adds a private flag to the slcan driver to switch the
"err-rst-on-open" setting on and off.

"err-rst-on-open" on  - Reset error states on opening command

"err-rst-on-open" off - Don't reset error states on opening command
                        (default)

The setting can only be changed if the interface is down:

    ip link set dev can0 down
    ethtool --set-priv-flags can0 err-rst-on-open {off|on}
    ip link set dev can0 up

Link: https://lore.kernel.org/all/20220628163137.413025-11-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan/Makefile        |  1 +
 drivers/net/can/slcan/slcan-core.c    | 36 +++++++++++++++
 drivers/net/can/slcan/slcan-ethtool.c | 65 +++++++++++++++++++++++++++
 drivers/net/can/slcan/slcan.h         | 18 ++++++++
 4 files changed, 120 insertions(+)
 create mode 100644 drivers/net/can/slcan/slcan-ethtool.c
 create mode 100644 drivers/net/can/slcan/slcan.h

diff --git a/drivers/net/can/slcan/Makefile b/drivers/net/can/slcan/Makefile
index 2e84f7bf7617..8a88e484ee21 100644
--- a/drivers/net/can/slcan/Makefile
+++ b/drivers/net/can/slcan/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_CAN_SLCAN) += slcan.o
 
 slcan-objs :=
 slcan-objs += slcan-core.o
+slcan-objs += slcan-ethtool.o
diff --git a/drivers/net/can/slcan/slcan-core.c b/drivers/net/can/slcan/slcan-core.c
index 249b5ade06fc..c1fd1e934d93 100644
--- a/drivers/net/can/slcan/slcan-core.c
+++ b/drivers/net/can/slcan/slcan-core.c
@@ -57,6 +57,8 @@
 #include <linux/can/dev.h>
 #include <linux/can/skb.h>
 
+#include "slcan.h"
+
 MODULE_ALIAS_LDISC(N_SLCAN);
 MODULE_DESCRIPTION("serial line CAN interface");
 MODULE_LICENSE("GPL");
@@ -98,6 +100,8 @@ struct slcan {
 #define SLF_INUSE		0		/* Channel in use            */
 #define SLF_ERROR		1               /* Parity, etc. error        */
 #define SLF_XCMD		2               /* Command transmission      */
+	unsigned long           cmd_flags;      /* Command flags             */
+#define CF_ERR_RST		0               /* Reset errors on open      */
 	wait_queue_head_t       xcmd_wait;      /* Wait queue for commands   */
 						/* transmission              */
 };
@@ -109,6 +113,28 @@ static const u32 slcan_bitrate_const[] = {
 	250000, 500000, 800000, 1000000
 };
 
+bool slcan_err_rst_on_open(struct net_device *ndev)
+{
+	struct slcan *sl = netdev_priv(ndev);
+
+	return !!test_bit(CF_ERR_RST, &sl->cmd_flags);
+}
+
+int slcan_enable_err_rst_on_open(struct net_device *ndev, bool on)
+{
+	struct slcan *sl = netdev_priv(ndev);
+
+	if (netif_running(ndev))
+		return -EBUSY;
+
+	if (on)
+		set_bit(CF_ERR_RST, &sl->cmd_flags);
+	else
+		clear_bit(CF_ERR_RST, &sl->cmd_flags);
+
+	return 0;
+}
+
  /************************************************************************
   *			SLCAN ENCAPSULATION FORMAT			 *
   ************************************************************************/
@@ -510,6 +536,15 @@ static int slc_open(struct net_device *dev)
 			goto cmd_transmit_failed;
 		}
 
+		if (test_bit(CF_ERR_RST, &sl->cmd_flags)) {
+			err = slcan_transmit_cmd(sl, "F\r");
+			if (err) {
+				netdev_err(dev,
+					   "failed to send error command 'F\\r'\n");
+				goto cmd_transmit_failed;
+			}
+		}
+
 		err = slcan_transmit_cmd(sl, "O\r");
 		if (err) {
 			netdev_err(dev, "failed to send open command 'O\\r'\n");
@@ -629,6 +664,7 @@ static struct slcan *slc_alloc(void)
 	snprintf(dev->name, sizeof(dev->name), "slcan%d", i);
 	dev->netdev_ops = &slc_netdev_ops;
 	dev->base_addr  = i;
+	slcan_set_ethtool_ops(dev);
 	sl = netdev_priv(dev);
 
 	/* Initialize channel control data */
diff --git a/drivers/net/can/slcan/slcan-ethtool.c b/drivers/net/can/slcan/slcan-ethtool.c
new file mode 100644
index 000000000000..bf0afdc4e49d
--- /dev/null
+++ b/drivers/net/can/slcan/slcan-ethtool.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (c) 2022 Amarula Solutions, Dario Binacchi <dario.binacchi@amarulasolutions.com>
+ *
+ */
+
+#include <linux/can/dev.h>
+#include <linux/ethtool.h>
+#include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/platform_device.h>
+
+#include "slcan.h"
+
+static const char slcan_priv_flags_strings[][ETH_GSTRING_LEN] = {
+#define SLCAN_PRIV_FLAGS_ERR_RST_ON_OPEN BIT(0)
+	"err-rst-on-open",
+};
+
+static void slcan_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
+{
+	switch (stringset) {
+	case ETH_SS_PRIV_FLAGS:
+		memcpy(data, slcan_priv_flags_strings,
+		       sizeof(slcan_priv_flags_strings));
+	}
+}
+
+static u32 slcan_get_priv_flags(struct net_device *ndev)
+{
+	u32 flags = 0;
+
+	if (slcan_err_rst_on_open(ndev))
+		flags |= SLCAN_PRIV_FLAGS_ERR_RST_ON_OPEN;
+
+	return flags;
+}
+
+static int slcan_set_priv_flags(struct net_device *ndev, u32 flags)
+{
+	bool err_rst_op_open = !!(flags & SLCAN_PRIV_FLAGS_ERR_RST_ON_OPEN);
+
+	return slcan_enable_err_rst_on_open(ndev, err_rst_op_open);
+}
+
+static int slcan_get_sset_count(struct net_device *netdev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_PRIV_FLAGS:
+		return ARRAY_SIZE(slcan_priv_flags_strings);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static const struct ethtool_ops slcan_ethtool_ops = {
+	.get_strings = slcan_get_strings,
+	.get_priv_flags = slcan_get_priv_flags,
+	.set_priv_flags = slcan_set_priv_flags,
+	.get_sset_count = slcan_get_sset_count,
+};
+
+void slcan_set_ethtool_ops(struct net_device *netdev)
+{
+	netdev->ethtool_ops = &slcan_ethtool_ops;
+}
diff --git a/drivers/net/can/slcan/slcan.h b/drivers/net/can/slcan/slcan.h
new file mode 100644
index 000000000000..d463c8d99e22
--- /dev/null
+++ b/drivers/net/can/slcan/slcan.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * slcan.h - serial line CAN interface driver
+ *
+ * Copyright (C) Laurence Culhane <loz@holmes.demon.co.uk>
+ * Copyright (C) Fred N. van Kempen <waltje@uwalt.nl.mugnet.org>
+ * Copyright (C) Oliver Hartkopp <socketcan@hartkopp.net>
+ * Copyright (C) 2022 Amarula Solutions, Dario Binacchi <dario.binacchi@amarulasolutions.com>
+ *
+ */
+
+#ifndef _SLCAN_H
+#define _SLCAN_H
+
+bool slcan_err_rst_on_open(struct net_device *ndev);
+int slcan_enable_err_rst_on_open(struct net_device *ndev, bool on);
+void slcan_set_ethtool_ops(struct net_device *ndev);
+
+#endif /* _SLCAN_H */
-- 
2.35.1

[PATCH net-next 14/15] can: slcan: extend the protocol with error info

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

It extends the protocol to receive the adapter CAN communication errors
and forward them to the netdev upper levels.

Link: https://lore.kernel.org/all/20220628163137.413025-12-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan/slcan-core.c | 140 ++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/slcan/slcan-core.c b/drivers/net/can/slcan/slcan-core.c
index c1fd1e934d93..4269b2267be2 100644
--- a/drivers/net/can/slcan/slcan-core.c
+++ b/drivers/net/can/slcan/slcan-core.c
@@ -175,7 +175,7 @@ int slcan_enable_err_rst_on_open(struct net_device *ndev, bool on)
   ************************************************************************/
 
 /* Send one completely decapsulated can_frame to the network layer */
-static void slc_bump(struct slcan *sl)
+static void slc_bump_frame(struct slcan *sl)
 {
 	struct sk_buff *skb;
 	struct can_frame *cf;
@@ -254,6 +254,144 @@ static void slc_bump(struct slcan *sl)
 	dev_kfree_skb(skb);
 }
 
+/* An error frame can contain more than one type of error.
+ *
+ * Examples:
+ *
+ * e1a : len 1, errors: ACK error
+ * e3bcO: len 3, errors: Bit0 error, CRC error, Tx overrun error
+ */
+static void slc_bump_err(struct slcan *sl)
+{
+	struct net_device *dev = sl->dev;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	char *cmd = sl->rbuff;
+	bool rx_errors = false, tx_errors = false, rx_over_errors = false;
+	int i, len;
+
+	/* get len from sanitized ASCII value */
+	len = cmd[1];
+	if (len >= '0' && len < '9')
+		len -= '0';
+	else
+		return;
+
+	if ((len + SLC_CMD_LEN + 1) > sl->rcount)
+		return;
+
+	skb = alloc_can_err_skb(dev, &cf);
+
+	if (skb)
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+
+	cmd += SLC_CMD_LEN + 1;
+	for (i = 0; i < len; i++, cmd++) {
+		switch (*cmd) {
+		case 'a':
+			netdev_dbg(dev, "ACK error\n");
+			tx_errors = true;
+			if (skb) {
+				cf->can_id |= CAN_ERR_ACK;
+				cf->data[3] = CAN_ERR_PROT_LOC_ACK;
+			}
+
+			break;
+		case 'b':
+			netdev_dbg(dev, "Bit0 error\n");
+			tx_errors = true;
+			if (skb)
+				cf->data[2] |= CAN_ERR_PROT_BIT0;
+
+			break;
+		case 'B':
+			netdev_dbg(dev, "Bit1 error\n");
+			tx_errors = true;
+			if (skb)
+				cf->data[2] |= CAN_ERR_PROT_BIT1;
+
+			break;
+		case 'c':
+			netdev_dbg(dev, "CRC error\n");
+			rx_errors = true;
+			if (skb) {
+				cf->data[2] |= CAN_ERR_PROT_BIT;
+				cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
+			}
+
+			break;
+		case 'f':
+			netdev_dbg(dev, "Form Error\n");
+			rx_errors = true;
+			if (skb)
+				cf->data[2] |= CAN_ERR_PROT_FORM;
+
+			break;
+		case 'o':
+			netdev_dbg(dev, "Rx overrun error\n");
+			rx_over_errors = true;
+			rx_errors = true;
+			if (skb) {
+				cf->can_id |= CAN_ERR_CRTL;
+				cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+			}
+
+			break;
+		case 'O':
+			netdev_dbg(dev, "Tx overrun error\n");
+			tx_errors = true;
+			if (skb) {
+				cf->can_id |= CAN_ERR_CRTL;
+				cf->data[1] = CAN_ERR_CRTL_TX_OVERFLOW;
+			}
+
+			break;
+		case 's':
+			netdev_dbg(dev, "Stuff error\n");
+			rx_errors = true;
+			if (skb)
+				cf->data[2] |= CAN_ERR_PROT_STUFF;
+
+			break;
+		default:
+			if (skb)
+				dev_kfree_skb(skb);
+
+			return;
+		}
+	}
+
+	if (rx_errors)
+		dev->stats.rx_errors++;
+
+	if (rx_over_errors)
+		dev->stats.rx_over_errors++;
+
+	if (tx_errors)
+		dev->stats.tx_errors++;
+
+	if (skb)
+		netif_rx(skb);
+}
+
+static void slc_bump(struct slcan *sl)
+{
+	switch (sl->rbuff[0]) {
+	case 'r':
+		fallthrough;
+	case 't':
+		fallthrough;
+	case 'R':
+		fallthrough;
+	case 'T':
+		return slc_bump_frame(sl);
+	case 'e':
+		return slc_bump_err(sl);
+	default:
+		return;
+	}
+}
+
 /* parse tty input stream */
 static void slcan_unesc(struct slcan *sl, unsigned char s)
 {
-- 
2.35.1

[PATCH net-next 15/15] can: slcan: extend the protocol with CAN state info

[Marc Kleine-Budde]

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>

It extends the protocol to receive the adapter CAN state changes
(warning, busoff, etc.) and forward them to the netdev upper levels.

Link: https://lore.kernel.org/all/20220628163137.413025-13-dario.binacchi@amarulasolutions.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/slcan/slcan-core.c | 74 +++++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/slcan/slcan-core.c b/drivers/net/can/slcan/slcan-core.c
index 4269b2267be2..54d29a410ad5 100644
--- a/drivers/net/can/slcan/slcan-core.c
+++ b/drivers/net/can/slcan/slcan-core.c
@@ -78,7 +78,11 @@ MODULE_PARM_DESC(maxdev, "Maximum number of slcan interfaces");
 #define SLC_CMD_LEN 1
 #define SLC_SFF_ID_LEN 3
 #define SLC_EFF_ID_LEN 8
-
+#define SLC_STATE_LEN 1
+#define SLC_STATE_BE_RXCNT_LEN 3
+#define SLC_STATE_BE_TXCNT_LEN 3
+#define SLC_STATE_FRAME_LEN       (1 + SLC_CMD_LEN + SLC_STATE_BE_RXCNT_LEN + \
+				   SLC_STATE_BE_TXCNT_LEN)
 struct slcan {
 	struct can_priv         can;
 	int			magic;
@@ -254,6 +258,72 @@ static void slc_bump_frame(struct slcan *sl)
 	dev_kfree_skb(skb);
 }
 
+/* A change state frame must contain state info and receive and transmit
+ * error counters.
+ *
+ * Examples:
+ *
+ * sb256256 : state bus-off: rx counter 256, tx counter 256
+ * sa057033 : state active, rx counter 57, tx counter 33
+ */
+static void slc_bump_state(struct slcan *sl)
+{
+	struct net_device *dev = sl->dev;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	char *cmd = sl->rbuff;
+	u32 rxerr, txerr;
+	enum can_state state, rx_state, tx_state;
+
+	switch (cmd[1]) {
+	case 'a':
+		state = CAN_STATE_ERROR_ACTIVE;
+		break;
+	case 'w':
+		state = CAN_STATE_ERROR_WARNING;
+		break;
+	case 'p':
+		state = CAN_STATE_ERROR_PASSIVE;
+		break;
+	case 'b':
+		state = CAN_STATE_BUS_OFF;
+		break;
+	default:
+		return;
+	}
+
+	if (state == sl->can.state || sl->rcount < SLC_STATE_FRAME_LEN)
+		return;
+
+	cmd += SLC_STATE_BE_RXCNT_LEN + SLC_CMD_LEN + 1;
+	cmd[SLC_STATE_BE_TXCNT_LEN] = 0;
+	if (kstrtou32(cmd, 10, &txerr))
+		return;
+
+	*cmd = 0;
+	cmd -= SLC_STATE_BE_RXCNT_LEN;
+	if (kstrtou32(cmd, 10, &rxerr))
+		return;
+
+	skb = alloc_can_err_skb(dev, &cf);
+	if (skb) {
+		cf->data[6] = txerr;
+		cf->data[7] = rxerr;
+	} else {
+		cf = NULL;
+	}
+
+	tx_state = txerr >= rxerr ? state : 0;
+	rx_state = txerr <= rxerr ? state : 0;
+	can_change_state(dev, cf, tx_state, rx_state);
+
+	if (state == CAN_STATE_BUS_OFF)
+		can_bus_off(dev);
+
+	if (skb)
+		netif_rx(skb);
+}
+
 /* An error frame can contain more than one type of error.
  *
  * Examples:
@@ -387,6 +457,8 @@ static void slc_bump(struct slcan *sl)
 		return slc_bump_frame(sl);
 	case 'e':
 		return slc_bump_err(sl);
+	case 's':
+		return slc_bump_state(sl);
 	default:
 		return;
 	}
-- 
2.35.1

Re: [PATCH net-next 01/15] tty: Add N_CAN327 line discipline ID for ELM327 based CAN driver

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Marc Kleine-Budde <mkl@pengutronix.de>:

On Sun,  3 Jul 2022 12:14:15 +0200 you wrote:
> From: Max Staudt <max@enpas.org>
> 
> The actual driver will be added via the CAN tree.
> 
> Link: https://lore.kernel.org/all/20220618180134.9890-1-max@enpas.org
> Link: https://lore.kernel.org/all/Yrm9Ezlw1dLmIxyS@kroah.com
> Signed-off-by: Max Staudt <max@enpas.org>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> [...]

Here is the summary with links:
  - [net-next,01/15] tty: Add N_CAN327 line discipline ID for ELM327 based CAN driver
    https://git.kernel.org/netdev/net-next/c/713eb3c1261a
  - [net-next,02/15] can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters
    https://git.kernel.org/netdev/net-next/c/43da2f07622f
  - [net-next,03/15] can: ctucanfd: ctucan_interrupt(): fix typo
    https://git.kernel.org/netdev/net-next/c/50f2944009a2
  - [net-next,04/15] can: slcan: use the BIT() helper
    https://git.kernel.org/netdev/net-next/c/3cd864901bc5
  - [net-next,05/15] can: slcan: use netdev helpers to print out messages
    https://git.kernel.org/netdev/net-next/c/da6788ea025c
  - [net-next,06/15] can: slcan: use the alloc_can_skb() helper
    https://git.kernel.org/netdev/net-next/c/92a31782c848
  - [net-next,07/15] can: netlink: dump bitrate 0 if can_priv::bittiming.bitrate is -1U
    https://git.kernel.org/netdev/net-next/c/036bff2800cb
  - [net-next,08/15] can: slcan: use CAN network device driver API
    https://git.kernel.org/netdev/net-next/c/c4e54b063f42
  - [net-next,09/15] can: slcan: allow to send commands to the adapter
    https://git.kernel.org/netdev/net-next/c/52f9ac85b876
  - [net-next,10/15] can: slcan: set bitrate by CAN device driver API
    https://git.kernel.org/netdev/net-next/c/dca796299462
  - [net-next,11/15] can: slcan: send the open/close commands to the adapter
    https://git.kernel.org/netdev/net-next/c/5bac315be7eb
  - [net-next,12/15] can: slcan: move driver into separate sub directory
    https://git.kernel.org/netdev/net-next/c/98b12064591d
  - [net-next,13/15] can: slcan: add ethtool support to reset adapter errors
    https://git.kernel.org/netdev/net-next/c/4de0e8efa052
  - [net-next,14/15] can: slcan: extend the protocol with error info
    https://git.kernel.org/netdev/net-next/c/b32ff4668544
  - [net-next,15/15] can: slcan: extend the protocol with CAN state info
    https://git.kernel.org/netdev/net-next/c/0a9cdcf098a4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 08/15] can: slcan: use CAN network device driver API

[Dario Binacchi]

Hi Marc,

On Sun, Jul 3, 2022 at 12:26 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> From: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>
> As suggested by commit [1], now the driver uses the functions and the
> data structures provided by the CAN network device driver interface.
>
> Currently the driver doesn't implement a way to set bitrate for SLCAN
> based devices via ip tool, so you'll have to do this by slcand or
> slcan_attach invocation through the -sX parameter:
>
> - slcan_attach -f -s6 -o /dev/ttyACM0
> - slcand -f -s8 -o /dev/ttyUSB0
>
> where -s6 in will set adapter's bitrate to 500 Kbit/s and -s8 to
> 1Mbit/s.
> See the table below for further CAN bitrates:
> - s0 ->   10 Kbit/s
> - s1 ->   20 Kbit/s
> - s2 ->   50 Kbit/s
> - s3 ->  100 Kbit/s
> - s4 ->  125 Kbit/s
> - s5 ->  250 Kbit/s
> - s6 ->  500 Kbit/s
> - s7 ->  800 Kbit/s
> - s8 -> 1000 Kbit/s
>
> In doing so, the struct can_priv::bittiming.bitrate of the driver is not
> set and since the open_candev() checks that the bitrate has been set, it
> must be a non-zero value, the bitrate is set to a fake value (-1U)
> before it is called.
>
> Using the rtnl_lock()/rtnl_unlock() functions has become a bit more
> tricky as the register_candev() function indirectly calls rtnl_lock()
> via register_netdev(). To avoid a deadlock it is therefore necessary to
> call rtnl_unlock() before calling register_candev(). The same goes for
> the unregister_candev() function.
>
> [1] commit 39549eef3587f ("can: CAN Network device driver and Netlink interface")
>
> Link: https://lore.kernel.org/all/20220628163137.413025-6-dario.binacchi@amarulasolutions.com
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/can/Kconfig | 40 ++++++++++----------
>  drivers/net/can/slcan.c | 82 ++++++++++++++++++++---------------------
>  2 files changed, 60 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 4078d0775572..3048ad77edb3 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -49,26 +49,6 @@ config CAN_VXCAN
>           This driver can also be built as a module.  If so, the module
>           will be called vxcan.
>
> -config CAN_SLCAN
> -       tristate "Serial / USB serial CAN Adaptors (slcan)"
> -       depends on TTY
> -       help
> -         CAN driver for several 'low cost' CAN interfaces that are attached
> -         via serial lines or via USB-to-serial adapters using the LAWICEL
> -         ASCII protocol. The driver implements the tty linediscipline N_SLCAN.
> -
> -         As only the sending and receiving of CAN frames is implemented, this
> -         driver should work with the (serial/USB) CAN hardware from:
> -         www.canusb.com / www.can232.com / www.mictronics.de / www.canhack.de
> -
> -         Userspace tools to attach the SLCAN line discipline (slcan_attach,
> -         slcand) can be found in the can-utils at the linux-can project, see
> -         https://github.com/linux-can/can-utils for details.
> -
> -         The slcan driver supports up to 10 CAN netdevices by default which
> -         can be changed by the 'maxdev=xx' module option. This driver can
> -         also be built as a module. If so, the module will be called slcan.
> -
>  config CAN_NETLINK
>         bool "CAN device drivers with Netlink support"
>         default y
> @@ -172,6 +152,26 @@ config CAN_KVASER_PCIEFD
>             Kvaser Mini PCI Express HS v2
>             Kvaser Mini PCI Express 2xHS v2
>
> +config CAN_SLCAN
> +       tristate "Serial / USB serial CAN Adaptors (slcan)"
> +       depends on TTY
> +       help
> +         CAN driver for several 'low cost' CAN interfaces that are attached
> +         via serial lines or via USB-to-serial adapters using the LAWICEL
> +         ASCII protocol. The driver implements the tty linediscipline N_SLCAN.
> +
> +         As only the sending and receiving of CAN frames is implemented, this
> +         driver should work with the (serial/USB) CAN hardware from:
> +         www.canusb.com / www.can232.com / www.mictronics.de / www.canhack.de
> +
> +         Userspace tools to attach the SLCAN line discipline (slcan_attach,
> +         slcand) can be found in the can-utils at the linux-can project, see
> +         https://github.com/linux-can/can-utils for details.
> +
> +         The slcan driver supports up to 10 CAN netdevices by default which
> +         can be changed by the 'maxdev=xx' module option. This driver can
> +         also be built as a module. If so, the module will be called slcan.
> +
>  config CAN_SUN4I
>         tristate "Allwinner A10 CAN controller"
>         depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
> diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
> index c39580b142e0..bf84698f1a81 100644
> --- a/drivers/net/can/slcan.c
> +++ b/drivers/net/can/slcan.c
> @@ -56,7 +56,6 @@
>  #include <linux/can.h>
>  #include <linux/can/dev.h>
>  #include <linux/can/skb.h>
> -#include <linux/can/can-ml.h>
>
>  MODULE_ALIAS_LDISC(N_SLCAN);
>  MODULE_DESCRIPTION("serial line CAN interface");
> @@ -79,6 +78,7 @@ MODULE_PARM_DESC(maxdev, "Maximum number of slcan interfaces");
>  #define SLC_EFF_ID_LEN 8
>
>  struct slcan {
> +       struct can_priv         can;
>         int                     magic;
>
>         /* Various fields. */
> @@ -394,6 +394,8 @@ static int slc_close(struct net_device *dev)
>                 clear_bit(TTY_DO_WRITE_WAKEUP, &sl->tty->flags);
>         }
>         netif_stop_queue(dev);
> +       close_candev(dev);
> +       sl->can.state = CAN_STATE_STOPPED;
>         sl->rcount   = 0;
>         sl->xleft    = 0;
>         spin_unlock_bh(&sl->lock);

Maybe better to move the spin_unlock_bh() before calling close_candev()?

Thanks and regards,
Dario

> @@ -405,20 +407,34 @@ static int slc_close(struct net_device *dev)
>  static int slc_open(struct net_device *dev)
>  {
>         struct slcan *sl = netdev_priv(dev);
> +       int err;
>
>         if (sl->tty == NULL)
>                 return -ENODEV;
>
> +       /* The baud rate is not set with the command
> +        * `ip link set <iface> type can bitrate <baud>' and therefore
> +        * can.bittiming.bitrate is CAN_BITRATE_UNSET (0), causing
> +        * open_candev() to fail. So let's set to a fake value.
> +        */
> +       sl->can.bittiming.bitrate = CAN_BITRATE_UNKNOWN;
> +       err = open_candev(dev);
> +       if (err) {
> +               netdev_err(dev, "failed to open can device\n");
> +               return err;
> +       }
> +
> +       sl->can.state = CAN_STATE_ERROR_ACTIVE;
>         sl->flags &= BIT(SLF_INUSE);
>         netif_start_queue(dev);
>         return 0;
>  }
>
> -/* Hook the destructor so we can free slcan devs at the right point in time */
> -static void slc_free_netdev(struct net_device *dev)
> +static void slc_dealloc(struct slcan *sl)
>  {
> -       int i = dev->base_addr;
> +       int i = sl->dev->base_addr;
>
> +       free_candev(sl->dev);
>         slcan_devs[i] = NULL;
>  }
>
> @@ -434,24 +450,6 @@ static const struct net_device_ops slc_netdev_ops = {
>         .ndo_change_mtu         = slcan_change_mtu,
>  };
>
> -static void slc_setup(struct net_device *dev)
> -{
> -       dev->netdev_ops         = &slc_netdev_ops;
> -       dev->needs_free_netdev  = true;
> -       dev->priv_destructor    = slc_free_netdev;
> -
> -       dev->hard_header_len    = 0;
> -       dev->addr_len           = 0;
> -       dev->tx_queue_len       = 10;
> -
> -       dev->mtu                = CAN_MTU;
> -       dev->type               = ARPHRD_CAN;
> -
> -       /* New-style flags. */
> -       dev->flags              = IFF_NOARP;
> -       dev->features           = NETIF_F_HW_CSUM;
> -}
> -
>  /******************************************
>    Routines looking at TTY side.
>   ******************************************/
> @@ -514,11 +512,8 @@ static void slc_sync(void)
>  static struct slcan *slc_alloc(void)
>  {
>         int i;
> -       char name[IFNAMSIZ];
>         struct net_device *dev = NULL;
> -       struct can_ml_priv *can_ml;
>         struct slcan       *sl;
> -       int size;
>
>         for (i = 0; i < maxdev; i++) {
>                 dev = slcan_devs[i];
> @@ -531,16 +526,14 @@ static struct slcan *slc_alloc(void)
>         if (i >= maxdev)
>                 return NULL;
>
> -       sprintf(name, "slcan%d", i);
> -       size = ALIGN(sizeof(*sl), NETDEV_ALIGN) + sizeof(struct can_ml_priv);
> -       dev = alloc_netdev(size, name, NET_NAME_UNKNOWN, slc_setup);
> +       dev = alloc_candev(sizeof(*sl), 1);
>         if (!dev)
>                 return NULL;
>
> +       snprintf(dev->name, sizeof(dev->name), "slcan%d", i);
> +       dev->netdev_ops = &slc_netdev_ops;
>         dev->base_addr  = i;
>         sl = netdev_priv(dev);
> -       can_ml = (void *)sl + ALIGN(sizeof(*sl), NETDEV_ALIGN);
> -       can_set_ml_priv(dev, can_ml);
>
>         /* Initialize channel control data */
>         sl->magic = SLCAN_MAGIC;
> @@ -605,26 +598,28 @@ static int slcan_open(struct tty_struct *tty)
>
>                 set_bit(SLF_INUSE, &sl->flags);
>
> -               err = register_netdevice(sl->dev);
> -               if (err)
> +               rtnl_unlock();
> +               err = register_candev(sl->dev);
> +               if (err) {
> +                       pr_err("slcan: can't register candev\n");
>                         goto err_free_chan;
> +               }
> +       } else {
> +               rtnl_unlock();
>         }
>
> -       /* Done.  We have linked the TTY line to a channel. */
> -       rtnl_unlock();
>         tty->receive_room = 65536;      /* We don't flow control */
>
>         /* TTY layer expects 0 on success */
>         return 0;
>
>  err_free_chan:
> +       rtnl_lock();
>         sl->tty = NULL;
>         tty->disc_data = NULL;
>         clear_bit(SLF_INUSE, &sl->flags);
> -       slc_free_netdev(sl->dev);
> -       /* do not call free_netdev before rtnl_unlock */
> +       slc_dealloc(sl);
>         rtnl_unlock();
> -       free_netdev(sl->dev);
>         return err;
>
>  err_exit:
> @@ -658,9 +653,11 @@ static void slcan_close(struct tty_struct *tty)
>         synchronize_rcu();
>         flush_work(&sl->tx_work);
>
> -       /* Flush network side */
> -       unregister_netdev(sl->dev);
> -       /* This will complete via sl_free_netdev */
> +       slc_close(sl->dev);
> +       unregister_candev(sl->dev);
> +       rtnl_lock();
> +       slc_dealloc(sl);
> +       rtnl_unlock();
>  }
>
>  static void slcan_hangup(struct tty_struct *tty)
> @@ -768,14 +765,15 @@ static void __exit slcan_exit(void)
>                 dev = slcan_devs[i];
>                 if (!dev)
>                         continue;
> -               slcan_devs[i] = NULL;
>
>                 sl = netdev_priv(dev);
>                 if (sl->tty) {
>                         netdev_err(dev, "tty discipline still running\n");
>                 }
>
> -               unregister_netdev(dev);
> +               slc_close(dev);
> +               unregister_candev(dev);
> +               slc_dealloc(sl);
>         }
>
>         kfree(slcan_devs);
> --
> 2.35.1
>
>


-- 

Dario Binacchi

Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com

www.amarulasolutions.com