Re: [PATCH v5 0/7] can: refactoring of can-dev module and of Kbuild

From: Vincent MAILHOL
Date: Tue Jun 07 2022 - 22:07:54 EST


On Wed. 8 Jun 2022 at 05:51, Oliver Hartkopp <socketcan@xxxxxxxxxxxx> wrote:
> On 07.06.22 22:27, Marc Kleine-Budde wrote:
> > On 07.06.2022 22:12:46, Oliver Hartkopp wrote:
> >> So what about:
> >>
> >> symbol: CONFIG_NETDEVICES
> >> |
> >> +-> CAN Device Drivers
> >> symbol: CONFIG_CAN_DEV
> >> |
> >> +-> software/virtual CAN device drivers
> >> | (at time of writing: slcan, vcan, vxcan)
> >> |
> >> +-> hardware CAN device drivers with Netlink support
> >> symbol: CONFIG_CAN_NETLINK (matches previous CONFIG_CAN_DEV)
> >> |
> >> +-> CAN bit-timing calculation (optional for all drivers)
> >> | symbol: CONFIG_CAN_BITTIMING
> >> |
> >> +-> CAN rx offload (optional but selected by some drivers)
> >> | symbol: CONFIG_CAN_RX_OFFLOAD
> >> |
> >> +-> CAN devices drivers
> >> (some may select CONFIG_CAN_RX_OFFLOAD)

OK, this does not follow the definition I set for the "x --> y" arrow,
but it is easy to read. I am OK with your suggestion. I will also
remove the definition of the "x --> y" arrow because your diagram is
self explanatory.

> >> (I also added 'hardware' to CAN device drivers with Netlink support) to have
> >> a distinction to 'software/virtual' CAN device drivers)

This line you modified is the verbatim copy of the title in
menuconfig. So you are suggesting adding "hardware" to the menuconfig
as well? It did not have this word in the title before this series.
I was hesitating on this. If we name the symbol CAN_NETLINK, then I do
not see the need to also add "hardware" in the title. If you look at
the help menu, you will see: "This is required by all platform and
hardware CAN drivers." Mentioning it in the help menu is enough for
me.

And because of the blur line between slcan (c.f. Marc's comment
below), I am not convinced to add this.

> > The line between hardware and software/virtual devices ist blurry, the
> > new can327 driver uses netlink and the slcan is currently being
> > converted....
>
> Right, which could mean that slcan and can327 should be located in the
> 'usual' CAN device driver section and not in the sw/virtual device section.

ACK, but as discussed with Marc, I will just focus on the series
itself and ignore (for the moment) that slcan will probably be moved
within CAN_NETLINK scope in the future.
https://lore.kernel.org/linux-can/20220607103923.5m6j4rykvitofsv4@xxxxxxxxxxxxxx/

> The slcan and can327 need some kind of hardware - while vcan and vxcan
> don't.


Yours sincerely,
Vincent Mailhol