by Marc Kleine-Budde

[permalink] [raw]

Subject: Re: [PATCH v5 2/7] can: bittiming: allow TDC{V,O} to be zero and add can_tdc_const::tdc{v,o,f}_min

On 15.08.2021 12:32:43, Vincent Mailhol wrote:
> ISO 11898-1 specifies in section 11.3.3 "Transmitter delay
> compensation" that "the configuration range for [the] SSP position
> shall be at least 0 to 63 minimum time quanta."
>
> Because SSP = TDCV + TDCO, it means that we should allow both TDCV and
> TDCO to hold zero value in order to honor SSP's minimum possible
> value.
>
> However, current implementation assigned special meaning to TDCV and
> TDCO's zero values:
> * TDCV = 0 -> TDCV is automatically measured by the transceiver.
> * TDCO = 0 -> TDC is off.
>
> In order to allow for those values to really be zero and to maintain
> current features, we introduce two new flags:
> * CAN_CTRLMODE_TDC_AUTO indicates that the controller support
> automatic measurement of TDCV.
> * CAN_CTRLMODE_TDC_MANUAL indicates that the controller support
> manual configuration of TDCV. N.B.: current implementation failed
> to provide an option for the driver to indicate that only manual
> mode was supported.
>
> TDC is disabled if both CAN_CTRLMODE_TDC_AUTO and
> CAN_CTRLMODE_TDC_MANUAL flags are off, c.f. the helper function
> can_tdc_is_enabled() which is also introduced in this patch.

Nitpick: We can only say that TDC is disabled, if the driver supports
the TDC interface at all, which is the case if tdc_const is set.

> Also, this patch adds three fields: tdcv_min, tdco_min and tdcf_min to
> struct can_tdc_const. While we are not convinced that those three
> fields could be anything else than zero, we can imagine that some
> controllers might specify a lower bound on these. Thus, those minimums
> are really added "just in case".

I'm not sure, if we talked about the mcp251xfd's tcdo, valid values are
-64...63.

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 |

Attachments:

(No filename) (2.03 kB)
signature.asc (499.00 B)
Download all attachments

2021-08-16 10:26:37

by Vincent MAILHOL

Hi Vincent,

I would like to add some comments below:

Am Montag, den 16.08.2021, 14:25 +0200 schrieb Marc Kleine-Budde:
> On 16.08.2021 19:24:43, Vincent MAILHOL wrote:
> > On Mon. 16 Aug 2021 at 17:42, Marc Kleine-Budde <[email protected]> wrote:
> > > On 15.08.2021 12:32:43, Vincent Mailhol wrote:
> > > > ISO 11898-1 specifies in section 11.3.3 "Transmitter delay
> > > > compensation" that "the configuration range for [the] SSP position
> > > > shall be at least 0 to 63 minimum time quanta."
> > > >
> > > > Because SSP = TDCV + TDCO, it means that we should allow both TDCV and
> > > > TDCO to hold zero value in order to honor SSP's minimum possible
> > > > value.
> > > >
> > > > However, current implementation assigned special meaning to TDCV and
> > > > TDCO's zero values:
> > > > * TDCV = 0 -> TDCV is automatically measured by the transceiver.
> > > > * TDCO = 0 -> TDC is off.
> > > >
> > > > In order to allow for those values to really be zero and to maintain
> > > > current features, we introduce two new flags:
> > > > * CAN_CTRLMODE_TDC_AUTO indicates that the controller support
> > > > automatic measurement of TDCV.
> > > > * CAN_CTRLMODE_TDC_MANUAL indicates that the controller support
> > > > manual configuration of TDCV. N.B.: current implementation failed
> > > > to provide an option for the driver to indicate that only manual
> > > > mode was supported.
> > > >
> > > > TDC is disabled if both CAN_CTRLMODE_TDC_AUTO and
> > > > CAN_CTRLMODE_TDC_MANUAL flags are off, c.f. the helper function
> > > > can_tdc_is_enabled() which is also introduced in this patch.
> > >
> > > Nitpick: We can only say that TDC is disabled, if the driver supports
> > > the TDC interface at all, which is the case if tdc_const is set.
> >
> > I would argue that saying that a device does not support TDC is
> > equivalent to saying that TDC is always disabled for that device.
> > Especially, the function can_tdc_is_enabled() can be used even if
> > the device does not support TDC (even if there is no benefit
> > doing so).
> >
> > Do you still want me to rephrase this part?
> >
> > > > Also, this patch adds three fields: tdcv_min, tdco_min and tdcf_min to
> > > > struct can_tdc_const. While we are not convinced that those three
> > > > fields could be anything else than zero, we can imagine that some
> > > > controllers might specify a lower bound on these. Thus, those minimums
> > > > are really added "just in case".
> > >
> > > I'm not sure, if we talked about the mcp251xfd's tcdo, valid values are
> > > -64...63.
> >
> > Yes! Stefan shed some light on this. The mcp251xfd uses a tdco
> > value which is relative to the sample point.
>
> I don't read the documentation this way....

@Vincent: I have to agree with Marc here. Perhaps my email
https://lore.kernel.org/linux-can/[email protected]/
was also misleading. I also referred there to a MicroChip Excel sheet
(https://ww1.microchip.com/downloads/en/DeviceDoc/MCP2517FD%20Bit%20Time%20Calculations%20-%20UG.xlsx)
that describes the calculation of the bit timing and the TDCO. The values calculated
there correspond to the SPO from the above email. Microchip calculates the TDCO as
TDCO = (DPRSEG + DPH1SEG) * DBRP.
Thus, as already discussed, negative values are not purposeful.

Sorry, that that email was misleading. So far I've seen now only the ESDACC
controller has a "relative" TDCO register value where a negative value may
be sensible.

> > > SSP = TDCV + absolute TDCO
> > > = TDCV + SP + relative TDCO
> >
> > Consequently:
> > > relative TDCO = absolute TDCO - SP
>
> In the mcp15xxfd family manual
> (http://ww1.microchip.com/downloads/en/DeviceDoc/MCP251XXFD-CAN-FD-Controller-Module-Family-Reference-Manual-20005678B.pdf)
> in the 2mbit/s data bit rate example in table 3-5 (page 21) it says:
>
> > DTSEG1 15 DTQ
> > DTSEG2 4 DTQ
> > TDCO 15 DTQ
>
> The mcp251xfd driver uses 15, the framework calculates 16 (== Sync Seg+
> tseg1, which is correct), and relative tdco would be 0:
>
> > mcp251xfd_set_bittiming: tdco=15, priv->tdc.tdc=16, relative_tdco=0
>
> Here the output with the patched ip tool:
>
> > 4: mcp251xfd0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UP mode DEFAULT group default qlen 10
> > link/can promiscuity 0 minmtu 0 maxmtu 0
> > can <FD,TDC_AUTO> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 100
> > bitrate 500000 sample-point 0.875
> > tq 25 prop-seg 34 phase-seg1 35 phase-seg2 10 sjw 1 brp 1
> > mcp251xfd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp_inc 1
> > dbitrate 2000000 dsample-point 0.750
> > dtq 25 dprop-seg 7 dphase-seg1 7 dphase-seg2 5 dsjw 1 dbrp 1
> > tdco 15
> > mcp251xfd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp_inc 1
> > tdco 0..127
> > clock 40000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 parentbus spi parentdev spi0.0
>
>
> > Which is also why TDCO can be negative.
> >
> > I added an helper function can_tdc_get_relative_tdco() in the
> > fourth path of this series:
> > https://lore.kernel.org/linux-can/[email protected]/T/#u
> >
> > Devices which use the absolute TDCO can directly use
> > can_priv->tdc.tdco. Devices which use the relative TDCO such as
> > the mcp251xfd should use this helper function instead.
>
> Don't think so....

@Vincent: Perhaps you should not implement this helper function as it is only needed
for the ESDACC so far.

> > However, you will still need to convert the TDCO valid range from
> > relative values to absolute ones. In your case 0..127.
>
> Marc
>

Stefan

2021-08-16 14:13:28

by Vincent MAILHOL

[permalink] [raw]

Subject: Re: [PATCH v5 2/7] can: bittiming: allow TDC{V,O} to be zero and add can_tdc_const::tdc{v,o,f}_min

On Mon. 16 Aug 2021 at 21:33, Marc Kleine-Budde <[email protected]> wrote:
> On 16.08.2021 14:25:19, Marc Kleine-Budde wrote:
> > > > I'm not sure, if we talked about the mcp251xfd's tcdo, valid values are
> > > > -64...63.
> > >
> > > Yes! Stefan shed some light on this. The mcp251xfd uses a tdco
> > > value which is relative to the sample point.
> >
> > I don't read the documentation this way....
> >
> > > | SSP = TDCV + absolute TDCO
> > > | = TDCV + SP + relative TDCO
> > >
> > > Consequently:
> > > | relative TDCO = absolute TDCO - SP
> >
> > In the mcp15xxfd family manual
> > (http://ww1.microchip.com/downloads/en/DeviceDoc/MCP251XXFD-CAN-FD-Controller-Module-Family-Reference-Manual-20005678B.pdf)
> > in the 2mbit/s data bit rate example in table 3-5 (page 21) it says:
> >
> > | DTSEG1 15 DTQ
> > | DTSEG2 4 DTQ
> > | TDCO 15 DTQ
> >
> > The mcp251xfd driver uses 15, the framework calculates 16 (== Sync Seg+
> > tseg1, which is correct), and relative tdco would be 0:
> >
> > | mcp251xfd_set_bittiming: tdco=15, priv->tdc.tdc=16, relative_tdco=0
> >
> > Here the output with the patched ip tool:
>
> Sorry, the previous output was not using the sample points of the
> example in the data sheet, this is the fixed output:
>
> | 6: mcp251xfd0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UP mode DEFAULT group default qlen 10
> | link/can promiscuity 0 minmtu 0 maxmtu 0
> | can <FD,TDC_AUTO> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 100
> | bitrate 500000 sample-point 0.800
> | tq 25 prop-seg 31 phase-seg1 32 phase-seg2 16 sjw 1 brp 1
> | mcp251xfd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp_inc 1
> | dbitrate 2000000 dsample-point 0.800
> | dtq 25 dprop-seg 7 dphase-seg1 8 dphase-seg2 4 dsjw 1 dbrp 1
> | tdco 16
> | mcp251xfd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp_inc 1
> | tdco 0..127
> | clock 40000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 parentbus spi parentdev spi0.0

After the discussion I had with Stefan, I assumed mcp251xxfd also
used relative TDCO. However, in the mcp15xxfd family manual,
Equation 3-10: "Secondary Sample Point" on page 18 states that:

| SSP = TDCV + TDCO

As I commented above, this is the formula of the absolute
TDCO. Furthermore, in the example you shared, TDCO is
16 (absolute), not 0 (relative).

*BUT*, if this is the absolute TDCO, I just do not get how it can
be negative (I already elaborated on this in the past: if you
subtract from TDCV, you are measuring the previous bit...)

Another thing which is misleading to me is that the mcp15xxfd
family manual lists the min and max values for most of the
bittiming parameters but not for TDCO.

Finally, I did a bit of research and found that:
http://ww1.microchip.com/downloads/en/DeviceDoc/Section_56_Controller_Area_Network_with_Flexible_Data_rate_DS60001549A.pdf

This is *not* the mcp25xxfd datasheet but it is still from
Microship and as you will see, it is mostly similar to the
mcp25xxfd except for, you guessed it, the TDCO.

It reads:
| TDCMOD<1:0>: Transmitter Delay Compensation Mode bits
| Secondary Sample Point (SSP).
| 10 = Auto; measure delay and add CFDxDBTCFG.TSEG1; add TDCO
| 11 = Auto; measure delay and add CFDxDBTCFG.TSEG1; add TDCO
| 01 = Manual; Do not measure, use TDCV plus TDCO from the register
| 00 = Disable

| TDCO<6:0>: Transmitter Delay Compensation Offset bits
| Secondary Sample Point (SSP). Two's complement; offset can be
positive, zero, or negative.
| 1111111 = -64 x SYSCLK
| .
| .
| .
| 0111111 = 63 x SYSCLK
| .
| .
| .
| 0000000 = 0 x SYSCLK

Here, you can clearly see that the TDCO has the exact same range
as the one of the mcp25xxfd but the description of TDCMOD
changes, telling us that:

| SSP = TDCV (measured delay) + CFDxDBTCFG.TSEG1 (sample point) + TDCO

Which means this is a relative TDCO.

I just do not get how two documents from Microchip can have the
TDCO relative range of -64..63 but use a different formula. I am
sorry but at that point, I just do not understand what is going
on with your controller...

2021-08-16 14:31:55

On 16.08.2021 16:30:52, Marc Kleine-Budde wrote:
> > Finally, I did a bit of research and found that:
> > http://ww1.microchip.com/downloads/en/DeviceDoc/Section_56_Controller_Area_Network_with_Flexible_Data_rate_DS60001549A.pdf

> > This is *not* the mcp25xxfd datasheet but it is still from
> > Microship and as you will see, it is mostly similar to the
> > mcp25xxfd except for, you guessed it, the TDCO.
> >
> > It reads:
> > | TDCMOD<1:0>: Transmitter Delay Compensation Mode bits
> > | Secondary Sample Point (SSP).
> > | 10 = Auto; measure delay and add CFDxDBTCFG.TSEG1; add TDCO
> > | 11 = Auto; measure delay and add CFDxDBTCFG.TSEG1; add TDCO
> > | 01 = Manual; Do not measure, use TDCV plus TDCO from the register
> > | 00 = Disable
> >
> > | TDCO<6:0>: Transmitter Delay Compensation Offset bits
> > | Secondary Sample Point (SSP). Two's complement; offset can be
> > positive, zero, or negative.
> > | 1111111 = -64 x SYSCLK
> > | .
> > | .
> > | .
> > | 0111111 = 63 x SYSCLK
> > | .
> > | .
> > | .
> > | 0000000 = 0 x SYSCLK
> >
> > Here, you can clearly see that the TDCO has the exact same range
> > as the one of the mcp25xxfd but the description of TDCMOD
> > changes, telling us that:
> >
> > | SSP = TDCV (measured delay) + CFDxDBTCFG.TSEG1 (sample point) + TDCO
> >
> > Which means this is a relative TDCO.

Good catch! Microchip is investigating this.

regards,
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 |

Attachments:

(No filename) (1.67 kB)
signature.asc (499.00 B)
Download all attachments

2021-08-17 19:57:23

(No filename) (694.00 B)
signature.asc (499.00 B)
Download all attachments

2021-08-19 10:12:20

by Marc Kleine-Budde

[permalink] [raw]

Subject: Re: [PATCH v5 1/7] can: netlink: allow user to turn off unsupported features

On 19.08.2021 18:24:27, Vincent MAILHOL wrote:
> On Thu. 19 Aug 2021 at 16:45, Marc Kleine-Budde <[email protected]> wrote:
> > On 15.08.2021 12:32:42, Vincent Mailhol wrote:
> > > The sanity checks on the control modes will reject any request related
> > > to an unsupported features, even turning it off.
> > >
> > > Example on an interface which does not support CAN-FD:
> > >
> > > $ ip link set can0 type can bitrate 500000 fd off
> > > RTNETLINK answers: Operation not supported
> > >
> > > This patch lets such command go through (but requests to turn on an
> > > unsupported feature are, of course, still denied).
> > >
> > > Signed-off-by: Vincent Mailhol <[email protected]>
> >
> > I'm planing to send a pull request to net-next today. I want to do some
> > more tests with this series
>
> Ack, I am also preparing a new version. But first, I am just
> waiting for your reply on the tdc-mode {auto, manual, off}. :)

I want to do some proper testing, if it's now working as I'm expecting,
before continuing the discussion. :D

> > but this patch is more or less unrelated,
> > so I can take it in this PR, should I?
>
> FYI, the reason to add it to the series is that when setting TDC to
> off, the ip tool sets both CAN_CTRLMODE_TDC_AUTO and
> CAN_CTRLMODE_TDC_MANUAL to zero (which the corresponding bits in
> can_ctrlmode::mask set to 1). Without this patch, netlink would
> return -ENOTSUPP if the driver only supported one of
> CAN_CTRLMODE_TDC_{AUTO,MANUAL}.

Oh, I see.

> Regardless, this patch makes sense as a standalone, I am fine if
> you include it in your PR.

Why not, reduces the patch amount on your side :)

> Also, if you want, you can include the latest patch of the series as well:
> https://lore.kernel.org/linux-can/[email protected]/
>
> It's a comment fix, it should be pretty harmless.

Ok, makes sense.

regards,
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 |

Attachments:

(No filename) (2.17 kB)
signature.asc (499.00 B)
Download all attachments

2021-08-20 06:16:38

by Vincent MAILHOL

[permalink] [raw]

Subject: Re: [PATCH v5 2/7] can: bittiming: allow TDC{V,O} to be zero and add can_tdc_const::tdc{v,o,f}_min

On Wed. 18 Aug 2021 at 23:17, Vincent MAILHOL
<[email protected]> wrote:
> On Wed. 18 Aug 2021 at 21:29, Marc Kleine-Budde <[email protected]> wrote:
> > On 18.08.2021 18:22:33, Vincent MAILHOL wrote:
> > > > Backwards compatibility using an old ip tool on a new kernel/driver must
> > > > work.
> > >
> > > I am not trying to argue against backward compatibility :)
> > > My comment was just to point out that I had other intents as well.
> > >
> > > > In case of the mcp251xfd the tdc mode must be activated and tdcv
> > > > set to the automatic calculated value and tdco automatically measured.
> > >
> > > Sorry but I am not sure if I will follow you. Here, do you mean
> > > that "nothing" should do the "fully automated" calculation?
> >
> > Sort of.
> > The use case is the old ip tool with a driver that supports tdc, for
> > CAN-FD to work it must be configured in fully automated mode.
>
> The current patch does that: "nothing" means that both TDC_AUTO
> and TDC_MANUAL are not set, same as what an old ip tool would
> do. And that triggers the default (fully automated) mode (call
> can_calc_tdco()).
>
> > > In your previous message, you said:
> > >
> > > > Does it make sense to let "mode auto" without a tdco value switch the
> > > > controller into full automatic mode and /* nothing */ not tough the tdc
> > > > config at all?
> > >
> > > So, you would like this behavior:
> > >
> > > | mode auto, no tdco provided -> kernel decides between TDC_AUTO and TDC off.
> >
> > NACK - mode auto, no tdco -> TDC_AUTO with tdco calculated by the kernel
>
> Currently, the tdco calculation is paired with the decision to
> enable or not TDC. If dbrp is one or two, then do the tdco
> calculation, else, TDC is off (c.f. can_calc_tdco()). This
> behaviour is to follow ISO 11898-1 which states that TDC is only
> applicable if data BRP is one or two. In the current patch I
> allow to have TDC enabled with a dbrp greater than 2 only if the
> tdco is provided by the user (i.e. I allow the user to forcefully
> go against ISO but the automatic calculation guarantees
> compliance).
>
> So what do you suggest to do when drbp is greater than 2? Still
> enable TDC (and violate ISO standard) or return an error
> code (e.g. -ENOTSUPP)?
>
> > > | mode auto, tdco provided -> TDC_AUTO
> >
> > ACK - TDC_AUTO with user supplied tdco
> >
> > > | mode manual, tdcv and tdco provided -> TDC_MANUAL
> >
> > ACK - TDC_MANUAL with tdco and tdcv user provided
> >
> > > | mode off is not needed anymore (redundant with "nothing")
> > > (TDCF left out of the picture intentionally)
> >
> > NACK - TDC is switched off
>
> Same as the current patch then :)
>
> > > | "nothing" -> TDC is off (not touch the tdc config at all)
> >
> > NACK - do not touch TDC setting, use previous setting
>
> Sorry but I still fail to understand your definition of "do not
> touch".
>
> The first time you start a device, all the structures are zeroed
> out meaning that TDC is off to begin with. So the first time the
> user do something like:
>
> | ip link set can0 type can bitrate 1000000 dbitrate 8000000 fd on
>
> If you "do not touch" TDC it means that all TDC values stays at
> zero, i.e. TDC stays off. This would contradict point 1/.
>
> > > Correct?
> >
> > See above. Plus a change that addresses your issue 1/ from below.
> >
> > If driver supports TDC it should be initially brought into TDC auto
> > mode, if no TDC mode is given. Maybe we need an explizit TDC off to make
> > that work.
> >
> > > If you do so, I see three issues:
> > >
> > > 1/ Some of the drivers already implement TDC. Those will
> > > automatically do a calculation as long as FD is on. If "nothing"
> > > now brings TDC off, some users will find themselves with some
> > > error on the bus after the iproute2 update if they continue using
> > > the same command.
> >
> > Nothing would mean "do not touch" and as TDC auto is default a new ip
> > would work out of the box. Old ip will work, too. Just failing to decode
> > TDC_AUTO...
>
> See above: if you "do not touch", my understanding is that the
> old ip tool will indefinitely keep TDC to its initial value:
> everything zeroed out.
>
> To turn TDC auto, you will eventually call can_calc_tdco() and
> that will touch something.
>
> > > 2/ Users will need to read and understand how to use the TDC
> > > parameters of iproute2. And by experience, too many people just
> > > don't read the doc. If I can make the interface transparent and
> > > do the correct thing by default ("nothing"), I prefer to do so.
> >
> > ACK, see above
> >
> > > 3/ Final one is more of a nitpick. The mode auto might result in
> > > TDC being off. If we have a TDC_AUTO flag, I would expect the
> > > auto mode to always set that flag (unless error occurs). I see
> > > this to be slightly counter intuitive (I recognize that my
> > > solution also has some aspects which are not intuitive, I just
> > > want to point here that none are perfect).
> >
> > What are the corner cases where TDC_AUTO results in TDC off?
>
> dbrp greater than 2 (see above).
>
> > > To be honest, I really preferred the v1 of this series where
> > > there were no tdc-mode {auto,manual,off} and where the "off"
> > > behavior was controlled by setting TDCO to zero. However, as we
> > > realized, zero is a valid value and thus, I had to add all this
> > > complexity just to allow that damn zero value.
> >
> > Maybe we should not put the TDC mode _not_ into ctrl-mode, but info a
> > dedicated tdc-mode (which is not bit-field) inside the struct tdc?
>
> If you do so, then you would need both a tdcmode and a
> tdcmode_supported in order for the device to announce which modes
> it supports (same as the ctrlmode and ctrlmode_supported in
> can_priv). I seriously thought about this option but it seemed
> like reinventing the wheel for me.
>
> Also, it needs to be bit field to differentiate between a device
> which would only support manual mode, one device which would only
> support auto mode and one device which would support both.

I just realized something. If the user first sets the TDC
parameters and then does another command without any data
bittiming parameters provided, then the TDC parameters will be
recalculated but the other data bittiming parameters would remain
unchanged.

Example:
$ ip link set can0 type can bitrate 1000000 dbitrate 8000000 fd on
tdcv 33 tdco 16 tdc-mode manual
$ ip link set can0 type can bitrate 500000

Here, can_calc_tdco() will be triggered resulting in a switch to
TDC_AUTO mode.
In this scenario, it is not normal to only have the TDC
recalculated but not the other data bittiming parameters. Is it
what you were trying to explain when saying "do not touch"?

I am preparing a new series with below behavior:

* data bittiming not provided: TDC parameters unchanged
* data bittiming provided: (unchanged from current behavior)
- tdc-mode not provided: do can_calc_tdco (fully automated)
- tdc-mode auto and tdco provided: TDC_AUTO
- tdc-mode manual and both of tdcv and tdco provided: TDC_MANUAL

N.B. TDC parameters must be provided together with data bittiming
parameters, e.g. data bittiming not provided + TDC parameters is
an invalid command.

Does that make more sense?

Yours sincerely,
Vinent