On Tue. 7 juin 2022 at 18:47, Dario Binacchi <dario.binacchi@xxxxxxxxxxxxxxxxxxxx> wrote: > 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. That's funny! Yesterday, I sent this comment: https://lore.kernel.org/linux-can/CAMZ6RqKZwC_OKcgH+WPacY6kbNbj4xR2Gdg2NQtm5Ka5Hfw79A@xxxxxxxxxxxxxx/ And today, you send a full series to remove all the dust from the slcan driver. Do I have some kind of mystical power to summon people on the mailing list? > Starting from these assumptions, I tried to: > - Use the CAN network device driver interface. In order to use the CAN network device driver, a.k.a. can-dev module, drivers/net/can/Kbuild has to be adjusted: move slcan inside CAN_DEV scope. @Mark: because I will have to send a new version for my can-dev/Kbuild cleanup, maybe I can take that change and add it to my series? > - 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. 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. > > > > Dario Binacchi (13): > can: slcan: use the BIT() helper > can: slcan: use netdev helpers to print out messages > can: slcan: use the alloc_can_skb() helper > can: slcan: use CAN network device driver API > can: slcan: simplify the device de-allocation > can: slcan: allow to send commands to the adapter > can: slcan: set bitrate by CAN device driver API > can: slcan: send the open command to the adapter > can: slcan: send the close command 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/Makefile | 2 +- > drivers/net/can/slcan/Makefile | 7 + > .../net/can/{slcan.c => slcan/slcan-core.c} | 464 +++++++++++++++--- > drivers/net/can/slcan/slcan-ethtool.c | 65 +++ > drivers/net/can/slcan/slcan.h | 18 + > 5 files changed, 480 insertions(+), 76 deletions(-) > create mode 100644 drivers/net/can/slcan/Makefile > rename drivers/net/can/{slcan.c => slcan/slcan-core.c} (67%) > create mode 100644 drivers/net/can/slcan/slcan-ethtool.c > create mode 100644 drivers/net/can/slcan/slcan.h
