Re: [RFC PATCH 0/5] can: slcan: extend supported features (step 2)

From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: linux-can <linux-can@vger.kernel.org>
Subject: Re: [RFC PATCH 0/5] can: slcan: extend supported features (step 2)
Date: Mon, 18 Jul 2022 09:15:55 +0200	[thread overview]
Message-ID: <aa5a6196-7e0a-25a8-c967-415857741bb2@hartkopp.net> (raw)
In-Reply-To: <CABGWkvokcOW2G+CAH4Lna_pRa1ck-zKRpSifjGj-VX_cb9xP5A@mail.gmail.com>

On 18.07.22 08:59, Dario Binacchi wrote:
> Hello Oliver,
> 
> On Sun, Jul 17, 2022 at 4:13 PM Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>>
>> Hello Dario,
>>
>> I have been maintaining the slcan.c driver for a long time and I'm still
>> in the MODULE_AUTHOR() macro.
>>
>> As you are very committed to the slcan driver and its extensions it
>> probably makes sense to take over the maintainer-ship and add yourself
>> to the MODULE_AUTHOR() macro.
>>
>> Analogue to the CAN FD driver from the CTU you could also provide a
>> similar patch for the MAINTAINER file:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=cfdb2f365cb9d
>>
> 
> I am happy and honored. I hope I am adequate for the task.

I think so ;-)

And with all your new enhancements and clean-ups that get away from the 
slow-moving slip.c I would be happy to get out of the line of fire when 
something breaks :-D

Many thanks!

Oliver

> Thanks and regards,
> 
> Dario
> 
>> Best regards,
>> Oliver
>>
>>
>> On 16.07.22 19:00, Dario Binacchi wrote:
>>> With this series I try to finish the task, started with the series [1],
>>> of completely removing the dependency of the slcan driver from the
>>> userspace slcand/slcan_attach applications.
>>>
>>> The series, however, still lacks a patch for sending the bitrate setting
>>> command to the adapter:
>>>
>>> slcan_attach -b <btr> <dev>
>>>
>>> Without at least this patch the task cannot be considered truly completed.
>>>
>>> The idea I got is that this can only happen through the ethtool API.
>>> Among the various operations made available by this interface I would
>>> have used the set_regs (but only the get_regs has been developed), or,
>>> the set_eeprom, even if the setting would not be stored in an eeprom.
>>> IMHO it would take a set_regs operation with a `struct ethtool_wregs'
>>> parameter similar to `struct ethtool_eeprom' without the magic field:
>>>
>>> struct ethtool_wregs {
>>>        __u32   cmd;
>>>        __u32   offset;
>>>        __u32   len;
>>>        __u8    data[0];
>>> };
>>>
>>> But I am not the expert and if there was an alternative solution already
>>> usable, it would be welcome.
>>>
>>> The series also contains patches that remove the legacy stuff (slcan_devs,
>>> SLCAN_MAGIC, ...) and do some module cleanup.
>>>
>>> The series has been created on top of the patches:
>>>
>>> can: slcan: convert comments to network style comments
>>> can: slcan: slcan_init() convert printk(LEVEL ...) to pr_level()
>>> can: slcan: fix whitespace issues
>>> can: slcan: convert comparison to NULL into !val
>>> can: slcan: clean up if/else
>>> can: slcan: use scnprintf() as a hardening measure
>>> can: slcan: do not report txerr and rxerr during bus-off
>>> can: slcan: do not sleep with a spin lock held
>>>
>>> applied to linux-next.
>>>
>>> [1] https://lore.kernel.org/all/20220628163137.413025-1-dario.binacchi@amarulasolutions.com/
>>>
>>>
>>> Dario Binacchi (5):
>>>     can: slcan: remove useless header inclusions
>>>     can: slcan: remove legacy infrastructure
>>>     can: slcan: change every `slc' occurrence in `slcan'
>>>     can: slcan: use the generic can_change_mtu()
>>>     can: slcan: send the listen-only command to the adapter
>>>
>>>    drivers/net/can/slcan/slcan-core.c | 465 +++++++++--------------------
>>>    1 file changed, 134 insertions(+), 331 deletions(-)
>>>
> 
> 
> 