Message-Id: <20220603102848.17907-1-mailhol.vincent@wanadoo.fr>
Date:   Fri,  3 Jun 2022 19:28:41 +0900
From:   Vincent Mailhol <mailhol.vincent@...adoo.fr>
To:     Marc Kleine-Budde <mkl@...gutronix.de>
Cc:     linux-can@...r.kernel.org, linux-kernel@...r.kernel.org,
        Max Staudt <max@...as.org>,
        Oliver Hartkopp <socketcan@...tkopp.net>,
        netdev@...r.kernel.org,
        Vincent Mailhol <mailhol.vincent@...adoo.fr>
Subject: [PATCH v4 0/7] can: refactoring of can-dev module and of Kbuild

Aside of calc_bittiming.o which can be configured with
CAN_CALC_BITTIMING, all objects from drivers/net/can/dev/ get linked
unconditionally to can-dev.o even if not needed by the user.

This series first goal it to split the can-dev modules so that the
user can decide which features get built in during
compilation. Additionally, the CAN Device Drivers menu is moved from
the "Networking support" category to the "Device Drivers" category
(where all drivers are supposed to be).

Below diagrams illustrate the changes made.
The arrow symbol "x --> y" denotes that "y depends on x".

* menu before this series *

CAN bus subsystem support
  symbol: CONFIG_CAN
  |
  +-> CAN Device Drivers
      (no symbol)
      |
      +-> software/virtual CAN device drivers
      |   (at time of writing: slcan, vcan, vxcan)
      |
      +-> Platform CAN drivers with Netlink support
          symbol: CONFIG_CAN_DEV
	  |
          +-> CAN bit-timing calculation  (optional for hardware drivers)
          |   symbol: CONFIG_CAN_BITTIMING
	  |
	  +-> All other CAN devices

* menu after this series *

Network device support
  symbol: CONFIG_NETDEVICES
  |
  +-> CAN Device Drivers
      symbol: CONFIG_CAN_DEV
      |
      +-> software/virtual CAN device drivers
      |   (at time of writing: slcan, vcan, vxcan)
      |
      +-> 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
	  |
	  +-> All other CAN devices not relying on RX offload
          |
          +-> CAN rx offload
              symbol: CONFIG_CAN_RX_OFFLOAD
              |
              +-> CAN devices relying on rx offload
                  (at time of writing: flexcan, ti_hecc and mcp251xfd)

Patches 1 to 5 of this series do above modification.

The last two patches add a check toward CAN_CTRLMODE_LISTENONLY in
can_dropped_invalid_skb() to discard tx skb (such skb can potentially
reach the driver if injected via the packet socket). In more details,
patch 6 moves can_dropped_invalid_skb() from skb.h to skb.o and patch
7 is the actual change.

Those last two patches are actually connected to the first five ones:
because slcan and v(x)can requires can_dropped_invalid_skb(), it was
necessary to add those three devices to the scope of can-dev before
moving the function to skb.o.


** N.B. **

This design results from the lengthy discussion in [1].

I did one change from Oliver's suggestions in [2]. The initial idea
was that the first two config symbols should be respectively
CAN_DEV_SW and CAN_DEV instead of CAN_DEV and CAN_NETLINK as proposed
in this series.

  * First symbol is changed from CAN_DEV_SW to CAN_DEV. The rationale
    is that it is this entry that will trigger the build of can-dev.o
    and it makes more sense for me to name the symbol share the same
    name as the module. Furthermore, this allows to create a menuentry
    with an explicit name that will cover both the virtual and
    physical devices (naming the menuentry "CAN Device Software" would
    be inconsistent with the fact that physical devices would also be
    present in a sub menu). And not using menuentry complexifies the
    menu.

  * Second symbol is renamed from CAN_DEV to CAN_NETLINK because
    CAN_DEV is now taken by the previous menuconfig and netlink is the
    predominant feature added at this level. I am opened to other
    naming suggestion (CAN_DEV_NETLINK, CAN_DEV_HW...?).

[1] https://lore.kernel.org/linux-can/20220514141650.1109542-1-mailhol.vincent@wanadoo.fr/
[2] https://lore.kernel.org/linux-can/22590a57-c7c6-39c6-06d5-11c6e4e1534b@hartkopp.net/


** Changelog **

v3 -> v4:

  * Five additional patches added to split can-dev module and refactor
    Kbuild. c.f. below (lengthy) thread:
    https://lore.kernel.org/linux-can/20220514141650.1109542-1-mailhol.vincent@wanadoo.fr/


v2 -> v3:

  * Apply can_dropped_invalid_skb() to slcan.

  * Make vcan, vxcan and slcan dependent of CONFIG_CAN_DEV by
    modifying Kbuild.

  * fix small typos.

v1 -> v2:

  * move can_dropped_invalid_skb() to skb.c instead of dev.h

  * also move can_skb_headroom_valid() to skb.c

Vincent Mailhol (7):
  can: Kbuild: rename config symbol CAN_DEV into CAN_NETLINK
  can: Kconfig: turn menu "CAN Device Drivers" into a menuconfig using
    CAN_DEV
  can: bittiming: move bittiming calculation functions to
    calc_bittiming.c
  can: Kconfig: add CONFIG_CAN_RX_OFFLOAD
  net: Kconfig: move the CAN device menu to the "Device Drivers" section
  can: skb: move can_dropped_invalid_skb() and can_skb_headroom_valid()
    to skb.c
  can: skb: drop tx skb if in listen only mode

 drivers/net/Kconfig                   |   2 +
 drivers/net/can/Kconfig               |  66 +++++++--
 drivers/net/can/dev/Makefile          |  20 ++-
 drivers/net/can/dev/bittiming.c       | 197 -------------------------
 drivers/net/can/dev/calc_bittiming.c  | 202 ++++++++++++++++++++++++++
 drivers/net/can/dev/dev.c             |   9 +-
 drivers/net/can/dev/skb.c             |  72 +++++++++
 drivers/net/can/spi/mcp251xfd/Kconfig |   1 +
 include/linux/can/skb.h               |  59 +-------
 net/can/Kconfig                       |   5 +-
 10 files changed, 351 insertions(+), 282 deletions(-)
 create mode 100644 drivers/net/can/dev/calc_bittiming.c

-- 
2.35.1

Powered by blists - more mailing lists