linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "David.Wu" <wdc@rock-chips.com>
To: Doug Anderson <dianders@chromium.org>,
	David Wu <david.wu@rock-chips.com>
Cc: "Heiko Stübner" <heiko@sntech.de>,
	"Wolfram Sang" <wsa@the-dreams.de>,
	andy.shevchenko@gmail.com, "Tao Huang" <huangtao@rock-chips.com>,
	Chris <zyw@rock-chips.com>, "Eddie Cai" <cf@rock-chips.com>,
	"Jianqun Xu" <xjq@rock-chips.com>,
	"Lin Huang" <hl@rock-chips.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	linux-i2c@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 3/4] i2c: rk3x: new method to calculate i2c clocks
Date: Fri, 15 Jan 2016 17:39:13 +0800	[thread overview]
Message-ID: <5698BE41.30005@rock-chips.com> (raw)
In-Reply-To: <CAD=FV=UXMzEQiKcMa7VjDx+arcbRo1e-0HPHBnG8GP7k6uFt1g@mail.gmail.com>

Hi Doug,

在 2016/1/15 0:12, Doug Anderson 写道:
> Hi,
>
> On Thu, Jan 14, 2016 at 4:31 AM, David Wu <david.wu@rock-chips.com> wrote:
>> There was an timing issue about "repeated start" time at the I2C
>> controller of version0, controller appears to drop SDA at .875x (7/8)
>> programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
>> requirements on 100k's Standard-mode. To resolve this issue,
>> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
>> timing information are added, new rules are designed to calculate
>> the timing information at new v1.
> I'm curious: does new hardware behave differently and that's why you
> need v1?
Yes , i didn't describe clearly about difference between old and new.
Old and new hardware behave the same except some timing rules.

v0 is the same with the v1 for tHigh and tLow :
  tHigh = 8 * divh * pclk_cycle;
  tLow = 8 * divl * pclk_cycle;

v0 rules' difference:
     start setup: 7/8 * tHigh + 1 pclk cycle
     start hold :    2 * (7/8 * tHigh) - 1 pclk cycle
     stop setup :   7/8 * tHigh + 1 pclk cycle

     data setup:    1/2 tLow - 1 pclk cycle
     data hold :    1/2  tLow + 1  pclk cycle

For 100k's example:
     spec_min_low_ns = 4700;
     spec_min_high_ns = 4000;
     spec_min_setup_start_ns = 4700;

We could calculate the timing info by the rules of v0, and ignore effect 
of the pclk cycle here.
     tSU;sta >=4700;
     tHigh_min = tSU;sta * 8/7 >= 5372ns;
     tHigh_min  = max(tHigh_min, spec_min_high_ns) = 5372ns;

We get the final scl clk rate is 99k(1000000000 / (5372ns + 4700ns)),  
it looks ok for the 100k
Standard mode. But the timing point of repeat start and low time is very 
critical, it is dangerous
to some slave devices which are perhaps not so specified.
So need to give some time margin for them, that would increase the cycle 
time and reduce
the clk rate, the final rate we get may be 97k or 96k.

In fact, we must think about scl rise time and scl fall time, and get 
final clk may be 93k or less.
After that, we get about 7 percent lost at clk rate, it would reduce the 
efficiency of i2c transfer.

In other words, we could say there is a timing issue about "repeat 
start" time when we want
accurate 100k's rate, it is short to meet I2C spec requirements.
3.4M clk rate also has the same issue.

The start_setup count is added to fix this issue, tHigh is not need to 
considered of "repeat start" time.
After divs calculated, the count would be calculated to meet i2c spec.
v1 rule:
start setup: tHigh + 1 pclk cycle
start hold:  [8h * (u + 1) - 1] * T;
  tSU;sto = (8h * p + 1) * T;
---------------------------------------------------------------------------------------------------------------

The data_upd_st is added for the Data set-up time and Data hold time on 
Highspeed mode.
For 1.7M's example on v0:
     tHD;DAT = 1/2 tLow;
     tHD;DAT <= spec_max_data_hold_ns = 150ns
     tLow <= 2 * tHD;DAT <= 300ns;
     tLow >= spec_min_low_ns = 320ns;
According to these, we could not get a value for tLow, need changes for 
the rule: tHD;DAT = 1/2 tLow.

Cut the tLow into eight euqal parts,the range of data_upd_st is 1 ~ 7.
It seems that v1 rule could resolve the issue.
v1 rule:
     tHD;sda = n/8 * tLow;
     tSU;sda = [(8 - n)/8 * tLow;

3.4M clk rate also has the same issue.

>   ...or does old and new hardware behave the same and you're
> just introducing v1 so you don't mess with how old boards are working?
The registers of counts added would not effect old boards.
No matter what values of count was written in regs, that old i2c 
controller didn't care,
it worked by original timing rule.

After picking this patch, pmic-rk818 and touchscreen-gt911 worked well 
on the rk3368 sdk
board which use old method.

>
> >From the description it sounds as if the old code had problems as 100k too...
>
> If the new controller is different, I'd probably reword like the
> following (you'd have to re-wordwrap):
>
> There was an timing issue about "repeated start" time at the I2C
> controller of version0, controller appears to drop SDA at .875x (7/8)
> programmed clk high. On version 1 of the controller, the rule(.875x)
> isn't enough to meet tSU;STA
> requirements on 100k's Standard-mode. To resolve this issue,
> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
> timing information are added, new rules are designed to calculate
> the timing information at new v1.

There was an timing issue about "repeated start" time at the I2C
controller of version0, controller appears to drop SDA at .875x (7/8)
programmed clk high. On version 1 of the controller, the rule(.875x)
isn't enough to meet tSU;STA requirements on 100k's Standard-mode.

To resolve this issue,data_upd_st, start_setup_cnt and stop_setup_cnt
configs for I2C timing information are added, new rules are designed
to calculate the timing information at new v1.

> -Doug
>
>
>

      parent reply	other threads:[~2016-01-15  9:39 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1452774699-57455-1-git-send-email-david.wu@rock-chips.com>
     [not found] ` <1452774699-57455-3-git-send-email-david.wu@rock-chips.com>
     [not found]   ` <CAHp75VdR01_suo3nTWuuT2OEusy6z6KGSg29LKD8T3kY9eGMPA@mail.gmail.com>
2016-01-15  8:34     ` [PATCH v3 2/4] i2c: rk3x: add ops to caculate i2c clocks David.Wu
     [not found] ` <1452774699-57455-4-git-send-email-david.wu@rock-chips.com>
     [not found]   ` <CAD=FV=UXMzEQiKcMa7VjDx+arcbRo1e-0HPHBnG8GP7k6uFt1g@mail.gmail.com>
2016-01-15  9:39     ` David.Wu [this message]

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=5698BE41.30005@rock-chips.com \
    --to=wdc@rock-chips.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=cf@rock-chips.com \
    --cc=david.wu@rock-chips.com \
    --cc=dianders@chromium.org \
    --cc=heiko@sntech.de \
    --cc=hl@rock-chips.com \
    --cc=huangtao@rock-chips.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=wsa@the-dreams.de \
    --cc=xjq@rock-chips.com \
    --cc=zyw@rock-chips.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).