From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752406AbcHKHPK (ORCPT ); Thu, 11 Aug 2016 03:15:10 -0400 Received: from mail1.bemta5.messagelabs.com ([195.245.231.153]:44009 "EHLO mail1.bemta5.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750724AbcHKHPI (ORCPT ); Thu, 11 Aug 2016 03:15:08 -0400 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprBKsWRWlGSWpSXmKPExsUS8J9tou531TX hBmsPs1msuraD0WLO+RYWi8ddM1gtVn2fymxxedccNov1i6awWBxbIGZx49U/doul93ayOnB6 bFl5k8nj46XbjB7N/4+we/T/NfDYfLra4/MmOY/+Sw8ZA9ijWDPzkvIrElgzJt9/w17QJlVx7 cdu9gbGnyJdjFwcQgINjBKz17exdzFyckgI+En8edjKCpGYzyhx89tZFpAEi4CqxNwZs1lBbD YBHYlz7c1gtgiQvfDfU2aQBmaBRUwS28/cZgJJCAuESPSfvMAMYvMK6ErsPjeNCWLqOyaJVcu 2QSUEJU7OfAK2gRlo0oLdn9i6GDmAbGmJ5f84QExOAQeJ2fPVQUxRARWJVwfrQYqFBJQkPr/o Y4a4WV5ib99isEYJgTiJL3dMIUxriYmbEycwCs9CsmkWkk2zEDYtYGRexahenFpUllqka66XV JSZnlGSm5iZo2toYKqXm1pcnJiempOYVKyXnJ+7iREYYwxAsIPx2GTnQ4ySHExKorzCMavDhf iS8lMqMxKLM+KLSnNSiw8xynBwKEnwyqusCRcSLEpNT61Iy8wBRjtMWoKDR0mEd5syUJq3uCA xtzgzHSJ1ilFRSpz3LUhCACSRUZoH1wZLMJcYZaWEeRmBDhHiKUgtys0sQZV/xSjOwagkzMsE sp0nM68EbvoroMVMQIuTVFeALC5JREhJNTCyRW3LZJka+8hK79vmpoUuXe4Xmyf+ltX5uNLaj HlChUnrJMmzXEvWRWgtf/aHo8ZsZe7akOefZqa+ei7quE7U5t30Y/YaLo9mml3LF2b8lj2/4E JMe/kGY/vq0oWzm2T7J2+Y1cu1QD/Iy4jjbNXvbN4zf+c8TXP+Z1f7jDli2sx7Vb84EjiUWIo zEg21mIuKEwEn2P7nKwMAAA== X-Env-Sender: Andreas.Werner@men.de X-Msg-Ref: server-15.tower-180.messagelabs.com!1470899702!38170852!1 X-Originating-IP: [80.255.6.145] X-StarScan-Received: X-StarScan-Version: 8.77; banners=-,-,- X-VirusChecked: Checked X-PGP-Universal: processed; by keys.men.de on Thu, 11 Aug 2016 09:15:03 +0200 Date: Thu, 11 Aug 2016 09:14:59 +0200 From: Andreas Werner To: Oliver Hartkopp CC: Andreas Werner , Wolfgang Grandegger , , , , , , , , Subject: Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver Message-ID: <20160811071458.GA25813@awelinux> References: <20160726091555.GA26227@awelinux> <9487fe77-dbd0-3a64-b648-cf76b897feda@grandegger.com> <20160808113938.GA17459@awelinux> <456c4f82-6a53-e821-3396-63d413db9eb7@grandegger.com> <20160808140546.GA1733@awelinux> <808e919b-8643-2113-9fde-001861883abe@grandegger.com> <20160809061038.GC1733@awelinux> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [192.1.1.170] X-ClientProxiedBy: MEN-EX01.intra.men.de (192.168.1.1) To MEN-EX01.intra.men.de (192.168.1.1) X-EXCLAIMER-MD-CONFIG: e4841e51-7998-49c0-ba41-8b8a0e2d8962 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Aug 10, 2016 at 10:28:45PM +0200, Oliver Hartkopp wrote: > 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 . > Ok I am with you. > >>>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. > . Thanks for the explanation. I make it more clear why its mandatory. > >> > >>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. > I will check those driver to get more information about the implementation. > 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. > At the moment, i think there is no way to detect those looped frames. I will talk to our IC designer and discuss this issue with him. Maybe we have the possibility to get a local loopback inside the CAN controller. This seems to be the best way to do it. > 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. > I agree with you. This is what we do not want to have. > Regards, > Oliver > > ps. Do you have any URL where one can get the MEN 16Z192 spec? No sorry, its not available. Regards Andy