Message-ID: <20130828153429.GB7066@ab42.lan>
Date:	Wed, 28 Aug 2013 17:34:31 +0200
From:	Christian Ruppert <christian.ruppert@...lis.com>
To:	Shinya Kuribayashi <skuribay@...ox.com>
Cc:	mika.westerberg@...ux.intel.com, linux-i2c@...r.kernel.org,
	wsa@...-dreams.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] i2c-designware: make *CNT values configurable

On Sat, Aug 24, 2013 at 01:58:47PM +0900, Shinya Kuribayashi wrote:
> On 8/21/13 11:39 PM, Christian Ruppert wrote:
> >On Fri, Aug 16, 2013 at 11:15:12AM +0900, Shinya Kuribayashi wrote:
> >>On 8/5/13 6:31 PM, Christian Ruppert wrote:
> >>>On Wed, Jul 24, 2013 at 11:31:44PM +0900, Shinya Kuribayashi wrote:
> >>>>As said before, all t_SCL things should go away.  Please forget
> >>>>about 100kbps, 400kbps, and so on.  Bus/clock speed is totally
> >>>>pointless concept for the I2C bus systems.  For example, as long
> >>>>as tr/tf, tHIGH/tLOW, tHD;STA, etc. are met by _all_ devices in a
> >>>>certain I2C bus, it doesn't matter that the resulting clock speed
> >>>>is, say 120 kbps with Standard-mode, or even 800 kbps for Fast-mode.
> >>>>Nobody in the I2C bus doesn't care about actual bus/clock speed.
> >>>>
> >>>>We don't have to care about the resulting bus speed, or rather
> >>>>we should/must not check to see if it's within the proper range.
> >>>
> >>>Actually, the I2C specification clearly defines f_SCL;max (and thus
> >>>implies t_SCL;min), both in the tables and the timing diagrams. Why can
> >>>we ignore this constraint while having to meet all the others?
> >>
> >>If we meet t_r, t_f, t_HIGH, t_LOW (and t_HIGH in this DW case),
> >>f_SCL;max will be met by itself.
> >
> >I'm not sure if I agree with this:
> >
> >Standard mode:
> >        t_r;min          0ns
> >        t_f;min     +    0ns
> >        t_HIGH;min  + 4000ns
> >        t_LOW;min   + 4700ns
> >        1/f_SCL     = 8700ns
> >    ==> f_SCL       = 115kHz    ==>    violation of f_SCL;max=100kHz
> >
> >Fast mode (let's assume V_DD = 5.5V):
> >        t_r;min         20ns
> >        t_f;min     +   20ns
> >        t_HIGH;min  +  600ns
> >        t_LIW;min   + 1300ns
> >        1/f_SCL     = 1940ns
> >    ==> f_SCL       = 515kHz    ==>    violation of f_SCL;max=400kHz
> 
> It's more realistic to calculate with say 50ns < tr,tf < 300ns,
> than with tt = tf = 0ns or <20ns.  Even if with such real tf/tr
> times, there is cases where f_SCL can be greater than 100/400Hz.

I agree. I just used these values to illustrate my point.

> I understand what you mean, but that was not my point.  See below.
> 
> >>And again, all I2C master and
> >>slave devices in the bus don't care about f_SCL; what they do care
> >>are t_f, t_r, t_HIGH, t_LOW, and so on.  That's why I'm saying
> >>f_SCL is pointless and has no value for HCNT/LCNT calculations.
> >
> >I partially agree: If I2C devices don't care about f_SCL but only about
> >t_r, t_f, t_HIGH and t_LOW there's no need to respect the f_SCL;max
> >constraint. If this is the case, I'm wondering why it is part of the
> >specification, though.
> 
> With t_r;max and t_f;max,
> 
> Standard mode:
>         t_r;max       1000ns (max time applied)
>         t_f;max     +  300ns (max time applied)
>         t_HIGH;min  + 4000ns
>         t_LOW;min   + 4700ns
>         1/f_SCL     =10000ns
>     ==> f_SCL       = 100kHz    ==> f_SCL;max for Standard-mode
> 
> Fast mode:
>         t_r;max        300ns (max time applied)
>         t_f;max     +  300ns (max time applied)
>         t_HIGH;min  +  600ns
>         t_LIW;min   + 1300ns
>         1/f_SCL     = 2500ns
>     ==> f_SCL       = 400kHz    ==> f_SCL;max for Fast-mode
> 
> f_SCL;max is defined as a resulting clock frequency with the
> combination of:
> 
> (1) the max. conditions of t_r and t_f
> (2) the min. conditions of t_HIGH and t_LOW
> 
> We can try to meet t_HIGH;min and t_LOW;min, but we can't meet
> t_r;min nor t_f;min in the actual systems.  The t_r and t_f are
> _minimum requisites_ for the I/O buffer characteristic of the
> master and the board designs, hence necessarily contain some
> time margin.

I agree.

> f_SCL is anything more than the resulting speed of (1) + (2),
> so I don't think we need to adjust HCNT/LCNT values at all.
> If with t_r < t_r;max and t_f < t_f;max, and you've got a faster
> clock speed than f_SCL;max, then it's a bonus and we can accept
> it gratefully.

Here I'm not sure: The I2C specification clearly states f_SCL min/max
requirements in table 10. I'm feeling a bit uneasy patching a driver
with (to date) rather pessimistic bus timings to something which might
be over-optimistic. If I submit a patch for this I would like to be sure
not to break existing systems. How do other drivers manage this?

> >>I'd make a compromise proposal; it's fine to make sure whether the
> >>resulting f_SCL is within a supported range, but should not make a
> >>correction of HCNT/LCNT values.  Just report warning messages that
> >>some parameters/calculations might be mis-configured an/or wrong.
> >
> >Not sure if this is a good idea: If f_SCL is met by design I'm perfectly
> >happy with dropping the t_HIGH/t_LOW adjustment code, no need to bloat
> >the kernel with confusing warnings. If f_SCL is not automatically met we
> >must either make sure t_HIGH/t_LOW are adjusted or we must take the
> >decision to ignore that constraint and document the reasons behind that
> >decision accordingly.
> 
> I tried to write my thought down, not sure well-explained, though.
> 
> Notes:
> 
> * As long as tHD;SDA issue remains in the DW I2C core, we need to
>   have t_HIGH with a relatively lager value than necessary.  In
>   such a case, the resulting f_SCL can never exceed f_SCL;max.

Not in my calculations (Let's assume fast mode with all rise and fall
times being 50ns as you say above and f_sys = 50MHz):
HCNT = f_SYS * (t_HD;STA + t_f;SDA) - 3
-> t_HIGH = t_HD_STA + t_f;SDA + 4 / f_sys
          = 0.6us + 0.05 us + 0.08 us = 0.73us
LCNT = f_SYS * (t_f;SCL + t_LOW)
-> t_LOW  = 0.05us + 1.3us = 1.35us

-> f_SCL = 1 / (t_HIGH + t_LOW + t_r;SCL + t_f;SCL)
         = 1 / (0.73us + 1.35 us + 0.05 us + 0.05us) = 458MHz  > 400MHz

> * I also wonder which values do you think should be adjusted to meet
>   f_SCL;max, HCNT or LCNT, and why is that?  I think it's hard to
>   explain, isn't it?

Originally I thought it would be best to adjust both in the same way.
Thinking about your question I guess it might be more in the spirit of
I2C to only adjust LCNT. The specification (table 10) allows for both,
there are no max values specified for either.

Greetings,
  Christian

-- 
  Christian Ruppert              ,          <christian.ruppert@...lis.com>
                                /|
  Tel: +41/(0)22 816 19-42     //|                 3, Chemin du Pré-Fleuri
                             _// | bilis Systems   CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists