On 10/19/2017 08:35 PM, Oliver Hartkopp wrote:

Hi Marc,

On 10/19/2017 01:26 PM, Marc Kleine-Budde wrote:

quoted

On 10/19/2017 01:14 PM, Oliver Hartkopp wrote:

quoted

quoted

quoted

quoted

quoted

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().

Actually I need some clarification. The sample point of the can core is
between the two time segments.
I always thought that the "sample point" options of the ip tool are used
in the internal
calculation of the two timing segments and is therefore no individual value.

quoted

quoted

quoted

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