From: Roger Quadros <rogerq@ti.com>
To: wg@grandegger.com, mkl@pengutronix.de
Cc: tony@atomide.com, tglx@linutronix.de, mugunthanvnm@ti.com,
george.cherian@ti.com, balbi@ti.com, nsekhar@ti.com,
sergei.shtylyov@cogentembedded.com, linux-omap@vger.kernel.org,
linux-can@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout
Date: Tue, 9 Sep 2014 17:54:26 +0300 [thread overview]
Message-ID: <540F14A2.6000904@ti.com> (raw)
In-Reply-To: <540F13E4.7020907@ti.com>
On 09/09/2014 05:51 PM, Nishanth Menon wrote:
> On 09/09/2014 09:45 AM, Roger Quadros wrote:
> [...]
>>>> /* We look only at the bits of our instance. */
>>>> val &= mask;
>>>> - while ((readl(priv->raminit_ctrlreg) & mask) != val)
>>>> + while ((readl(priv->raminit_ctrlreg) & mask) != val) {
>>>> udelay(1);
>>>> + timeout++;
>>>> +
>>>> + if (timeout == 1000) {
>>>
>>> How did we come up with this number?
>>
>> wild guess ;), that it should be set in a few microseconds and the delay is not too
>> large.
>>
>> Till I don't hear from hardware guys, it will remain a guess.
>>
>
> in cases like these, I suggest using emperical data as point ->
> example doing some 10,000 iterations of the operation and picking up
> the worse case number and double it.
In my tests the bit was either set immediately or never at all.
Not sure if we should increase it further.
>
> Either way, you need to document the same, else a few years down the
> line, when that number is in question, no one will know what it's
> basis was..
>
OK. I'll add a comment there.
cheers,
-roger
WARNING: multiple messages have this Message-ID (diff)
From: Roger Quadros <rogerq@ti.com>
To: Nishanth Menon <nm@ti.com>, <wg@grandegger.com>, <mkl@pengutronix.de>
Cc: <tony@atomide.com>, <tglx@linutronix.de>, <mugunthanvnm@ti.com>,
<george.cherian@ti.com>, <balbi@ti.com>, <nsekhar@ti.com>,
<sergei.shtylyov@cogentembedded.com>,
<linux-omap@vger.kernel.org>, <linux-can@vger.kernel.org>,
<netdev@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout
Date: Tue, 9 Sep 2014 17:54:26 +0300 [thread overview]
Message-ID: <540F14A2.6000904@ti.com> (raw)
In-Reply-To: <540F13E4.7020907@ti.com>
On 09/09/2014 05:51 PM, Nishanth Menon wrote:
> On 09/09/2014 09:45 AM, Roger Quadros wrote:
> [...]
>>>> /* We look only at the bits of our instance. */
>>>> val &= mask;
>>>> - while ((readl(priv->raminit_ctrlreg) & mask) != val)
>>>> + while ((readl(priv->raminit_ctrlreg) & mask) != val) {
>>>> udelay(1);
>>>> + timeout++;
>>>> +
>>>> + if (timeout == 1000) {
>>>
>>> How did we come up with this number?
>>
>> wild guess ;), that it should be set in a few microseconds and the delay is not too
>> large.
>>
>> Till I don't hear from hardware guys, it will remain a guess.
>>
>
> in cases like these, I suggest using emperical data as point ->
> example doing some 10,000 iterations of the operation and picking up
> the worse case number and double it.
In my tests the bit was either set immediately or never at all.
Not sure if we should increase it further.
>
> Either way, you need to document the same, else a few years down the
> line, when that number is in question, no one will know what it's
> basis was..
>
OK. I'll add a comment there.
cheers,
-roger
next prev parent reply other threads:[~2014-09-09 14:54 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-09 14:31 [PATCH v2 0/3] net: can: Use syscon regmap for TI specific RAMINIT register Roger Quadros
2014-09-09 14:31 ` Roger Quadros
2014-09-09 14:31 ` [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout Roger Quadros
2014-09-09 14:31 ` Roger Quadros
2014-09-09 14:34 ` Marc Kleine-Budde
2014-09-09 14:34 ` Marc Kleine-Budde
2014-09-09 14:39 ` Roger Quadros
2014-09-09 14:39 ` Roger Quadros
2014-09-16 14:13 ` Marc Kleine-Budde
2014-09-16 14:13 ` Marc Kleine-Budde
2014-09-09 14:34 ` Nishanth Menon
2014-09-09 14:34 ` Nishanth Menon
2014-09-09 14:45 ` Roger Quadros
2014-09-09 14:45 ` Roger Quadros
2014-09-09 14:51 ` Nishanth Menon
2014-09-09 14:51 ` Nishanth Menon
2014-09-09 14:54 ` Roger Quadros [this message]
2014-09-09 14:54 ` Roger Quadros
2014-09-09 14:31 ` [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism Roger Quadros
2014-09-09 14:31 ` Roger Quadros
2014-09-30 13:26 ` Wolfram Sang
2014-09-30 13:26 ` Wolfram Sang
2014-09-30 13:33 ` Roger Quadros
2014-09-30 13:33 ` Roger Quadros
2014-09-30 13:52 ` Wolfram Sang
2014-09-30 13:52 ` Wolfram Sang
2014-09-30 13:58 ` Roger Quadros
2014-09-30 13:58 ` Roger Quadros
2014-09-30 14:19 ` Wolfram Sang
2014-09-30 14:19 ` Wolfram Sang
2014-09-30 14:22 ` Marc Kleine-Budde
2014-09-30 14:22 ` Marc Kleine-Budde
2014-09-30 14:49 ` Wolfram Sang
2014-09-30 14:49 ` Wolfram Sang
2014-09-30 15:01 ` Marc Kleine-Budde
2014-09-30 15:01 ` Marc Kleine-Budde
2014-09-30 15:25 ` Wolfram Sang
2014-09-30 15:25 ` Wolfram Sang
2014-09-30 16:04 ` Marc Kleine-Budde
2014-09-30 16:04 ` Marc Kleine-Budde
2014-10-01 8:45 ` Roger Quadros
2014-10-01 8:45 ` Roger Quadros
2014-10-01 8:47 ` Marc Kleine-Budde
2014-10-01 8:47 ` Marc Kleine-Budde
2014-10-01 9:06 ` Roger Quadros
2014-10-01 9:06 ` Roger Quadros
2014-10-01 10:01 ` Marc Kleine-Budde
2014-10-01 10:01 ` Marc Kleine-Budde
2014-10-01 10:12 ` Roger Quadros
2014-10-01 10:12 ` Roger Quadros
2014-10-01 10:26 ` Marc Kleine-Budde
2014-10-01 10:26 ` Marc Kleine-Budde
2014-10-01 10:43 ` Wolfram Sang
2014-10-01 10:43 ` Wolfram Sang
2014-10-01 10:57 ` Roger Quadros
2014-10-01 10:57 ` Roger Quadros
2014-10-01 11:06 ` Marc Kleine-Budde
2014-10-01 11:06 ` Marc Kleine-Budde
2014-10-01 11:10 ` Wolfram Sang
2014-10-01 11:10 ` Wolfram Sang
2014-10-01 11:11 ` Marc Kleine-Budde
2014-10-01 11:11 ` Marc Kleine-Budde
2014-09-30 13:45 ` Marc Kleine-Budde
2014-09-30 13:45 ` Marc Kleine-Budde
2014-09-30 14:02 ` Roger Quadros
2014-09-30 14:02 ` Roger Quadros
2014-09-30 14:11 ` Marc Kleine-Budde
2014-09-30 14:11 ` Marc Kleine-Budde
2014-09-09 14:31 ` [PATCH v2 3/3] net: can: c_can: Add support for START pulse in RAMINIT sequence Roger Quadros
2014-09-09 14:31 ` Roger Quadros
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=540F14A2.6000904@ti.com \
--to=rogerq@ti.com \
--cc=balbi@ti.com \
--cc=george.cherian@ti.com \
--cc=linux-can@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=mugunthanvnm@ti.com \
--cc=netdev@vger.kernel.org \
--cc=nsekhar@ti.com \
--cc=sergei.shtylyov@cogentembedded.com \
--cc=tglx@linutronix.de \
--cc=tony@atomide.com \
--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.