Inter-revision diff: patch 6

Comparing v6 (message) to v5 (message)

--- v6
+++ v5
@@ -1,74 +1,74 @@
-struct can_tdc::tdco represents the absolute offset from TDCV. Some
-controllers use instead an offset relative to the Sample Point (SP)
-such that:
-| SSP = TDCV + absolute TDCO
-|     = TDCV + SP + relative TDCO
+Some CAN device can measure the TDCV (Transmission Delay Compensation
+Value) automatically for each transmitted CAN frames.
 
-Consequently:
-| relative TDCO = absolute TDCO - SP
+A callback function do_get_auto_tdcv() is added to retrieve that
+value. This function is used only if CAN_CTRLMODE_TDC_AUTO is enabled
+(if CAN_CTRLMODE_TDC_MANUAL is selected, the TDCV value is provided by
+the user).
 
-The function can_tdc_get_relative_tdco() allow to retrieve this
-relative TDCO value.
+If the device does not support reporting of TDCV, do_get_auto_tdcv()
+should be set to NULL and TDCV will not be reported by the netlink
+interface.
 
-CC: Stefan Mätje <Stefan.Maetje@esd.eu>
+On success, do_get_auto_tdcv() shall return 0. If the value can not be
+measured by the device, for example because network is down or because
+no frames were transmitted yet, can_priv::do_get_auto_tdcv() shall
+return a negative error code (e.g. -EINVAL) to signify that the value
+is not yet available. In such cases, TDCV is not reported by the
+netlink interface.
+
+CC: Stefan Mätje <stefan.maetje@esd.eu>
 Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
 ---
-Hi Marc,
+ drivers/net/can/dev/netlink.c | 15 ++++++++++++---
+ include/linux/can/dev.h       |  1 +
+ 2 files changed, 13 insertions(+), 3 deletions(-)
 
-As Stefan pointed out in:
-https://lore.kernel.org/linux-can/79691916e4280970f583a54cd5010ece025a1c53.c
-amel@esd.eu/
-it seems that no so many CAN devices are using the relative TDCO
-(maybe the ESDACC is the only one?). Depending on the output of your
-discussion with Microchip and if the mcp25xxfd is indeed using an
-absolute tdco, it might make sense to drop this patch from the series.
-
-Yours sincerely,
-Vincent
----
- include/linux/can/dev.h | 29 +++++++++++++++++++++++++++++
- 1 file changed, 29 insertions(+)
-
+diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
+index f05745c96b9c..776284593b5d 100644
+--- a/drivers/net/can/dev/netlink.c
++++ b/drivers/net/can/dev/netlink.c
+@@ -362,7 +362,8 @@ static size_t can_tdc_get_size(const struct net_device *dev)
+ 	}
+ 
+ 	if (can_tdc_is_enabled(priv)) {
+-		if (priv->ctrlmode & CAN_CTRLMODE_TDC_MANUAL)
++		if (priv->ctrlmode & CAN_CTRLMODE_TDC_MANUAL ||
++		    priv->do_get_auto_tdcv)
+ 			size += nla_total_size(sizeof(u32));	/* IFLA_CAN_TDCV */
+ 		size += nla_total_size(sizeof(u32));		/* IFLA_CAN_TDCO */
+ 		if (priv->tdc_const->tdcf_max)
+@@ -435,8 +436,16 @@ static int can_tdc_fill_info(struct sk_buff *skb, const struct net_device *dev)
+ 		goto err_cancel;
+ 
+ 	if (can_tdc_is_enabled(priv)) {
+-		if (priv->ctrlmode & CAN_CTRLMODE_TDC_MANUAL &&
+-		    nla_put_u32(skb, IFLA_CAN_TDC_TDCV, tdc->tdcv))
++		u32 tdcv;
++		int err = -EINVAL;
++
++		if (priv->ctrlmode & CAN_CTRLMODE_TDC_MANUAL) {
++			tdcv = tdc->tdcv;
++			err = 0;
++		} else if (priv->do_get_auto_tdcv) {
++			err = priv->do_get_auto_tdcv(dev, &tdcv);
++		}
++		if (!err && nla_put_u32(skb, IFLA_CAN_TDC_TDCV, tdcv))
+ 			goto err_cancel;
+ 		if (nla_put_u32(skb, IFLA_CAN_TDC_TDCO, tdc->tdco))
+ 			goto err_cancel;
 diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
-index b4aa0f048cab..45f19d9db5ca 100644
+index fa75e29182a3..266d0fe9de9d 100644
 --- a/include/linux/can/dev.h
 +++ b/include/linux/can/dev.h
-@@ -102,6 +102,35 @@ static inline bool can_tdc_is_enabled(const struct can_priv *priv)
- 	return !!(priv->ctrlmode & CAN_CTRLMODE_TDC_MASK);
- }
+@@ -74,6 +74,7 @@ struct can_priv {
+ 			    enum can_state *state);
+ 	int (*do_get_berr_counter)(const struct net_device *dev,
+ 				   struct can_berr_counter *bec);
++	int (*do_get_auto_tdcv)(const struct net_device *dev, u32 *tdcv);
  
-+/*
-+ * can_get_relative_tdco() - TDCO relative to the sample point
-+ *
-+ * struct can_tdc::tdco represents the absolute offset from TDCV. Some
-+ * controllers use instead an offset relative to the Sample Point (SP)
-+ * such that:
-+ *
-+ * SSP = TDCV + absolute TDCO
-+ *     = TDCV + SP + relative TDCO
-+ *
-+ * -+----------- one bit ----------+-- TX pin
-+ *  |<--- Sample Point --->|
-+ *
-+ *                         --+----------- one bit ----------+-- RX pin
-+ *  |<-------- TDCV -------->|
-+ *                           |<------------------------>| absolute TDCO
-+ *                           |<--- Sample Point --->|
-+ *                           |                      |<->| relative TDCO
-+ *  |<------------- Secondary Sample Point ------------>|
-+ */
-+static inline s32 can_get_relative_tdco(const struct can_priv *priv)
-+{
-+	const struct can_bittiming *dbt = &priv->data_bittiming;
-+	s32 sample_point_in_tc = (CAN_SYNC_SEG + dbt->prop_seg +
-+				  dbt->phase_seg1) * dbt->brp;
-+
-+	return (s32)priv->tdc.tdco - sample_point_in_tc;
-+}
-+
- /* helper to define static CAN controller features at device creation time */
- static inline void can_set_static_ctrlmode(struct net_device *dev,
- 					   u32 static_mode)
+ 	unsigned int echo_skb_max;
+ 	struct sk_buff **echo_skb;
 -- 
-2.32.0
+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