Commit 0ebd5529 authored by Marc Kleine-Budde's avatar Marc Kleine-Budde

Merge branch 'can-slcan-extend-supported-features'

Dario Binacchi says:
====================
This series originated as a result of CAN communication tests for an
application using the USBtin adapter (https://www.fischl.de/usbtin/).
The tests showed some errors but for the driver everything was ok.
Also, being the first time I used the slcan driver, I was amazed that
it was not possible to configure the bitrate via the ip tool.
For these two reasons, I started looking at the driver code and realized
that it didn't use the CAN network device driver interface.

Starting from these assumptions, I tried to:
- Use the CAN network device driver interface.
- Set the bitrate via the ip tool.
- Send the open/close command to the adapter from the driver.
- Add ethtool support to reset the adapter errors.
- Extend the protocol to forward the adapter CAN communication
  errors and the CAN state changes to the netdev upper layers.

Except for the protocol extension patches (i. e. forward the adapter CAN
communication errors and the CAN state changes to the netdev upper
layers), the whole series has been tested under QEMU with Linux 4.19.208
using the USBtin adapter.
Testing the extension protocol patches requires updating the adapter
firmware. Before modifying the firmware I think it makes sense to know if
these extensions can be considered useful.

Before applying the series I used these commands:

slcan_attach -f -s6 -o /dev/ttyACM0
slcand ttyACM0 can0
ip link set can0 up

After applying the series I am using these commands:

slcan_attach /dev/ttyACM0
slcand ttyACM0 can0
ip link set dev can0 down
ip link set can0 type can bitrate 500000
ethtool --set-priv-flags can0 err-rst-on-open on
ip link set dev can0 up

Now there is a clearer separation between serial line and CAN,
but above all, it is possible to use the ip and ethtool commands
as it happens for any CAN device driver. The changes are backward
compatible, you can continue to use the slcand and slcan_attach
command options.

Changes in v5:
- Update the commit message.
- Restore the use of rtnl_lock() and rtnl_unlock().

Changes in v4:
- Move the patch in front of the patch "[v3,04/13] can: slcan: use CAN network device driver API".
- Add the CAN_BITRATE_UNSET (0) and CAN_BITRATE_UNKNOWN (-1U) macros.
- Simplify the bitrate check to dump it.
- Update the commit description.
- Update the commit description.
- Use the CAN_BITRATE_UNKNOWN macro.
- Use kfree_skb() instead of can_put_echo_skb() in the slc_xmit().
- Remove the `if (slcan_devs)' check in the slc_dealloc().
- Replace `sl->tty == NULL' with `!sl->tty'.
- Use CAN_BITRATE_UNSET (0) and CAN_BITRATE_UNKNOWN (-1U) macros.
- Don't reset the bitrate in ndo_stop() if it has been configured.
- Squashed to the patch [v3,09/13] can: slcan: send the close command to the adapter.
- Use the CAN_BITRATE_UNKNOWN macro.
- Add description of slc_bump_err() function.
- Remove check for the 'e' character at the beggining of the function.
  It was already checked by the caller function.
- Protect decoding against the case the len value is longer than the
  received data.
- Some small changes to make the decoding more readable.
- Increment all the error counters at the end of the function.
- Add description of slc_bump_state() function.
- Remove check for the 's' character at the beggining of the function.
  It was already checked by the caller function.
- Protect decoding against the case the frame len is longer than the
  received data (add SLC_STATE_FRAME_LEN macro).
- Set cf to NULL in case of alloc_can_err_skb() failure.
- Some small changes to make the decoding more readable.
- Use the character 'b' instead of 'f' for bus-off state.

Changes in v3:
- Increment the error counter in case of decoding failure.
- Replace (-1) with (-1U) in the commit description.
- Update the commit description.
- Remove the slc_do_set_bittiming().
- Set the bitrate in the ndo_open().
- Replace -1UL with -1U in setting a fake value for the bitrate.
- Drop the patch "can: slcan: simplify the device de-allocation".
- Add the patch "can: netlink: dump bitrate 0 if can_priv::bittiming.bitrate is -1U".

Changes in v2:
- Put the data into the allocated skb directly instead of first
  filling the "cf" on the stack and then doing a memcpy().
- Move CAN_SLCAN Kconfig option inside CAN_DEV scope.
- Improve the commit message.
- Use the CAN framework support for setting fixed bit rates.
- Improve the commit message.
- Protect decoding against the case the len value is longer than the
  received data.
- Continue error handling even if no skb can be allocated.
- Continue error handling even if no skb can be allocated.
====================

Link: https://lore.kernel.org/all/20220628163137.413025-1-dario.binacchi@amarulasolutions.com/Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parents 50f29440 0a9cdcf0
......@@ -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
......
......@@ -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/
......
......@@ -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)) ||
......
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_CAN_SLCAN) += slcan.o
slcan-objs :=
slcan-objs += slcan-core.o
slcan-objs += slcan-ethtool.o
// 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;
}
/* 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 */
......@@ -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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment