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 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.
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
drivers/net/can/Kconfig | 40 +-
drivers/net/can/Makefile | 2 +-
drivers/net/can/dev/netlink.c | 3 +-
drivers/net/can/slcan/Makefile | 7 +
.../net/can/{slcan.c => slcan/slcan-core.c} | 527 ++++++++++++++----
drivers/net/can/slcan/slcan-ethtool.c | 65 +++
drivers/net/can/slcan/slcan.h | 18 +
include/linux/can/bittiming.h | 2 +
8 files changed, 549 insertions(+), 115 deletions(-)
create mode 100644 drivers/net/can/slcan/Makefile
rename drivers/net/can/{slcan.c => slcan/slcan-core.c} (64%)
create mode 100644 drivers/net/can/slcan/slcan-ethtool.c
create mode 100644 drivers/net/can/slcan/slcan.h
--
2.32.0
Replace printk() calls with corresponding netdev helpers.
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v1)
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.32.0
It is used successfully by most (if not all) CAN device drivers. It
allows to remove replicated code.
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v3)
Changes in v3:
- Increment the error counter in case of decoding failure.
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().
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.32.0
It extends the protocol to receive the adapter CAN state changes
(warning, busoff, etc.) and forward them to the netdev upper levels.
Signed-off-by: Dario Binacchi <[email protected]>
---
Changes in v4:
- 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:
- 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:
- Continue error handling even if no skb can be allocated.
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 6c7c815eaf45..e2d7645ff8d2 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;
@@ -255,6 +259,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:
@@ -388,6 +458,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.32.0
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.
Signed-off-by: Dario Binacchi <[email protected]>
---
Changes in v4:
- Squashed to the patch [v3,09/13] can: slcan: send the close command to the adapter.
- Use the CAN_BITRATE_UNKNOWN macro.
Changes in v2:
- Improve the commit message.
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 bd3cf53246c7..b08e63f59b8e 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -436,9 +436,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);
}
@@ -497,14 +508,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.32.0
It extends the protocol to receive the adapter CAN communication errors
and forward them to the netdev upper levels.
Signed-off-by: Dario Binacchi <[email protected]>
---
Changes in v4:
- 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.
Changes in v2:
- Protect decoding against the case the len value is longer than the
received data.
- Continue error handling even if no skb can be allocated.
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 b4f29ab2ab72..6c7c815eaf45 100644
--- a/drivers/net/can/slcan/slcan-core.c
+++ b/drivers/net/can/slcan/slcan-core.c
@@ -176,7 +176,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;
@@ -255,6 +255,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.32.0
This patch moves the slcan driver into a separate directory, a later
patch will add more files.
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v1)
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 0af85983634c..210354df273c 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.32.0
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.
Signed-off-by: Dario Binacchi <[email protected]>
---
Changes in v4:
- Replace `sl->tty == NULL' with `!sl->tty'.
Changes in v3:
- Update the commit description.
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 c7ff11dd2278..2afddaf62586 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;
@@ -315,12 +318,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++;
@@ -384,6 +397,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)
{
@@ -541,6 +584,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.32.0
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
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v1)
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 b08e63f59b8e..b4f29ab2ab72 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 */
};
@@ -110,6 +114,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 *
************************************************************************/
@@ -511,6 +537,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");
@@ -630,6 +665,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 <[email protected]>
+ *
+ */
+
+#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 <[email protected]>
+ * Copyright (C) Fred N. van Kempen <[email protected]>
+ * Copyright (C) Oliver Hartkopp <[email protected]>
+ * Copyright (C) 2022 Amarula Solutions, Dario Binacchi <[email protected]>
+ *
+ */
+
+#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.32.0
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.
Suggested-by: Marc Kleine-Budde <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>
---
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.
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 7633d98e3912..5427712fcf80 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -509,7 +509,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.32.0