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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4b53c9ff-395f-670f-b55c-38ccfb37ac30@gmx.net>
Date:   Thu, 19 Oct 2017 21:54:54 +0200
From:   Mario Hüttel <mario.huettel@....net>
To:     Oliver Hartkopp <socketcan@...tkopp.net>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        Sekhar Nori <nsekhar@...com>,
        Franklin S Cooper Jr <fcooper@...com>,
        "Yang, Wenyou" <Wenyou.Yang@...rochip.com>, wg@...ndegger.com,
        quentin.schulz@...e-electrons.com, edumazet@...gle.com,
        linux-can@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Wenyou Yang <wenyou.yang@...el.com>,
        Dong Aisheng <b29396@...escale.com>,
        "Quadros, Roger" <rogerq@...com>
Subject: Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates


On 10/19/2017 08:35 PM, Oliver Hartkopp wrote:
> 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().
>
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.

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


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)