All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Werner <andreas.werner@men.de>
To: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Andreas Werner <andreas.werner@men.de>,
	Wolfgang Grandegger <wg@grandegger.com>,
	mkl@pengutronix.de, linux-can@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	davem@davemloft.net, jthumshirn@suse.de, andy@wernerandy.de,
	michael.miehling@men.de
Subject: Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver
Date: Thu, 11 Aug 2016 10:58:26 +0200	[thread overview]
Message-ID: <20160811085826.GB25813@awelinux> (raw)
In-Reply-To: <1d81436b-4ad2-a34f-e2e1-accde0b16a47@hartkopp.net>

On Thu, Aug 11, 2016 at 10:45:00AM +0200, Oliver Hartkopp wrote:
> On 08/11/2016 09:14 AM, Andreas Werner wrote:
> >On Wed, Aug 10, 2016 at 10:28:45PM +0200, Oliver Hartkopp wrote:
> 
> >>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.
> 
> Great :-)
> 
> >>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 still have the possibility to change the IP core I would suggest to
> create some kind of 16/32 bit value which you can pass to the CAN controller
> along with the CAN frame to be sent.
> 
> And when this frame comes back due to the loopback you can use this non-zero
> 16/32 bit value to match into a list of tx skb pointers for IFF_ECHO.
> 
> E.g. when this 16/32 bit value is zero this CAN frame obviously was received
> from another CAN node.
> 
> Just an idea.
> 

I am not sure if we have a way to change the IP but i will try to talk with
my IC designer. He will be available next week.

Your idea sounds good. I will check a few more driver to get more information
how they did the implementation.

> Regards,
> Oliver

Thanks your comments and explanations Oliver.

Regards
Andy

WARNING: multiple messages have this Message-ID (diff)
From: Andreas Werner <andreas.werner@men.de>
To: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Andreas Werner <andreas.werner@men.de>,
	Wolfgang Grandegger <wg@grandegger.com>, <mkl@pengutronix.de>,
	<linux-can@vger.kernel.org>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <davem@davemloft.net>,
	<jthumshirn@suse.de>, <andy@wernerandy.de>,
	<michael.miehling@men.de>
Subject: Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver
Date: Thu, 11 Aug 2016 10:58:26 +0200	[thread overview]
Message-ID: <20160811085826.GB25813@awelinux> (raw)
In-Reply-To: <1d81436b-4ad2-a34f-e2e1-accde0b16a47@hartkopp.net>

On Thu, Aug 11, 2016 at 10:45:00AM +0200, Oliver Hartkopp wrote:
> On 08/11/2016 09:14 AM, Andreas Werner wrote:
> >On Wed, Aug 10, 2016 at 10:28:45PM +0200, Oliver Hartkopp wrote:
> 
> >>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.
> 
> Great :-)
> 
> >>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 still have the possibility to change the IP core I would suggest to
> create some kind of 16/32 bit value which you can pass to the CAN controller
> along with the CAN frame to be sent.
> 
> And when this frame comes back due to the loopback you can use this non-zero
> 16/32 bit value to match into a list of tx skb pointers for IFF_ECHO.
> 
> E.g. when this 16/32 bit value is zero this CAN frame obviously was received
> from another CAN node.
> 
> Just an idea.
> 

I am not sure if we have a way to change the IP but i will try to talk with
my IC designer. He will be available next week.

Your idea sounds good. I will check a few more driver to get more information
how they did the implementation.

> Regards,
> Oliver

Thanks your comments and explanations Oliver.

Regards
Andy

  reply	other threads:[~2016-08-11  8:59 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-26  9:16 [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver Andreas Werner
2016-07-26  9:16 ` Andreas Werner
2016-08-08  3:58 ` Benjamin Poirier
2016-08-08  7:26   ` Andreas Werner
2016-08-08  7:26     ` Andreas Werner
2016-08-09  3:23     ` Benjamin Poirier
2016-08-09  6:11       ` Andreas Werner
2016-08-09  6:11         ` Andreas Werner
2016-08-08  9:27 ` Wolfgang Grandegger
2016-08-08 11:39   ` Andreas Werner
2016-08-08 11:39     ` Andreas Werner
2016-08-08 12:28     ` Wolfgang Grandegger
2016-08-08 13:06       ` Kurt Van Dijck
2016-08-08 14:12         ` Andreas Werner
2016-08-08 14:12           ` Andreas Werner
2016-08-08 14:05       ` Andreas Werner
2016-08-08 14:05         ` Andreas Werner
2016-08-08 14:35         ` Wolfgang Grandegger
2016-08-09  6:10           ` Andreas Werner
2016-08-09  6:10             ` Andreas Werner
2016-08-09 11:54             ` Wolfgang Grandegger
2016-08-10 20:28             ` Oliver Hartkopp
2016-08-11  7:14               ` Andreas Werner
2016-08-11  7:14                 ` Andreas Werner
2016-08-11  8:45                 ` Oliver Hartkopp
2016-08-11  8:58                   ` Andreas Werner [this message]
2016-08-11  8:58                     ` Andreas Werner
2016-08-11 11:46                     ` Oliver Hartkopp
2016-08-09  9:35 ` Ramesh Shanmugasundaram

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160811085826.GB25813@awelinux \
    --to=andreas.werner@men.de \
    --cc=andy@wernerandy.de \
    --cc=davem@davemloft.net \
    --cc=jthumshirn@suse.de \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.miehling@men.de \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=socketcan@hartkopp.net \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.