On 07/10/2012 08:09 AM, Wolfgang Grandegger wrote:

Hi Ira,

On 07/09/2012 09:56 PM, Ira W. Snyder wrote:

quoted

From: "Ira W. Snyder" <redacted>

This is another different approach to fixing the Janz ICAN3 support for
CAN_RAW_RECV_OWN_MSGS.

The can_put_echo_skb() function is changed to always keep all packets
until can_get_echo_skb() is called. Previously, it would drop packets if
they were not needed to be looped back. This makes it possible for the
new function can_cmp_echo_skb() to work with hardware-assisted loopback
support on the Janz ICAN3, which does not have TX-complete interrupts of
any kind.

Since we are now storing packets even in the non-loopback case, there is
some extra memory overhead, to store the extra packets between the calls
to can_put_echo_skb() and can_get_echo_skb().

Well, I don't like such device specific quirk going into the common
interface.

Be aware, all other drivers will suffer from that change!

quoted

After this patch series is applied, the SocketCAN tst-rcv-own-msgs test
passes.

Performance is rougly 15% less than using the previously posted patch:
[PATCH ALTERNATE VERSION 1/1] can: janz-ican3: fix support for CAN_RAW_RECV_OWN_MSGS

Are the 15% only relevant if CAN_RAW_RECV_OWN_MSGS is off?

Oops, that's a lot and a clear contra.

quoted

This performance drop is due to the extra overhead of receiving the echo
packets from the card itself. This involves one extra interrupt for each
packet sent, and the associated overhead of running ican3_napi() for
each packet sent.

Do we really want that. I agree that CAN_RAW_RECV_OWN_MSGS should work
as expected but we should not try hard to partially fix the timing
issue. Why do we not just use the default protocol callback (flags &=
!IFF_ECHO) if the hardware cannot do it better.

Well, getting a looped back message even if it did not go out to the bus
is not nice either.

What about a device specific solution where you do the dropping of
looped back messages only if CAN_RAW_RECV_OWN_MSGS is off.

Wolfgang,