Bug 195009 - [patch]: [arm] Use 400 kHz as the default OMAP4 I2C bus speed
Summary: [patch]: [arm] Use 400 kHz as the default OMAP4 I2C bus speed
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: arm Any
: Normal Affects Only Me
Assignee: freebsd-arm (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-14 15:17 UTC by Scott Ellis
Modified: 2014-12-27 02:58 UTC (History)
2 users (show)

See Also:
bugmeister: mfc-stable10?
bugmeister: mfc-stable9?
bugmeister: mfc-stable8?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Ellis 2014-11-14 15:17:51 UTC
The I2C bus speed for OMAP4 boards defaults to 842 kHz.

The formula for calculating this comes from the TRM table 23-8

    scl = i2c_fclk / ( ( psc + 1) * ( (scll + 7) + (sclh + 5) ) )

The code says it's attempting 1 MHz, but it's using the wrong values.

The values for 400 kHz and 100 kHz are okay.

Here are the calculations

IIC_FASTEST = 96 MHz / ((5 + 1) * ((3 + 7) + (4 + 5))) = 96 MHz / (6 * 19) = 842 kHz

IIC_SLOW = 96 MHz / ((23 + 1) * ((13 + 7) + (15 + 5))) = 96 MHz / (24 * 40) = 100 kHz

IIC_FAST = 96 MHz / ((9 + 1) * ((5 + 7) + (7 + 5))) = 96 MHz / (10 * 24) = 400 kHz

To make the I2C bus friendlier for userland apps accessing /dev/iicX,
I'm proposing that IIC_FASTEST default to 400 kHz for OMAP4.


Index: sys/arm/ti/ti_i2c.c
===================================================================
--- sys/arm/ti/ti_i2c.c (revision 274498)
+++ sys/arm/ti/ti_i2c.c (working copy)
@@ -116,7 +116,7 @@
        { IIC_UNKNOWN,   100000, 23, 13, 15,  0, 0},
        { IIC_SLOW,      100000, 23, 13, 15,  0, 0},
        { IIC_FAST,      400000,  9,  5,  7,  0, 0},
-       { IIC_FASTEST,  1000000,  5,  3,  4,  0, 0},
+       { IIC_FASTEST,   400000,  9,  5,  7,  0, 0},
        /* { IIC_FASTEST, 3200000,  1, 113, 115, 7, 10}, - HS mode */
        { -1, 0 }
 };
Comment 1 Ian Lepore freebsd_committer freebsd_triage 2014-11-16 04:16:02 UTC
This isn't a great fix since the bus can actually run faster than 400KHz.  The real problem is that we've long lacked a way of configuring the speed of each i2c bus in a system, so I attacked the problem from that angle.  The patchset is available for review or download here at 

 https://reviews.freebsd.org/D1174

This will allow you to set the bus speed from device hints, from the FDT data, or from a loader.conf file using for example dev.iicbus.0.frequency=400000, or it can be changed on the fly using a sysctl with the same oid/path.
Comment 2 Luiz Otavio O Souza,+55 (14) 99772-1255 freebsd_committer freebsd_triage 2014-11-17 13:47:45 UTC
(In reply to Scott Ellis from comment #0)
> The I2C bus speed for OMAP4 boards defaults to 842 kHz.
> 
> The formula for calculating this comes from the TRM table 23-8
> 
>     scl = i2c_fclk / ( ( psc + 1) * ( (scll + 7) + (sclh + 5) ) )
> 
> The code says it's attempting 1 MHz, but it's using the wrong values.

Yes, that is right.

These values comes from TRM table 23-9 in "Fast Mode+".

The TRM doesn't take into account the '+ 1' for the prescaler value (psc), but only for Fast Mode+, all the other values seems correct.

For a internal clock frequency of 19.2 MHz we need to set the prescaler to 4:

96 MHz / (4 + 1) = 19.2 MHz

And then the bus frequency is:

19.2 MHz / (3 + 7) + (4 + 5) = 1.010 MHz

Good catch! Thanks!
Comment 3 commit-hook freebsd_committer freebsd_triage 2014-11-18 01:55:30 UTC
A commit references this bug:

Author: ian
Date: Tue Nov 18 01:54:33 UTC 2014
New revision: 274641
URL: https://svnweb.freebsd.org/changeset/base/274641

Log:
  Allow i2c bus speed to be configured via hints, FDT data, and sysctl.

  The current support for controlling i2c bus speed is an inconsistant mess.
  There are 4 symbolic speed values defined, UNKNOWN, SLOW, FAST, FASTEST.
  It seems to be universally assumed that SLOW means the standard 100KHz
  rate from the original spec.  Nothing ever calls iicbus_reset() with a
  speed of FAST, although some drivers would treat it as the 400KHz standard
  speed.  Mostly iicbus_reset() is called with the speed set to UNKNOWN or
  FASTEST, and there's really no telling what any individual driver will do
  with those.

  The speed of an i2c bus is limited by the speed of the slowest device on
  the bus.  This means that generally the bus speed needs to be configured
  based on the board/system and the components within it.  Historically for
  i2c we've configured with device hints.  Newer systems use FDT data and it
  documents a clock-frequency property for i2c busses.  Hobbyists and
  developers are likely to want on the fly changes.  These changes provide
  all 3 methods, but do not require any existing drivers to change to use
  the new facilities.

  This adds an iicbus method, iicbus_get_frequency(dev, speed) that gets the
  frequency for the requested symbolic speed.  If the symbolic speed is SLOW
  or if there is no speed configured for the bus, the returned value is
  100KHz, always.  Otherwise, if bus speed is configured by hints, fdt,
  tunable, or sysctl, that speed is returned.  It also adds a helper
  function, iicbus_init_frequency() that any bus driver subclassed from
  iicbus can initialize the frequency from some other source of info.

  Initial driver implementations are provided for Freescale and TI.

  Differential Revision:        https://reviews.freebsd.org/D1174
  PR:		195009

Changes:
  head/sys/arm/freescale/imx/imx_i2c.c
  head/sys/arm/ti/ti_i2c.c
  head/sys/dev/iicbus/iicbus.c
  head/sys/dev/iicbus/iicbus.h
  head/sys/dev/iicbus/iicbus_if.m
  head/sys/dev/ofw/ofw_iicbus.c
Comment 4 Ian Lepore freebsd_committer freebsd_triage 2014-11-18 02:02:15 UTC
(In reply to Luiz Otavio O Souza from comment #2)
> (In reply to Scott Ellis from comment #0)
> > The I2C bus speed for OMAP4 boards defaults to 842 kHz.
> > 
> > The formula for calculating this comes from the TRM table 23-8
> > 
> >     scl = i2c_fclk / ( ( psc + 1) * ( (scll + 7) + (sclh + 5) ) )
> > 
> > The code says it's attempting 1 MHz, but it's using the wrong values.
> 
> Yes, that is right.
> 
> These values comes from TRM table 23-9 in "Fast Mode+".
> 
> The TRM doesn't take into account the '+ 1' for the prescaler value (psc),
> but only for Fast Mode+, all the other values seems correct.
> 
> For a internal clock frequency of 19.2 MHz we need to set the prescaler to 4:
> 
> 96 MHz / (4 + 1) = 19.2 MHz
> 
> And then the bus frequency is:
> 
> 19.2 MHz / (3 + 7) + (4 + 5) = 1.010 MHz
> 
> Good catch! Thanks!

I'm thinking of fixing this a different way...

  { 1000000,  5,   1,   3,  0,  0},

That gives 96 / 6 = 16 MHz then 16 / (1 + 7 + 3 + 5) = 1 MHz exactly.  I've got another commit coming that fixes this and the fact that we've been running the i2c bus at half speed on beaglebone/am335x systems (root clock is 48mhz not 96).
Comment 5 commit-hook freebsd_committer freebsd_triage 2014-11-18 03:27:37 UTC
A commit references this bug:

Author: ian
Date: Tue Nov 18 03:26:52 UTC 2014
New revision: 274644
URL: https://svnweb.freebsd.org/changeset/base/274644

Log:
  Fix the i2c bus speed divisors for TI OMAP4 and AM335x.

  For OMAP4, the old values for 1MHz gave a bus frequency of about 890KHz.
  The new numbers hit 1MHz exactly.

  For AM335x the prescaler values are adjusted to give a 24MHz clock for
  all 3 standard speeds, as the manual recommends (as near as we can tell,
  there are errors and typos apparent in the document).  Also, 1MHz speed
  is added, and has been tested successfully on a BeagleboneWhite board.

  PR:		195009

Changes:
  head/sys/arm/ti/ti_i2c.c
Comment 6 Scott Ellis 2014-11-18 15:25:40 UTC
Running r274646 on an OMAP4 Duovero

Modifying i2c speeds through sysctl isn't working. sysctl says the speed has changed, but the when using the bus, the speed remains what it was at boot.

Modifying the speed through loader.conf or the dts file works.
Comment 7 Ian Lepore freebsd_committer freebsd_triage 2014-11-18 15:33:52 UTC
You need to reset the bus after changing the speed with sysctl to make the change take effect, i2c -r -f /dev/iic#.  I'm waiting for a review from the documentation team before I check in the new manpage that tells you that.  :)

Also, I forgot to mention in the commit message, but I checked the bus speeds with an oscilloscope on a beaglebone white to verify that the new divisors are correct.  I also verified that i2c eeprom devices can be read at all 3 speeds.  I don't have hardware to test omap4.
Comment 8 Scott Ellis 2014-11-18 16:15:52 UTC
Excellent! 

i2c -r -f /dev/iic# is working great here.
Comment 9 commit-hook freebsd_committer freebsd_triage 2014-11-21 21:30:42 UTC
A commit references this bug:

Author: ian
Date: Fri Nov 21 21:30:09 UTC 2014
New revision: 274822
URL: https://svnweb.freebsd.org/changeset/base/274822

Log:
  Document the recent enhancements for configuring bus speed in iicbus(4).

  Differential Revision:        https://reviews.freebsd.org/D1182
  PR:		195009

Changes:
  head/share/man/man4/iicbus.4
Comment 10 Scott Ellis 2014-12-02 14:35:39 UTC
The values chosen for the OMAP4 1MHz I2C speed aren't working for me
on either a Duovero or PandaBoard

{ 1000000,  5,   1,   3,  0,  0},

They generate the correct SCL clock speed, but the driver times out
when you make a request. 

It's the second mtx_sleep() in ti_i2c_transfer(), line 445 in 
ti_i2c.c that times out.

The error you get back is EWOULDBLOCK/EAGAIN.

Also either the SCL never stops running and you have to reset the
bus or the SCL goes idle again (high) but you cannot run a second
test without resetting the bus. You get no activity on the SCL line
on subsequent attempts.


I don't have an explanation, but the following values also give a
1 MHz SCL and the bus behaves properly when you use it. The error
you get back is a correct ENXIO if the device is not present. And 
the SCL clocks only the nine bits expected.


{ 1000000,  3,   5,   7,  0,  0},


These values also work.


{ 1000000,  1,  17,  19,  0,  0},


Something about the current values chosen for 1 MHz cause problems.


Here's how I'm testing the PandaBoard

I2C2 (/dev/iic1).
Reading SCL on J4-14
No device at the address I am accessing
Custom program doing an I2CRDWR request


Original { 5, 1, 3, } divider values

With a 560 ohm pullup, SCL runs at 1 MHz but never stops, error is EWOULDBLOCK
With a 1k ohm pullup, SCL runs at ~960 kHz, but only once without a reset, EWOULDBLOCK


Using { 3, 5, 7, } divider values

With a 560 ohm pullup, SCL runs ~960-985 kHz, error is ENXIO, repeated tests ok
With a 1k ohm pullup, SCL runs at ~960 kHz, ENXIO, repeated tests ok


I get similar results with the Duovero whether or not there is a device
that can respond. With the Duovero the clock always continues running
though.

And just to be clear, 100kHz and 400kHz work fine using the same test app
and the same busses and in the case of the Duovero, when there is a device
on the other end.


I can test further if there is a specific request.
Comment 11 commit-hook freebsd_committer freebsd_triage 2014-12-22 00:50:42 UTC
A commit references this bug:

Author: ian
Date: Mon Dec 22 00:50:02 UTC 2014
New revision: 276049
URL: https://svnweb.freebsd.org/changeset/base/276049

Log:
  Replace the clock divisor terms with values that also result in a 1 MHz
  clock, but actually work on real hardware, unlike the original set of
  values I chose.

  PR:		195009
  Submitted by:	Scott Ellis <jumpnowtek@gmail.com>

Changes:
  head/sys/arm/ti/ti_i2c.c
Comment 12 commit-hook freebsd_committer freebsd_triage 2014-12-27 02:38:08 UTC
A commit references this bug:

Author: ian
Date: Sat Dec 27 02:37:54 UTC 2014
New revision: 276278
URL: https://svnweb.freebsd.org/changeset/base/276278

Log:
  MFC r274641, r274644, r274822, r276049:

    Allow i2c bus speed to be configured via hints, FDT data, and sysctl.

    Implement bus speed setting for OMAP4, AM335x, and imx5/6.

    Fix the i2c bus speed divisors for TI OMAP4 and AM335x to give the
    advertised 100, 400, and 1000 KHz speeds.

  PR:		195009

Changes:
_U  stable/10/
  stable/10/share/man/man4/iicbus.4
  stable/10/sys/arm/freescale/imx/imx_i2c.c
  stable/10/sys/arm/ti/ti_i2c.c
  stable/10/sys/dev/iicbus/iicbus.c
  stable/10/sys/dev/iicbus/iicbus.h
  stable/10/sys/dev/iicbus/iicbus_if.m
  stable/10/sys/dev/ofw/ofw_iicbus.c
Comment 13 Ian Lepore freebsd_committer freebsd_triage 2014-12-27 02:58:19 UTC
Fixes applied and MFC'd to 10-stable.