Hi Vincent,
I found some issues in the code and fixed up the problems below.
The funniest thing was this copy/paste problem in netlink.h ;-)
(see attached patch with my changes)
The patch descriptions are not finalized - but it becomes usable now.
I will add the CAN XL transceiver switch to the controlmode definitions.
For the PWM configuration we would need some more discussions.
https://lore.kernel.org/linux-can/20241201112333.6950-1-socketcan@xxxxxxxxxxxx/T/#u
https://lore.kernel.org/linux-can/20241201112230.6917-1-socketcan@xxxxxxxxxxxx/T/#t
Best regards,
Oliver
On 21.11.24 21:10, Oliver Hartkopp wrote:
Hi Vincent,
On 11.11.24 16:32, Oliver Hartkopp wrote:
No problem! I will give some feedback when I managed to integrate the
extended netlink API to my driver.
I managed to set up my CAN XL dev board with the latest Linux kernel
(6.13 merge) and upgraded to Ubuntu 24.04 LTS.
Applying your RFC patches for the kernel and the iproute2 package
(including the sed hack) works great.
While the xl transceiver switch and the and the PWM configuration (with
3 values with 6 bit each) are still missing I tried your current code
as-is.
# modprobe dummyxl
(created can0)
# ip link set can0 type can bitrate 500000 xbitrate 8000000 xl on
dbitrate 4000000 fd on
# ip -det link show can0
7: can0: <NOARP> mtu 2060 qdisc noop state DOWN mode DEFAULT group
default qlen 10
link/can promiscuity 0 allmulti 0 minmtu 0 maxmtu 0
can <FD,TDC-AUTO,XL> state STOPPED restart-ms 0
Should there be a XTDC-AUTO too?
bitrate 500000 sample-point 0.875
tq 12 prop-seg 69 phase-seg1 70 phase-seg2 20 sjw 10 brp 1
dummyxl nominal: tseg1 2..256 tseg2 2..128 sjw 1..128 brp 1..512
brp_inc 1
dbitrate 4000000 dsample-point 0.750
dtq 12 dprop-seg 7 dphase-seg1 7 dphase-seg2 5 dsjw 2 dbrp 1
tdco 15 tdcf 0
Should the tdc* values be placed behind dbrp 1 without a line break?
The gso/gro stuff below also heavily exceeds the 80 columns. And it is
more in the same shape like with the CAN CC settings.
dummyxl FD: dtseg1 2..256 dtseg2 2..128 dsjw 1..128 dbrp 1..512
dbrp_inc 1
tdco 0..127 tdcf 0..127
same here
xbitrate 8000000 xsample-point 0.700
xtq 12 xprop-seg 3 xphase-seg1 3 xphase-seg2 3 xsjw 1 xbrp 1
xtdco/xtdcf missing here?
dummyxl XL: xtseg1 2..256 xtseg2 2..128 xsjw 1..128 xbrp 1..512
xbrp_inc 1
xtdco/xtdcf missing here?
clock 80000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536
gso_max_segs 65535 tso_max_size 65536 tso_max_segs 65535 gro_max_size
65536 gso_ipv4_max_size 65536 gro_ipv4_max_size 65536
Best regards,
Oliver
diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 0a6700194ab0..64f675e874f6 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -127,10 +127,19 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
if (err) {
NL_SET_ERR_MSG_FMT(extack,
"TDC parameters are incorrect");
return err;
}
+ err = can_validate_tdc(data[IFLA_CAN_XL_TDC],
+ cm->flags & CAN_CTRLMODE_XL_TDC_AUTO,
+ cm->flags & CAN_CTRLMODE_XL_TDC_MANUAL,
+ extack);
+ if (err) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "XL TDC parameters are incorrect");
+ return err;
+ }
}
if (data[IFLA_CAN_BITTIMING]) {
struct can_bittiming bt;
@@ -570,20 +579,21 @@ static size_t can_get_size(const struct net_device *dev)
return size;
}
static int can_tdc_fill_info(struct sk_buff *skb, const struct net_device *dev,
struct data_bittiming_params *dbt_params,
- bool tdc_is_enabled, bool tdc_manual)
+ bool tdc_is_enabled, bool tdc_manual,
+ int ifla_can_tdc_type)
{
struct nlattr *nest;
struct can_tdc *tdc = &dbt_params->tdc;
const struct can_tdc_const *tdc_const = dbt_params->tdc_const;
if (!tdc_const)
return 0;
- nest = nla_nest_start(skb, IFLA_CAN_TDC);
+ nest = nla_nest_start(skb, ifla_can_tdc_type);
if (!nest)
return -EMSGSIZE;
if (tdc_manual &&
(nla_put_u32(skb, IFLA_CAN_TDC_TDCV_MIN, tdc_const->tdcv_min) ||
@@ -702,11 +712,12 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
(nla_put(skb, IFLA_CAN_BITRATE_MAX,
sizeof(priv->bitrate_max),
&priv->bitrate_max)) ||
can_tdc_fill_info(skb, dev, &priv->fd, can_fd_tdc_is_enabled(priv),
- priv->ctrlmode & CAN_CTRLMODE_TDC_MANUAL) ||
+ priv->ctrlmode & CAN_CTRLMODE_TDC_MANUAL,
+ IFLA_CAN_TDC) ||
can_ctrlmode_ext_fill_info(skb, priv) ||
(priv->xl.data_bittiming.bitrate &&
nla_put(skb, IFLA_CAN_XL_DATA_BITTIMING,
@@ -722,11 +733,12 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
sizeof(*priv->xl.data_bitrate_const) *
priv->xl.data_bitrate_const_cnt,
priv->xl.data_bitrate_const)) ||
can_tdc_fill_info(skb, dev, &priv->xl, can_xl_tdc_is_enabled(priv),
- priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MANUAL)
+ priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MANUAL,
+ IFLA_CAN_XL_TDC)
)
return -EMSGSIZE;
return 0;
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index c81fd153a07f..f4fb8eea8f35 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -102,12 +102,12 @@ struct can_ctrlmode {
#define CAN_CTRLMODE_FD_NON_ISO 0x80 /* CAN FD in non-ISO mode */
#define CAN_CTRLMODE_CC_LEN8_DLC 0x100 /* Classic CAN DLC option */
#define CAN_CTRLMODE_TDC_AUTO 0x200 /* FD transceiver automatically calculates TDCV */
#define CAN_CTRLMODE_TDC_MANUAL 0x400 /* FD TDCV is manually set up by user */
#define CAN_CTRLMODE_XL 0x800 /* CAN XL mode */
-#define CAN_CTRLMODE_XL_TDC_AUTO 0x200 /* XL transceiver automatically calculates TDCV */
-#define CAN_CTRLMODE_XL_TDC_MANUAL 0x400 /* XL TDCV is manually set up by user */
+#define CAN_CTRLMODE_XL_TDC_AUTO 0x1000 /* XL transceiver automatically calculates TDCV */
+#define CAN_CTRLMODE_XL_TDC_MANUAL 0x2000 /* XL TDCV is manually set up by user */
/*
* CAN device statistics
*/
struct can_device_stats {
