Inter-revision diff: cover letter

Comparing v1 (message) to v5 (message)

--- v1
+++ v5
@@ -1,8 +1,10 @@
-This serie contains a single patch which adds a netlink interface for
-the TDC parameters using netlink nested attributes.
+The main goal of this series is to add a netlink interface for the TDC
+parameters using netlink nested attributes. The series also contains a
+few fix on the current TDC implementation.
 
-In March, I introduced the Transmitter Delay Compensation (TDC) to the
-kernel though two patches:
+This series completes the implementation of the Transmitter Delay
+Compensation (TDC) in the kernel which started in March with those two
+patches:
   - commit 289ea9e4ae59 ("can: add new CAN FD bittiming parameters:
     Transmitter Delay Compensation (TDC)")
   - commit c25cc7993243 ("can: bittiming: add calculation for CAN FD
@@ -13,38 +15,98 @@
 changes.
 
 At that time, Marc suggested to take inspiration from the recently
-released ethtool-netlink interface.
-Ref: https://lore.kernel.org/linux-can/20210407081557.m3sotnepbgasarri@pengutronix.de/
+released ethtool-netlink interface (Ref [1]).
 
-After further research, it appears that ethtool uses nested attributes
-(c.f. NLA_NESTED type in validation policy). A bit of trivia: the
-NLA_NESTED type was introduced in version 2.6.15 of the kernel and
-thus actually predates Socket CAN.
-Ref: commit bfa83a9e03cf ("[NETLINK]: Type-safe netlink
-messages/attributes interface")
+ethtool uses nested attributes (c.f. NLA_NESTED type in validation
+policy). A bit of trivia: the NLA_NESTED type was introduced in
+version 2.6.15 of the kernel and thus actually predates Socket CAN.
+Ref: commit bfa83a9e03cf ("[NETLINK]: Type-safe netlink messages/attributes interface")
 
-It took me a bit of time to understand and figure out how to use those
-nested attributes. While the patch should be functional, I am not fully
-done with my testing yet. I thus send this version as an RFC. I wish
-to receive comments of the overall design. Contents of the functions
-might still be subjected to small changes.
+Since then, Stephan shared additional remarks for improvement which
+are addressed in revision v4 of this series [2].
 
-After gathering the comments, I will send a new version in which I
-will also include an update to Documentation/networking/can.rst.
+The first patch allow a user to turn off a feature even if not
+supported (e.g. allow "ip link set can0 type can bitrate 500000 fd
+off" even if fd is not supported). This feature will be used later by
+the fifth patch of the series.
 
-Vincent Mailhol (1):
+The second patch allows TDCV and TDCO to be zero (previously, those
+values were assigned a special meaning).
+
+The third patch fixes the unit of the TDC parameters. In fact, those
+should be measured in clock period (a.k.a. minimum time quantum) and
+not in time quantum as it is done in current implementation.
+
+The fourth patch addresses the concern of Marc and Stefan concerning
+some devices which use a TDCO value relative to the Sample Point (so
+far, the TDC implementation used the formula SSP = TDCV + TDCO). To do
+so, an helper function to convert TDCO from an absolute value to a
+value relative to the sample point is added.
+
+The fifth patch is the real thing: the TDC netlink interface.
+
+The sixth patch allows to retrieve TDCV from the device through a
+callback function.
+
+The seventh and last patch does a bit of cleanup in the ES58x driver
+to remove some redundant TDC information from the documentation.
+
+[1] https://lore.kernel.org/linux-can/20210407081557.m3sotnepbgasarri@pengutronix.de/
+[2] https://lore.kernel.org/linux-can/75fa87a71a3f5fd7d7407a2c514b71690e56eb4e.camel@esd.eu/
+
+
+** Changelog **
+
+v4 -> v5:
+  - move to can_validate() all of the checks on TDC parameters that do
+    not need access to priv.
+  - in can_tdc_get_size(), field IFLA_CAN_TDCV was not taken in
+    account for size calculation when in automatic mode.
+  - if can_tdc_is_enabled(priv) returns true, we know that one of
+    either CAN_CTRLMODE_TDC_AUTO or CAN_CTRLMODE_TDC_MANUAL is
+    set. After confirming that CAN_CTRLMODE_TDC_MANUAL if off, we then
+    implicitly know that CAN_CTRLMODE_TDC_AUTO is set and do not need
+    to check it again. Remove such redundant check in
+    can_tdc_fill_info().
+
+v3 -> v4:
+  - add a patch to the series to change the unit from time quantum to
+    clock period (a.k.a. minimum time quantum).
+  - add a callback function to retrieve TDCV from the device.
+
+v2 -> v3:
+  - allows both TDCV and TDCO to be zero.
+  - introduce the can_tdc_get_relative_tdco() helper function
+  - other path of the series modified accordingly to above changes.
+
+RFC v1 -> v2:
+  The v2 fixes several issue in can_tdc_get_size() and
+  can_tdc_fill_info(). Namely: can_tdc_get_size() returned an
+  incorrect size if TDC was not implemented and can_tdc_fill_info()
+  did not include a fail path with nla_nest_cancel().
+
+
+Vincent Mailhol (7):
+  can: netlink: allow user to turn off unsupported features
+  can: bittiming: allow TDC{V,O} to be zero and add
+    can_tdc_const::tdc{v,o,f}_min
+  can: bittiming: change unit of TDC parameters to clock periods
+  can: dev: add can_tdc_get_relative_tdco() helper function
   can: netlink: add interface for CAN-FD Transmitter Delay Compensation
     (TDC)
+  can: netlink: add can_priv::do_get_auto_tdcv() to retrieve tdcv from
+    device
+  can: etas_es58x: clean-up documentation of struct es58x_fd_tx_conf_msg
 
- drivers/net/can/dev/Makefile      |   1 +
- drivers/net/can/dev/netlink-tdc.c | 122 ++++++++++++++++++++++++++++++
- drivers/net/can/dev/netlink-tdc.h |  18 +++++
- drivers/net/can/dev/netlink.c     |  15 +++-
- include/uapi/linux/can/netlink.h  |  28 ++++++-
- 5 files changed, 179 insertions(+), 5 deletions(-)
- create mode 100644 drivers/net/can/dev/netlink-tdc.c
- create mode 100644 drivers/net/can/dev/netlink-tdc.h
+ drivers/net/can/dev/bittiming.c           |  19 +-
+ drivers/net/can/dev/netlink.c             | 213 +++++++++++++++++++++-
+ drivers/net/can/usb/etas_es58x/es58x_fd.c |   7 +-
+ drivers/net/can/usb/etas_es58x/es58x_fd.h |  23 +--
+ include/linux/can/bittiming.h             |  80 +++++---
+ include/linux/can/dev.h                   |  34 ++++
+ include/uapi/linux/can/netlink.h          |  31 +++-
+ 7 files changed, 353 insertions(+), 54 deletions(-)
 
 -- 
-2.26.3
+2.31.1
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help