All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tomoya MORINAGA" <tomoya-linux@dsn.okisemi.com>
To: "Marc Kleine-Budde" <mkl@pengutronix.de>
Cc: <andrew.chih.howe.khor@intel.com>,
	<socketcan-core@lists.berlios.de>,
	"Samuel Ortiz" <sameo@linux.intel.com>, <margie.foster@intel.com>,
	<netdev@vger.kernel.org>,
	"Christian Pellegrin" <chripell@fsfe.org>,
	<linux-kernel@vger.kernel.org>, <yong.y.wang@intel.com>,
	<kok.howg.ewe@intel.com>, <joel.clark@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Wolfgang Grandegger" <wg@grandegger.com>, <qi.wang@intel.com>
Subject: Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
Date: Thu, 25 Nov 2010 21:03:45 +0900	[thread overview]
Message-ID: <001b01cb8c98$d06aaa00$66f8800a@maildom.okisemi.com> (raw)
In-Reply-To: 4CED0650.7030004@pengutronix.de

On Wednesday, November 24, 2010 9:34 PM, Marc Kleine-Budde wrote :
>On 11/24/2010 01:09 AM, Tomoya MORINAGA wrote:
>> On Monday, November 22, 2010 5:27 PM, Marc Kleine-Budde wrote:
>>>>>>> Still we have the busy waiting in the TX path. Maybe you can move the
>>>>>>> waiting before accessing the if[1] and remove the busy waiting here.
>>>>>> I can't understand your saying.
>>>>>> For transmitting data, calling pch_can_rw_msg_obj is mandatory.
>>>>> Yes, but the busy wait is not needed. It should be enough to do the
>>>>> busy-waiting _before_ accessing the if[1].
>>>>
>>>> Do you mean we should create other pch_can_rw_msg_obj which doesn't have busy wait ?
>>> ACK, and this non busy waiting is use in the TX path. But you add a busy
>>> wait only function before accessing the if[1] in the TX path.
>> 
>> The "busy waiting" of pch_can_rw_msg_obj is for next processing accesses to Message object.
>> If deleting this busy waiting, next processing can access to Message object, regardless previous transfer doesn't
>> complete yet.
>> Thus, I think, the "busy waiting" is necessary.
>
>Yes, it's necessary, but not where it is done currently.
>Let me outline how I think the TX path should look like:
>
>pch_xmit() {
> take_care_about_flow_control();
> prepare_can_frame_to_be_copied_to_tx_if();
>
> /* most likely we don't have to wait here */
> wait_until_tx_if_is_ready();
>
> copy_can_frame_to_tx_if();
>
> /* trigger tx in hardware */
> send_tx_if_but_dont_do_busywait();
>
> /* tx_if is busy now, but before we access it, we'll check tx_if is ready */
>}

This Tx path also has Read-Modify-Write for MessageRAM access.
Do you mean Tx path shouldn't have Read-Modify-Write ?


---
Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

WARNING: multiple messages have this Message-ID (diff)
From: "Tomoya MORINAGA" <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
To: "Marc Kleine-Budde" <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	qi.wang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	"David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
	Christian Pellegrin <chripell-VaTbYqLCNhc@public.gmane.org>,
	Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Subject: Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
Date: Thu, 25 Nov 2010 21:03:45 +0900	[thread overview]
Message-ID: <001b01cb8c98$d06aaa00$66f8800a@maildom.okisemi.com> (raw)
In-Reply-To: 4CED0650.7030004@pengutronix.de

On Wednesday, November 24, 2010 9:34 PM, Marc Kleine-Budde wrote :
>On 11/24/2010 01:09 AM, Tomoya MORINAGA wrote:
>> On Monday, November 22, 2010 5:27 PM, Marc Kleine-Budde wrote:
>>>>>>> Still we have the busy waiting in the TX path. Maybe you can move the
>>>>>>> waiting before accessing the if[1] and remove the busy waiting here.
>>>>>> I can't understand your saying.
>>>>>> For transmitting data, calling pch_can_rw_msg_obj is mandatory.
>>>>> Yes, but the busy wait is not needed. It should be enough to do the
>>>>> busy-waiting _before_ accessing the if[1].
>>>>
>>>> Do you mean we should create other pch_can_rw_msg_obj which doesn't have busy wait ?
>>> ACK, and this non busy waiting is use in the TX path. But you add a busy
>>> wait only function before accessing the if[1] in the TX path.
>> 
>> The "busy waiting" of pch_can_rw_msg_obj is for next processing accesses to Message object.
>> If deleting this busy waiting, next processing can access to Message object, regardless previous transfer doesn't
>> complete yet.
>> Thus, I think, the "busy waiting" is necessary.
>
>Yes, it's necessary, but not where it is done currently.
>Let me outline how I think the TX path should look like:
>
>pch_xmit() {
> take_care_about_flow_control();
> prepare_can_frame_to_be_copied_to_tx_if();
>
> /* most likely we don't have to wait here */
> wait_until_tx_if_is_ready();
>
> copy_can_frame_to_tx_if();
>
> /* trigger tx in hardware */
> send_tx_if_but_dont_do_busywait();
>
> /* tx_if is busy now, but before we access it, we'll check tx_if is ready */
>}

This Tx path also has Read-Modify-Write for MessageRAM access.
Do you mean Tx path shouldn't have Read-Modify-Write ?


---
Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

  reply	other threads:[~2010-11-25 12:03 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-16  8:39 [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control, Tomoya MORINAGA
2010-11-16 11:04 ` Wolfgang Grandegger
2010-11-16 12:22 ` Marc Kleine-Budde
2010-11-16 12:22   ` Marc Kleine-Budde
2010-11-19  6:18   ` Tomoya MORINAGA
2010-11-19  6:18     ` Tomoya MORINAGA
     [not found]     ` <007401cb87b1$a773b3d0$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>
2010-11-19  9:20       ` Marc Kleine-Budde
2010-11-22  5:05         ` Tomoya MORINAGA
2010-11-22  5:05           ` Tomoya MORINAGA
2010-11-22  8:27           ` Marc Kleine-Budde
2010-11-22  8:27             ` Marc Kleine-Budde
2010-11-24  0:09             ` Tomoya MORINAGA
2010-11-24  0:09               ` Tomoya MORINAGA
2010-11-24 12:34               ` Marc Kleine-Budde
2010-11-24 12:34                 ` Marc Kleine-Budde
2010-11-25 12:03                 ` Tomoya MORINAGA [this message]
2010-11-25 12:03                   ` Tomoya MORINAGA
2010-11-25 12:08                   ` Marc Kleine-Budde
2010-11-25 12:08                     ` Marc Kleine-Budde
2010-11-25 12:34                     ` Tomoya MORINAGA
2010-11-25 12:34                       ` Tomoya MORINAGA
2010-11-25 12:40                       ` Marc Kleine-Budde
2010-11-25 12:40                         ` Marc Kleine-Budde
  -- strict thread matches above, loose matches on Subject: below --
2010-11-16  8:39 Tomoya MORINAGA

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='001b01cb8c98$d06aaa00$66f8800a@maildom.okisemi.com' \
    --to=tomoya-linux@dsn.okisemi.com \
    --cc=andrew.chih.howe.khor@intel.com \
    --cc=chripell@fsfe.org \
    --cc=davem@davemloft.net \
    --cc=joel.clark@intel.com \
    --cc=kok.howg.ewe@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=margie.foster@intel.com \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=qi.wang@intel.com \
    --cc=sameo@linux.intel.com \
    --cc=socketcan-core@lists.berlios.de \
    --cc=wg@grandegger.com \
    --cc=yong.y.wang@intel.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.