On Fri. 18 juin 2021 at 18:10, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote: > > On 18.06.2021 17:19:03, Vincent Mailhol wrote: > > When the FD is turned off through the netlink interface, the value > > values > > > still remain in data_bittiming and are displayed despite of the > > feature being disabled. > > > > Example: > > > > $ ip link set can0 type can bitrate 500000 dbitrate 2000000 fd on > > $ ip --details link show can0 > > 1: can0: <NOARP,ECHO> mtu 72 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 10 > > link/can promiscuity 0 minmtu 0 maxmtu 0 > > can <FD> state STOPPED restart-ms 0 > > bitrate 500000 sample-point 0.875 > > tq 12 prop-seg 69 phase-seg1 70 phase-seg2 20 sjw 1 > > ES582.1/ES584.1: tseg1 2..256 tseg2 2..128 sjw 1..128 brp 1..512 brp-inc 1 > > dbitrate 2000000 dsample-point 0.750 > > dtq 12 dprop-seg 14 dphase-seg1 15 dphase-seg2 10 dsjw 1 > > ES582.1/ES584.1: dtseg1 2..32 dtseg2 1..16 dsjw 1..8 dbrp 1..32 dbrp-inc 1 > > clock 80000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 > > > > $ ip link set can0 type can bitrate 500000 fd off > > $ ip --details link show can0 > > 1: can0: <NOARP,ECHO> mtu 16 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 10 > > link/can promiscuity 0 minmtu 0 maxmtu 0 > > can state STOPPED restart-ms 0 > > bitrate 500000 sample-point 0.875 > > tq 12 prop-seg 69 phase-seg1 70 phase-seg2 20 sjw 1 > > ES582.1/ES584.1: tseg1 2..256 tseg2 2..128 sjw 1..128 brp 1..512 brp-inc 1 > > dbitrate 2000000 dsample-point 0.750 > > dtq 12 dprop-seg 14 dphase-seg1 15 dphase-seg2 10 dsjw 1 > > ES582.1/ES584.1: dtseg1 2..32 dtseg2 1..16 dsjw 1..8 dbrp 1..32 dbrp-inc 1 > > clock 80000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 > > > > Remark: once FD is turned off, it is not possible to just turn fd back > > on and reuse the previously input data bittiming values: > > > $ ip link set can0 type can bitrate 500000 fd on > > RTNETLINK answers: Operation not supported > > > > This means that the user will need to overwrite data bittiming with > ^^^^^^^^^ > set > > At least with your change it's more a "set again" than to overwrite. > > > fresh values in order to turn fd on again. > > > > Because old data bittiming values can not be reused, this patch just > > clears priv->data_bittiming whenever FD is turned off. This way, the > > data bittiming variables are not displayed anymore. > > > > Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx> > > --- > > Hi Marc, > > > > I suggest to rebase this patch before the netlink TDC series. > > makes sense - If you're OK with the changes, I'll add them while > applying. I am OK with the changes. > Your patch makes the interface consistent, another option would be to > allow FD mode if the data bit timing values have been set before. > Opinions? One part of me says it would make sense but let's also try to be realistic. The only reason I spotted this issue is because I was actively trying to find defects while testing my TDC implementation. Under normal usage, I never had have such needs. How many people will need to set fd on, then off, then on again? Too few I think. Let's force those few people to always provide the data bittiming values. The overhead which would be needed to implement this in drivers/net/can/dev/netlink.c is just not worth it. > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | https://www.pengutronix.de | > Vertretung West/Dortmund | Phone: +49-231-2826-924 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
