drivers/net/can/dev/calc_bittiming.c | 10 +- drivers/net/can/dev/dev.c | 33 ++ drivers/net/can/dev/netlink.c | 607 ++++++++++++++++++++++------------- include/linux/can/bittiming.h | 48 ++- include/linux/can/dev.h | 42 +-- include/uapi/linux/can/netlink.h | 14 +- 6 files changed, 471 insertions(+), 283 deletions(-)
In November last year, I sent an RFC to introduce CAN XL [1]. That
RFC, despite positive feedback, was put on hold due to some unanswered
question concerning the PWM encoding [2].
While stuck, some small preparation work was done in parallel in [3]
by refactoring the struct can_priv and doing some trivial clean-up and
renaming. [3] received zero feedback but was eventually merged after
splitting it in smaller parts and resending it.
Finally, in July this year, we clarified the remaining mysteries about
PWM calculation, thus unlocking the series. Summer being a bit busy
because of some personal matters brings us to now.
After doing all the refactoring and adding all the CAN XL features,
the final result is roughly 30 patches, probably too much for a single
series. So I am splitting it in two:
- preparation (this series)
- CAN XL
And so, this series continues and finishes the preparation work done
in [3]. It contains all the refactoring needed to smoothly introduce
CAN XL. The goal is to:
- split the function in smaller pieces: CAN XL will introduce a fair
amount of code. And some functions which are already fairly long
(86 lines for can_validate(), 216 lines for can_changelink())
would grow to disproportionate sizes if the CAN XL logic were to
be inlined in those functions.
- repurpose the existing code to handle both CAN FD and CAN XL: a
huge part of CAN XL simply reuses the CAN FD logic. All the
existing CAN FD logic is made more generic to handle both CAN FD
and XL.
In more details:
- Patch #1 moves struct data_bittiming_params from dev.h to
bittiming.h and patch #2 makes can_get_relative_tdco() FD agnostic
before also moving it to bittiming.h.
- Patch #3 adds some comments to netlink.h tagging which IFLA
symbols are FD specific.
- Patch #4 to #7 are refactoring can_validate() and
can_validate_bittiming().
- Patch #8 to #12 are refactoring can_changelink() and
can_tdc_changelink().
- Patch #13 and #14 are refactoring can_get_size() and
can_tdc_get_size().
- Patch #15 to #18 are refactoring can_fill_info() and
can_tdc_fill_info().
- Patch #19 makes can_calc_tdco() FD agnostic.
- Patch #20 adds can_get_ctrlmode_str() which converts control mode
flags into strings. This is done in preparation of patch #20.
- Patch #21 is the final patch and improves the user experience by
providing detailed error messages whenever invalid parameters are
provided. All those error messages came into handy when debugging
the upcoming CAN XL patches.
Aside from the last patch, the other changes do not impact any of the
existing functionalities.
The follow up series which introduces CAN XL is nearly completed but
will be sent only once this one is approved: one thing at a time, I do
not want to overwhelm people (including myself).
[1] https://lore.kernel.org/linux-can/20241110155902.72807-16-mailhol.vincent@wanadoo.fr/
[2] https://lore.kernel.org/linux-can/c4771c16-c578-4a6d-baee-918fe276dbe9@wanadoo.fr/
[3] https://lore.kernel.org/linux-can/20241110155902.72807-16-mailhol.vincent@wanadoo.fr/
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Vincent Mailhol <mailhol@kernel.org>
Cc: Stéphane Grosjean <stephane.grosjean@hms-networks.com>
Cc: Robert Nawrath <mbro1689@gmail.com>
Cc: Minh Le <minh.le.aj@renesas.com>
Cc: Duy Nguyen <duy.nguyen.rh@renesas.com>
Cc: linux-can@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Vincent Mailhol (21):
can: dev: move struct data_bittiming_params to linux/can/bittiming.h
can: dev: make can_get_relative_tdco() FD agnostic and move it to bittiming.h
can: netlink: document which symbols are FD specific
can: netlink: refactor can_validate_bittiming()
can: netlink: add can_validate_tdc()
can: netlink: add can_validate_databittiming()
can: netlink: remove comment in can_validate()
can: netlink: refactor CAN_CTRLMODE_TDC_{AUTO,MANUAL} flag reset logic
can: netlink: remove useless check in can_tdc_changelink()
can: netlink: make can_tdc_changelink() FD agnostic
can: netlink: add can_dtb_changelink()
can: netlink: add can_ctrlmode_changelink()
can: netlink: make can_tdc_get_size() FD agnostic
can: netlink: add can_data_bittiming_get_size()
can: netlink: add can_bittiming_fill_info()
can: netlink: add can_bittiming_const_fill_info()
can: netlink: add can_bitrate_const_fill_info()
can: netlink: make can_tdc_fill_info() FD agnostic
can: calc_bittiming: make can_calc_tdco() FD agnostic
can: dev: add can_get_ctrlmode_str()
can: netlink: add userland error messages
drivers/net/can/dev/calc_bittiming.c | 10 +-
drivers/net/can/dev/dev.c | 33 ++
drivers/net/can/dev/netlink.c | 607 ++++++++++++++++++++++-------------
include/linux/can/bittiming.h | 48 ++-
include/linux/can/dev.h | 42 +--
include/uapi/linux/can/netlink.h | 14 +-
6 files changed, 471 insertions(+), 283 deletions(-)
---
base-commit: 2fd4161d0d2547650d9559d57fc67b4e0a26a9e3
change-id: 20250831-canxl-netlink-prep-9dbf8498fd9d
Best regards,
--
Vincent Mailhol <mailhol@kernel.org>
On 03/09/2025 à 17:49, Vincent Mailhol wrote: (...) > The follow up series which introduces CAN XL is nearly completed but > will be sent only once this one is approved: one thing at a time, I do > not want to overwhelm people (including myself). If you want a preview of the full CAN XL, you can have a look at my work in progress here: https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/linux.git/ log/?h=b4/canxl-netlink https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/iproute2-next.git/log/?h=canxl-netlink The kernel part is nearly completed, but I am still playing some whack-a-mole to find potential gaps in the configuration validation. I also need to rewrite or fine tune the commit description. The iproute2 part is still under development. It has the PWM interface but I have not added all the control modes yet. Regardless, the two links above are just an FYI. Beware that there will be some random force pushes without any notice. You can play with it but it is *not* opened for comments until the preparation series is approved. Looking forward for your comments on this CAN XL preparation series, it took me a fair amount of effort :) Yours sincerely, Vincent Mailhol
On 03.09.25 11:26, Vincent Mailhol wrote: > On 03/09/2025 à 17:49, Vincent Mailhol wrote: > > (...) > >> The follow up series which introduces CAN XL is nearly completed but >> will be sent only once this one is approved: one thing at a time, I do >> not want to overwhelm people (including myself). > > If you want a preview of the full CAN XL, you can have a look at my work in > progress here: > > https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/linux.git/ > log/?h=b4/canxl-netlink > https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/iproute2-next.git/log/?h=canxl-netlink > > The kernel part is nearly completed, but I am still playing some whack-a-mole to > find potential gaps in the configuration validation. I also need to rewrite or > fine tune the commit description. > > The iproute2 part is still under development. It has the PWM interface but I > have not added all the control modes yet. > Thanks Vincent! The repos are very helpful. With "missing" control modes you refer to CAN_CTRLMODE_XL_ERR_SIGNAL, CAN_CTRLMODE_XL_RRS and CAN_CTRLMODE_RESTRICTED, right? Best regards, Oliver > Regardless, the two links above are just an FYI. Beware that there will be some > random force pushes without any notice. You can play with it but it is > *not* opened for comments until the preparation series is approved. > > Looking forward for your comments on this CAN XL preparation series, it took me > a fair amount of effort :) > > > Yours sincerely, > Vincent Mailhol
On 04/09/2025 at 15:36, Oliver Hartkopp wrote: > On 03.09.25 11:26, Vincent Mailhol wrote: >> On 03/09/2025 à 17:49, Vincent Mailhol wrote: >> >> (...) >> >>> The follow up series which introduces CAN XL is nearly completed but >>> will be sent only once this one is approved: one thing at a time, I do >>> not want to overwhelm people (including myself). >> >> If you want a preview of the full CAN XL, you can have a look at my work in >> progress here: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/linux.git/ >> log/?h=b4/canxl-netlink >> https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/iproute2-next.git/ >> log/?h=canxl-netlink >> >> The kernel part is nearly completed, but I am still playing some whack-a-mole to >> find potential gaps in the configuration validation. I also need to rewrite or >> fine tune the commit description. >> >> The iproute2 part is still under development. It has the PWM interface but I >> have not added all the control modes yet. >> > > Thanks Vincent! > > The repos are very helpful. > > With "missing" control modes you refer to CAN_CTRLMODE_XL_ERR_SIGNAL, > CAN_CTRLMODE_XL_RRS and CAN_CTRLMODE_RESTRICTED, right? Yes, I only added the CAN_CTRLMODE_XL_TMS so far in iproute2. The kernel has all of the four flags (but because I did not finish testing, I highly suspect that there are still some bugs somewhere). Concerning the CAN_CTRLMODE_XL_RRS, I am not sure if that one is needed. I still have it in my WIP series but I am recently considering to remove it. The reason is that when reading ISO 11898-1 having RRS configurable looks mandatory to me. In the logical Link control (LLC) this RRS bit is named FTYP (for Frame Type). For example, CiA only mentions FTYP in their CAN XL knowledge page: https://www.can-cia.org/can-knowledge/can-xl Contrarily to CAN FD's RRS which is indeed specified as being dominant and which is just ignored in the LLC, the CAN XL FTYP/RRS is part of the LLC interface and is meant to be configurable. Nothing in the standard tells us that this should be a dominant bit. I think your intention was to add CAN_CTRLMODE_XL_RRS as a quirk for the devices which expose this flag. But as far as I can see, it seems that a device which does not expose it is just not compliant. If some day a device which can not set the FTYP/RRS flag appears in the wild, then maybe we can add a flag which would specify that RRS is not configurable (opposite logic as what you suggested). But as long as such a device do not exist, it is better to add nothing. Yours sincerely, Vincent Mailhol
On 04.09.25 11:18, Vincent Mailhol wrote: > Concerning the CAN_CTRLMODE_XL_RRS, I am not sure if that one is needed. I still > have it in my WIP series but I am recently considering to remove it. The reason > is that when reading ISO 11898-1 having RRS configurable looks mandatory to me. > > In the logical Link control (LLC) this RRS bit is named FTYP (for Frame Type). > For example, CiA only mentions FTYP in their CAN XL knowledge page: > > https://www.can-cia.org/can-knowledge/can-xl > > Contrarily to CAN FD's RRS which is indeed specified as being dominant and which > is just ignored in the LLC, the CAN XL FTYP/RRS is part of the LLC interface and > is meant to be configurable. I double checked my XCANB CAN XL controller spec and indeed the RRS bit is part of every RX/TX FIFO element and the figures see it as configurable element too. > Nothing in the standard tells us that this should be a dominant bit. I think > your intention was to add CAN_CTRLMODE_XL_RRS as a quirk for the devices which > expose this flag. But as far as I can see, it seems that a device which does not > expose it is just not compliant. Let's see if we will find CAN XL IP cores where the engineers have a different view on this. I currently have a discussion on this RRS bit with the Vector support because the RRS bit is not visible in the CANalyser 19 GUI. > If some day a device which can not set the FTYP/RRS flag appears in the wild, > then maybe we can add a flag which would specify that RRS is not configurable > (opposite logic as what you suggested). But as long as such a device do not > exist, it is better to add nothing. ACK. After this discussion I would also vote to omit my glorious patch which added the CAN_CTRLMODE_XL_RRS flag. Let's see if we find a CAN XL controller that does not support the variable RRS bit in reading and writing. And if it shows up we can add this flag to handle it (similar to the fd-non-iso feature). Best regards, Oliver
On 05/09/2025 at 20:11, Oliver Hartkopp wrote: > On 04.09.25 11:18, Vincent Mailhol wrote: > >> Concerning the CAN_CTRLMODE_XL_RRS, I am not sure if that one is needed. I still >> have it in my WIP series but I am recently considering to remove it. The reason >> is that when reading ISO 11898-1 having RRS configurable looks mandatory to me. >> >> In the logical Link control (LLC) this RRS bit is named FTYP (for Frame Type). >> For example, CiA only mentions FTYP in their CAN XL knowledge page: >> >> https://www.can-cia.org/can-knowledge/can-xl >> >> Contrarily to CAN FD's RRS which is indeed specified as being dominant and which >> is just ignored in the LLC, the CAN XL FTYP/RRS is part of the LLC interface and >> is meant to be configurable. > > I double checked my XCANB CAN XL controller spec and indeed the RRS bit is part > of every RX/TX FIFO element and the figures see it as configurable element too. > >> Nothing in the standard tells us that this should be a dominant bit. I think >> your intention was to add CAN_CTRLMODE_XL_RRS as a quirk for the devices which >> expose this flag. But as far as I can see, it seems that a device which does not >> expose it is just not compliant. > > Let's see if we will find CAN XL IP cores where the engineers have a different > view on this. I currently have a discussion on this RRS bit with the Vector > support because the RRS bit is not visible in the CANalyser 19 GUI. > >> If some day a device which can not set the FTYP/RRS flag appears in the wild, >> then maybe we can add a flag which would specify that RRS is not configurable >> (opposite logic as what you suggested). But as long as such a device do not >> exist, it is better to add nothing. > > ACK. After this discussion I would also vote to omit my glorious patch which > added the CAN_CTRLMODE_XL_RRS flag. Let's see if we find a CAN XL controller > that does not support the variable RRS bit in reading and writing. And if it > shows up we can add this flag to handle it (similar to the fd-non-iso feature). OK. Good that we reached out to the same conclusion :) Because I already implemented all the logic, I will save the current RRS patch somewhere in case we need to resurrect it some days. Yours sincerely, Vincent Mailhol
© 2016 - 2025 Red Hat, Inc.