Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates

From: Oliver Hartkopp
Date: Thu Oct 19 2017 - 14:35:25 EST


Hi Marc,

On 10/19/2017 01:26 PM, Marc Kleine-Budde wrote:
On 10/19/2017 01:14 PM, Oliver Hartkopp wrote:
Since we have a netlink socket interface to configure sample point, I
wonder if that should be extended to configure SSP too (or at least the
offset part of SSP)?

+1 too

The struct can_bittiming in defined in uapi, so we have to keep ABI
compatibility in mind.


Oh, this is fortunately NO problem ;-)

struct can_bittiming {
__u32 bitrate; /* Bit-rate in bits/second */
__u32 sample_point; /* Sample point in one-tenth of a percent */
__u32 tq; /* Time quanta (TQ) in nanoseconds */
__u32 prop_seg; /* Propagation segment in TQs */
__u32 phase_seg1; /* Phase buffer segment 1 in TQs */
__u32 phase_seg2; /* Phase buffer segment 2 in TQs */
__u32 sjw; /* Synchronisation jump width in TQs */
__u32 brp; /* Bit-rate prescaler */
};

So we have two of these: One for the arbitration bitrate and one sample_point for the data bitrate -> the 'secondary' SP -> SSP

:-)

We already have this 'dsample-point' implemented in the ip tool:

$ ip link set vcan0 type can help
Usage: ip link set DEVICE type can
[ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
[ tq TQ prop-seg PROP_SEG phase-seg1 PHASE-SEG1
phase-seg2 PHASE-SEG2 [ sjw SJW ] ]

[ dbitrate BITRATE [ dsample-point SAMPLE-POINT] ] | <<-- here!
[ dtq TQ dprop-seg PROP_SEG dphase-seg1 PHASE-SEG1
dphase-seg2 PHASE-SEG2 [ dsjw SJW ] ]

But AFAIK m_can is not using that value in m_can_set_bittiming().

If good default values are transceiver and board specific, they can go
into the DT. We need a generic (this means driver agnostic) binding for
this. If this table needs to be tweaked for special purpose, then we can
add a netlink interface for this as well. >
Comments?

By now we calculate reasonable default values (e.g. for SP and SJW), you
can override by setting alternative values via netlink configuration.

I would tend to stay on this approach and not hide these things in DTs -
just because of someone wants to initialize his specific interface 'easier'.

If the values are not board specific, then it makes no sense to put them
into the DT.

When they are NOT(?) board specific?

Thinking about non-SoC CAN adapters with PCI and USB pushing the SSP to the DT looks wrong to me.

Best,
Oliver