Re: [PATCH v5 00/12] can: slcan: extend supported features

From: Dario Binacchi <dario.binacchi@amarulasolutions.com>
To: Jeroen Hofstee <jhofstee@victronenergy.com>
Cc: michael@amarulasolutions.com,
	Amarula patchwork <linux-amarula@amarulasolutions.com>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jakub Kicinski <kuba@kernel.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	Paolo Abeni <pabeni@redhat.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
	Wolfgang Grandegger <wg@grandegger.com>,
	linux-can@vger.kernel.org
Subject: Re: [PATCH v5 00/12] can: slcan: extend supported features
Date: Fri, 1 Jul 2022 19:02:08 +0200	[thread overview]
Message-ID: <CABGWkvpMsFBOAXx+ihH8s8=d8Z4-MqRSPWX_eWSd9F+PH4TKww@mail.gmail.com> (raw)
In-Reply-To: <cc2e57b8-5b29-11b8-aa7f-fea7ac96d68a@victronenergy.com>

Hi Jeroen,

On Fri, Jul 1, 2022 at 6:36 PM Jeroen Hofstee
<jhofstee@victronenergy.com> wrote:
>
> Hello Dario,
>
> On 6/28/22 18:31, Dario Binacchi 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.
> >
> > 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.
>
> Many thanks,
>
> For what it is worth, up to "Send the open/close command to the adapter
> from the driver.", I cherry-picked this back to 5.10. That seems to work.
>

For me it is meaningful feedback.
Thank you very much

With kind regards,
Dario

> With kind regards,
>
> Jeroen
>
> [1]
> https://github.com/victronenergy/linux/compare/v5.10.109-venus-5...victronenergy:linux:v5.10.109-venus-6?expand=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