lkml.org 
[lkml]   [2016]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver
From
Date
Hi Andreas,

On 08/09/2016 08:10 AM, Andreas Werner wrote:
> On Mon, Aug 08, 2016 at 04:35:34PM +0200, Wolfgang Grandegger wrote:

>>>>>> You specify here one echo_skb but it's not used anywhere. Local loopback
>>>>>> seems not to be implemented.
>>>>>>
>>>>>
>>>>> Agree with you, will set it to "0".
>>>>
>>>> No, the local loopback is mandetory!
>>>>
>>>
>>> Hm ok, but if i check alloc_candev() in drivers/net/can/dev.c
>>> it is not mandatory.

It is.

Even those drivers that show up to use zero echo skbs in alloc_candev()
implement the echo functionality correct.

Just check 'git grep IFF_ECHO'. Even grcan.c and janz-ican3.c have
IFF_ECHO set - but implement it in a different way without using the
provided machanism from dev.c .

>>> In the Documentation/networking/can.txt
>>> there is also a "should" and a fallback mechnism if the driver
>>> does not support the local loopback.

But this fallback mechanism is bad - really bad!

E.g. the slcan.c driver sends a stream of CAN frames without knowing
whether the frames ever hit the wire. The slcan driver is more less for
hobby users. The CAN frame echo with IFF_ECHO gives a correct
representation of the traffic on the wire - including the correct
timestamps.

You really want to know whether a CAN frame was sent correctly on the
bus instead of getting some shortcut info from inside the network layer.
.
>>
>> Well, s/driver/hardware/ ! Local loopback is the preferred mechanism.
>>
>
> Sure...
>
>>> I'm currently ok with this fallback mechanism.

/me not.

>>> Anyway I am not sure that the driver can handle the echo skb correctly.
>>> If i understand it correctly, the can_get_echo_skb() is normally called
>>> on a "TX done IRQ" to get the skb and loop it back.

ack.

>>> I do not have such a "TX done IRQ" and have not implemented implemented
>>> and added the local looback.

I'm not really sure how grcan.c and janz-ican3.c implemented the echo
functionality but they must have faced a similar situation.

A local loopback inside the CAN controller which is generated after
successful transmit is an excellent implementation with excellent
timestamps. The only problem for you is to detect the looped CAN frames
and match them to the skb pointer of the outgoing frame to 'receive' the
correct echo skb.

When you send CAN frames to an unconnected CAN bus it can't be sent out
due to the missing acknowledge from other nodes. So when you send frames
and you get echo frames due to the fallback mode your cool CAN
controller degrades to slcan level.

Regards,
Oliver

ps. Do you have any URL where one can get the MEN 16Z192 spec?

\
 
 \ /
  Last update: 2016-08-10 23:01    [W:0.070 / U:9.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site