[Bug 1001397] New: I2C driver for Kinetic microcontrollers

[Bug 1001397] New: I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

           Summary: I2C driver for Kinetic microcontrollers
           Product: eCos
           Version: CVS
          Platform: All
        OS/Version: Other
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: low
         Component: I2C
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: tf+bugs.ecos@r-finger.com
                CC: ecos-bugs@ecos.sourceware.org, ilijak@siva.com.mk
             Class: Advice Request


Created an attachment (id=1443)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1443)
Kinetis i2c driver

The attached patch provides a basic (7 bit addressing only for now) i2c driver
for Freescale Kinetis K40 and K60 microcontrollers. The driver is formally
based on the lm3s driver implementation and is irq driven, and been tested on
the TWR-K60N512 board. (The patch applies over the GPIO patch attached to bug
1001388.)

Any feedback would be welcome.

PS. I am in process of sorting out copyright assignment with the FSF.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #1 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-01 15:24:01 GMT ---
Tomas,

Thank you for contribution. I will try to get some time to test your code as
soon as I can, but it may take some time. Anyway we are waiting for copyright
assignment.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ecos-patches@ecos.sourcewar
                   |                            |e.org
          Component|I2C                         |Patches and contributions
         OS/Version|Other                       |Cortex-M

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |1001388

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #2 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2011-12-01 15:58:38 GMT ---
There is no rush, I just wanted to get this out, in case there are any formal
changes required. I am working on an a driver for the MMA7660 accelerometer
(included on the TWR boards), which provides one way of testing i2c in both
directions; it's not ready yet, but not far off. I'll post that into a separate
bug eventually.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |1001419

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #3 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2011-12-23 10:40:41 GMT ---
In the light of the discussion in bug 1001428, I think the hal bits in this
patch should be moved under hal/misc/freescale/i2c?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

John Dallaway <john@dallaway.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |john@dallaway.org.uk

--- Comment #4 from John Dallaway <john@dallaway.org.uk> 2011-12-23 13:27:33 GMT ---
(In reply to comment #3)
> In the light of the discussion in bug 1001428, I think the hal bits in this
> patch should be moved under hal/misc/freescale/i2c?

Tomas, will this package be applicable to _all_ Freescale I2C peripheral blocks
across all Freescale product families? If not, please make the "i2c" directory
name (and corresponding eCos package name) less generic. In an ideal world, all
peripheral blocks which are shared across multiple architectures would have
unique names assigned by the manufacturer.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #5 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-23 14:12:07 GMT ---
(In reply to comment #3)
> In the light of the discussion in bug 1001428, I think the hal bits in this
> patch should be moved under hal/misc/freescale/i2c?

(In reply to comment #3)
> In the light of the discussion in bug 1001428, I think the hal bits in this
> patch should be moved under hal/misc/freescale/i2c?


I assume you are referring to i2c header. They should stay with driver source,
either in driver .c file or (preferably) in a header.
As there is existing devs/i2c directory, I propose following path:
    - devs/i2c/freescale/<freescale_i2c>

where <freescale_i2c> is the name of this particular freescale spi instance
(typically as referred to in ref. man.).

Besides the device structure itself, there is some information provided by HAL
(base address(es), pin configuration or so. They can be be considered variant
or platform dependent and so defined in var_io_devs.h and/or plf_io.h
respectively. If defined in both then plf shall override var.

I hope in future to produce some docs but in meantime pse refer to ENET driver
for example.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #6 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-23 14:15:09 GMT ---
(In reply to comment #5)
> (In reply to comment #3)
> > In the light of the discussion in bug 1001428, I think the hal bits in this
> > patch should be moved under hal/misc/freescale/i2c?
> 
> (In reply to comment #3)
> > In the light of the discussion in bug 1001428, I think the hal bits in this
> > patch should be moved under hal/misc/freescale/i2c?
> 
> 
> I assume you are referring to i2c header. They should stay with driver source,
> either in driver .c file or (preferably) in a header.
> As there is existing devs/i2c directory, I propose following path:
>     - devs/i2c/freescale/<freescale_i2c>
> 
(Just to clarify this is path to device that will have usual subtree
             <version>/cdl
                      /include
                      /src
                      /ChangeLog

> where <freescale_i2c> is the name of this particular freescale spi instance
> (typically as referred to in ref. man.).
> 
> Besides the device structure itself, there is some information provided by HAL
> (base address(es), pin configuration or so. They can be be considered variant
> or platform dependent and so defined in var_io_devs.h and/or plf_io.h
> respectively. If defined in both then plf shall override var.
> 
> I hope in future to produce some docs but in meantime pse refer to ENET driver
> for example.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #7 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2011-12-24 09:21:52 GMT ---
(In reply to comment #5)
> As there is existing devs/i2c directory, I propose following path:
>     - devs/i2c/freescale/<freescale_i2c>

OK, but the situation is exactly the same for the SLCD controller, and so we
end up with bits under devs/ and bits under hal/misc, with no clear logic for
the distinction I can see.


> where <freescale_i2c> is the name of this particular freescale spi instance
> (typically as referred to in ref. man.).

The services have only generic names in the Freescale RMs, so we end up with
very generic names. That is fine if Freescale only make a single i2c controller
and never will design another one, perhaps that is the case?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #8 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-24 11:58:57 GMT ---
(In reply to comment #7)
> (In reply to comment #5)
> > As there is existing devs/i2c directory, I propose following path:
> >     - devs/i2c/freescale/<freescale_i2c>
> 
> OK, but the situation is exactly the same for the SLCD controller, and so we
> end up with bits under devs/ and bits under hal/misc, with no clear logic for
> the distinction I can see.
> 

We need to distinguish/combine two sources of information:
   Device structure (registers, bits)
   Device instantiation (base address, pins, etc).

Device structure is needed only by the driver and should live there (in
driver's .c file or in a header file). Also the naming should be in device name
space, common patterns are: CYGHWR_IO_I2C_FREESCALE_I2C_xxxx or
CYGHWR_DEV_I2C_FREESCALE_I2C_xxx 

Device instants are HAL dependent, hence var_io_devs.h or plf_io.h.

Example:
var_io_devs.h -----------------------------

#define CYGHWR_IO_I2C_FREESCALE_I2C1_P \
((cyghwr_io_i2c_freescale_i2c_t*)0x40067000)

--------------------------------------------

plf_io.h -----------------------------------

#define CYGHWR_IO_I2C_FREESCALE_I2C0_PIN_SDA CYGHWR_HAL_KINETIS_PIN(D, 9, 2, 0)

--------------------------------------------

>
> > where <freescale_i2c> is the name of this particular freescale spi instance
> > (typically as referred to in ref. man.).
> 
> The services have only generic names in the Freescale RMs, so we end up with
> very generic names. That is fine if Freescale only make a single i2c controller
> and never will design another one, perhaps that is the case?

It seem that to be the case with i2c. I checked Coldfire+ seem to have the same
i2c but PXN have some older version looks like some bits match but probably not
all. Maybe we can use i2c now and if there's next entry with generic name we
add the new device name some suffix/prefix.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #9 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2011-12-26 07:32:13 GMT ---
Thanks Ilija, I will prepare an updated patch sometime next week.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Bug 1001397 depends on bug 1001388, which changed state.

Bug 1001388 Summary: Convenience macros for manipulating GPIO pins
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001388

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |CURRENTRELEASE

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #11 from Ilija Kocho <ilijak@siva.com.mk> 2012-01-11 20:32:34 GMT ---
Hi Tomas, thank you for the update and here are my first notes:

ISR priority
Cortex-M NVIC implements priority arbitration with most significant /n/ bits
implemented. Consequently in Kinetis (4 bits) they are bits 7..4 so priorities
range 0x00, 0x10,..,0xf0. But this is not end of the story for eCos. Please
visit this thread for full explanation (both references and follow-ups):
http://ecos.sourceware.org/ml/ecos-devel/2010-11/msg00007.html

It would be good to have configurable ISR priority. I understand that some
drivers do not provide such control, but IMO that ISR priority manipulation may
be important for tuning system behavior. And it is just a cdl per bus.

I2C buses
It would be good that unused buses are inactive - not present. Then user (or
higher level I2C driver) activates the necessary bus by implementing respective
interface. 

Also some parameters such as speed should be private to the bus. I can imagine
two devices using separate buses at different speeds simultaneously.

Also we don't need 2 init functions. Instead you can place parameters such as
CYGHWR_IO_I2C_FREESCALE_I2Cx_PIN_SDA, etc. in the bus structure (or some const
if you wish to save some RAM).

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1507|0                           |1
        is obsolete|                            |

--- Comment #12 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-01-16 14:30:33 GMT ---
Created an attachment (id=1519)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1519)
Freescale i2c driver

Updated patch:

 * made bus presence, bus speed and isr priority configurable,
 * only one kinetis_i2c_init() function.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #13 from Ilija Kocho <ilijak@siva.com.mk> 2012-02-07 20:37:36 GMT ---
Hi Tomas

I'm sorry for this delay.

You have placed things such as IRQ priorities in HAL rather than with device
CDL which is not an usual practice. On second consideration I realized that it
is good to have IRQ priorities at HAL because they are being resolved by HAL
(NVIC). Next step was a logical consequence: Why not consolidate all IRQ
priorities and put them togeteher. Anyway it's not priority what counts but
relative priority differences between IRQs.So I introduced the same to all
drivers and it took some time.
You can find update at bug Bug 1001450.

Now little discussion on your driver. Recently I worked on DSPI so this
quiestion is based on that experience and it may or may not have snse here:
    -Can I2C clock be defined on device basis like at SPI?
Next is general, could it be possible to provide user with option to set I2C
speed (clock) rather than prescaler value?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #14 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-02-08 09:12:22 GMT ---
(In reply to comment #13)
>     -Can I2C clock be defined on device basis like at SPI?

Yes, I think it should be possible to change the bus speed when the device
takes over the mastership of the bus, I shall look into that.

> Next is general, could it be possible to provide user with option to set I2C
> speed (clock) rather than prescaler value?

I can't see any way to translate the SCL/SDA values that are need to calculate
the i2c speed into the MUL + ICR values that need to go into the frequency
divider register; the RM just provides a one big LUT for this.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #15 from Ilija Kocho <ilijak@siva.com.mk> 2012-02-08 09:54:55 GMT ---
(In reply to comment #14)
> (In reply to comment #13)
> >     -Can I2C clock be defined on device basis like at SPI?
> 
> Yes, I think it should be possible to change the bus speed when the device
> takes over the mastership of the bus, I shall look into that.
> 
> > Next is general, could it be possible to provide user with option to set I2C
> > speed (clock) rather than prescaler value?
> 
> I can't see any way to translate the SCL/SDA values that are need to calculate
> the i2c speed into the MUL + ICR values that need to go into the frequency
> divider register; the RM just provides a one big LUT for this.

Yeah, it's kind of reverse calculation. I had similar problem with SPI and I
use a brute force algorithm. But some SPI devices (example MMC) want to set
speed on their own so having this calculation is necessary anyway. In I2C world
it may be not the case (frankly, I have no insight) so it could be just "nice"
but not mandatory.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #16 from Ilija Kocho <ilijak@siva.com.mk> 2012-02-11 00:15:45 GMT ---
Some lines (such as some macros in i2c_freescale.h) are too long. General rule
is that lines are below 78 characters. Somewhat longer lines can be accepted
for macros or in CDL but these are too long. Please reformat.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #17 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-02-11 10:14:24 GMT ---
Will do; also just noticed the priority level for the IRQs is incorrectly
defined in the cdl (should be 0, 0x10, 0x20, etc.), so will fix that as well.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|1001388                     |1001450

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1519|0                           |1
        is obsolete|                            |

--- Comment #18 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-02-12 19:10:38 GMT ---
Created an attachment (id=1571)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1571)
Kinetis i2c driver

Updated patch:

 * rebased on top of bug 1001450,
 * integrated with the new ISR priority scheme,
 * fixed ISR priority values in the cdl, comment 17,
 * fixed the long lines in i2c_freescale.h, comment 16,

I have not yet had the time to address the issues raised in comment 13 (per
device configuratble bus speed; setting clock speed rather than prescaler), but
thought it is worth to update the patch here anyway.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #19 from Ilija Kocho <ilijak@siva.com.mk> 2012-02-23 16:11:04 GMT ---
Please check against current CVS and synchronize if necessary.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1571|0                           |1
        is obsolete|                            |

--- Comment #20 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-03-20 15:16:45 GMT ---
Created an attachment (id=1653)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1653)
Kinetis i2c driver

I have updated the patch against the current cvs, as well as implemented a
per-device bus speed configuration.

However, I left the cdls and API to use the MULT+ICR values (as per 50.3.2 and
50.4.1.10 of the RM) rather then actual frequency -- the ICR value is a
function of multiple parameters, and there seems to be no obvious way to
conjure it up from frequency alone.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #21 from Ilija Kocho <ilijak@siva.com.mk> 2012-03-31 18:17:18 BST ---
(In reply to comment #20)
> Created an attachment (id=1653)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1653) [details]
> Kinetis i2c driver
> 
> I have updated the patch against the current cvs, as well as implemented a
> per-device bus speed configuration.
> 
> However, I left the cdls and API to use the MULT+ICR values (as per 50.3.2 and
> 50.4.1.10 of the RM) rather then actual frequency -- the ICR value is a
> function of multiple parameters, and there seems to be no obvious way to
> conjure it up from frequency alone.

There is a conflict that requires at least one active I2C bus:

18:46:30> ecosconfig new twr_k60n512
U CYGHWR_HAL_CORTEXM, new inferred value M4
U CYGHWR_HAL_CORTEXM_SYSTICK_CLK_SOURCE, new inferred value INTERNAL
C CYGPKG_DEVS_I2C_FREESCALE_I2C, "requires" constraint not satisfied:
CYGINT_HAL_FREESCALE_I2C_BUS
18:46:30>

Probably something like following would be best fix, but unfortunately
configtool crashes when I try to activate an I2C bus:

--- i2c_freescale_org.cdl    2012-03-31 18:15:45.364128606 +0200
+++ i2c_freescale.cdl    2012-03-31 18:53:28.388222427 +0200
@@ -46,10 +46,17 @@
 #####DESCRIPTIONEND####
 # ====================================================================

+cdl_interface CYGINT_HAL_FREESCALE_I2C_BUS {
+    display      "Number of I2C buses"
+}
+    
 cdl_package CYGPKG_DEVS_I2C_FREESCALE_I2C {
     display       "I2C driver for Freescale Kinetis and ColdFire+ chips"
     parent        CYGPKG_IO_I2C
     requires      CYGPKG_IO_I2C
+    
+    active_if     CYGINT_HAL_FREESCALE_I2C_BUS
+    
     include_dir   cyg/io
     description   "
         This package provides a generic I2C device driver for the on-chip
@@ -57,12 +64,6 @@

     compile       -library=libextras.a i2c_freescale.c

-    cdl_interface CYGINT_HAL_FREESCALE_I2C_BUS {
-        display      "Number of I2C buses"
-    }
-
-    requires CYGINT_HAL_FREESCALE_I2C_BUS
-
     cdl_option CYGPKG_DEVS_I2C_FREESCALE_I2C_TRACE {
         display       "I2C trace"
         flavor        bool

Then I tried the following:

--- i2c_freescale_org.cdl    2012-03-31 18:15:45.364128606 +0200
+++ i2c_freescale.cdl    2012-03-31 18:56:09.916229122 +0200
@@ -46,10 +46,17 @@
 #####DESCRIPTIONEND####
 # ====================================================================

+cdl_interface CYGINT_HAL_FREESCALE_I2C_BUS {
+    display      "Number of I2C buses"
+}
+    
 cdl_package CYGPKG_DEVS_I2C_FREESCALE_I2C {
     display       "I2C driver for Freescale Kinetis and ColdFire+ chips"
     parent        CYGPKG_IO_I2C
     requires      CYGPKG_IO_I2C
+
+    requires      CYGPKG_HAL_CORTEXM_KINETIS
+
     include_dir   cyg/io
     description   "
         This package provides a generic I2C device driver for the on-chip
@@ -57,12 +64,6 @@

     compile       -library=libextras.a i2c_freescale.c

-    cdl_interface CYGINT_HAL_FREESCALE_I2C_BUS {
-        display      "Number of I2C buses"
-    }
-
-    requires CYGINT_HAL_FREESCALE_I2C_BUS
-
     cdl_option CYGPKG_DEVS_I2C_FREESCALE_I2C_TRACE {
         display       "I2C trace"
         flavor        bool

I hope you'll find better fix, otherwise If this is good enough I could prepare
updated patch.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Bug 1001397 depends on bug 1001450, which changed state.

Bug 1001450 Summary: Kinetis HAL and driver upgrade
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001450

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |CURRENTRELEASE

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #22 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-21 20:46:39 GMT ---
Tomas

I am trying to clean up my backlog. Your I2C driver appears to be almost ready
for check in. I wander if you have tried the patch proposed in comment 21.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #23 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-12-24 09:52:31 GMT ---
(In reply to comment #22)
> I am trying to clean up my backlog. Your I2C driver appears to be almost ready
> for check in. I wander if you have tried the patch proposed in comment 21.

The patch makes sense to me, though I have not had a chance to test it.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1653|0                           |1
        is obsolete|                            |

--- Comment #24 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-24 16:53:06 GMT ---
Created an attachment (id=2004)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=2004)
Freescale I2C driver 121224

Here is the driver with applied patch from comment 21 and synced with CVS as of
2012.12.24.

This patch contains only the I2C driver. HAL and ecos.db are in separated
diffs.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #25 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-24 16:54:24 GMT ---
Created an attachment (id=2005)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=2005)
Freescale I2C Kinetis HAL files

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2005|Freescale I2C Kinetis HAL   |Freescale I2C Kinetis HAL
        description|files                       |121224

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #26 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-24 16:55:52 GMT ---
Created an attachment (id=2006)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=2006)
Freescale I2C driver ecos.db patch 121224

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEEDINFO
     Ever Confirmed|0                           |1

--- Comment #27 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-24 17:14:00 GMT ---
Hi Tomas

Thank you for your reply.

(In reply to comment #23)
> (In reply to comment #22)
> > I am trying to clean up my backlog. Your I2C driver appears to be almost ready
> > for check in. I wander if you have tried the patch proposed in comment 21.
> 
> The patch makes sense to me, though I have not had a chance to test it.

As you may have seen I took the liberty to apply the patch from comment 21. It
compiles fine, but I haven't tested the runtime.

One thing that bothers me is the WAIT_BUS_READY(), it looks like busy waiting,
but I can't estimate implications since I don't have enough insight into
I2C. Can you comment on this please?

Thanks
Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Mike Jones <mjones@linear.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjones@linear.com

--- Comment #28 from Mike Jones <mjones@linear.com> 2012-12-24 17:38:26 GMT ---
I can make a couple of comments based on my experience with K60 and the
Freescale POSIX I2C driver and a real application.

First is the issue of using the interrupt driver vs polling driver. I use the
interrupt driver so that other tasks can operate while waiting. My application
is PMBus, and clock stretching can slow things down, so I figure I don't want
to be in a polling loop.

Second, is the issue of polling on bits. I discovered that the Freescale driver
polls on bits at times, even when using the interrupt driver. This causes some
serious problems in real use. I'll give an example.

When multimastering, it is always possible to have a misbehaving master. I have
one of these, and it basically breaks the rules and asserts the clock or data
line randomly.

Because I can't predict what master might misbehave, I tried to make the
telemetry task immune to this. What I did was create a watchdog. Since most of
my PMBus/I2C transactions are telemetry, it is easy to do. When the misbehaving
master does its deed, I found the I2C driver got stuck in a polling loop on a
bit, even though it was in interrupt driver.

Once in this condition, you are helpless to get out of the loop without
modifying the driver code, because it does not have a counter/timeout on the
loop. The driver assumes the other masters and slaves follow the rules.

So my watchdog kills the task and tries to recover. This means my code has to
clean up the I2C driver state either by reopening the device, or manually. This
proved a bit difficult because in MQX once you are stuck in this loop, you
can't clean up mutexes from a different task. So you have a catch 22. If you
don't kill it, you can't get out of the loop and have it clean up. And if you
kill it, you can't clean up because you don't have permission. 

So I had to create tasks that did not own the resources so I could kill the
task and clean up. Then I restart the task.

This is a pretty nasty way to deal with the problem, but it does in fact work.
In general, a bad master will not be a permanent state of affairs. A bad master
is probably someone putting a USB based dongle on the bus for debug,
programming, etc. A production hardware will probably behave, but even then, in
High Availability systems, one may still want to try to recover.

So potential solutions are to have driver calls that timeout or driver calls
that when stuck, can easily be cleared. If a watchdog can make a call to cause
the driver to break its loop and return an error, one can solve the stuck loop
that was as well. If you use a timeout, it is best to remain compliant with
PMBus/SMBus and set the loops to timeout on a specific time, and best if you
use a hardware timer so the loop does not consume the CPU. From my point of
view this just suggests making a easy way for the watchdog to kill the stuck
loop at clean up so that the time function is under the application's control.
But I would still use an interrupt approach in general for those things where
the I2C peripheral supports it. I don't know why and when people use the
Freescale polled driver. Perhaps there is a case where other uses of interrupts
makes this a problem. So I guess for completeness you could do both.

The Freescale driver code is publicly available for inspection. I am not
familiar with it.

The last thing I will say is that a good I2C driver will open up opportunities
for eCos. eCos would be a good choice for implementing IPMI Board Controllers,
which is my area of interest.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #29 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-25 21:31:55 GMT ---
(In reply to comment #27)

Hi again Tomas

> 
> Thank you for your reply.
> 
> (In reply to comment #23)
> > (In reply to comment #22)
> > > I am trying to clean up my backlog. Your I2C driver appears to be almost ready
> > > for check in. I wander if you have tried the patch proposed in comment 21.
> > 
> > The patch makes sense to me, though I have not had a chance to test it.
> 
> As you may have seen I took the liberty to apply the patch from comment 21. It
> compiles fine, but I haven't tested the runtime.
> 
> One thing that bothers me is the WAIT_BUS_READY(), it looks like busy waiting,
> but I can't estimate implications since I don't have enough insight into
> I2C. Can you comment on this please?

I looked through other I2C drivers and I found out that some of them have
similar macro. So we have a precedent, but I would look for better solution.
For instance what would happen if we replace the macro with something like this
(untested):

bool is_bus_free(freescale_i2c_extra *extra )
{
    bool bus_free;
    if (!extra->i2c_owner) {
        freescale_i2c_t *i2c_s = FREESCALE_I2C_P(extra->i2c_base);
        if(i2c_s->s & FREESCALE_I2C_S_BUSY_M){
            bus_free = false;
        } else {
            extra->i2c_got_nack = 0;
            extra->i2c_owner = 1;
            bus_free = true;
        }
    } else {
        bus_free = true;
    }
    return bus_free;
}

Then something like this:

static cyg_bool
freescale_i2c_handle_xfer(const cyg_i2c_device * dev, int address)
{
  freescale_i2c_device *fdev = (freescale_i2c_device*) dev;
  freescale_i2c_extra  *extra = (freescale_i2c_extra *)
dev->i2c_bus->i2c_extra;
  cyg_uint8             data = *extra->i2c_data.i2c_tx_data;
  freescale_i2c_t      *i2c_s = FREESCALE_I2C_P (extra->i2c_base);

  // Nothing to do
  if (extra->i2c_count == 0)
    return 0;

-  // Take the bus ownership
-  WAIT_BUS_READY (extra);
+  // Check the bus ownership
+  if(!is_bus_free(extra))
+    return 0;

  .....

}

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #30 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-25 21:33:38 GMT ---
Hi Mike, thank you for your considerations.

(In reply to comment #28)
> I can make a couple of comments based on my experience with K60 and the
> Freescale POSIX I2C driver and a real application.
> 
> First is the issue of using the interrupt driver vs polling driver. I use the
> interrupt driver so that other tasks can operate while waiting. My application
> is PMBus, and clock stretching can slow things down, so I figure I don't want
> to be in a polling loop.

Polling is not an issue with eCos I2C driver. It is interrupt driven indeed.

My question is about the macro WAIT_BUS_READY() and my concern is for
multi-master system since, in a single master system it is returns immediately.

Perhaps you could give it a try in your environment. Any feedback is
appreciated.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #31 from Mike Jones <mjones@linear.com> 2012-12-25 23:37:55 GMT ---
I think that looping on bus ready is probably ok if there is a SMBus like
timeout of 25ms to 30ms, and as long as the scheduler can break in switch
tasks. If the wait can't be interrupted by the scheduler it is more problematic
when the bus is stuck. 

A worst case scenario might have a task for ALERTB and one for telemetry. The
second master hogs the bus and the first master is having trouble getting in.
The the ALERTB comes along and needs immediate service. The ALERTB task may
flip a GPIO bit and kill power, or it may wait for the timeout and then attempt
to use the bus.

All that matters to me is that it the driver waits for the bus to be free,
times out, and allows a higher priority task do its job while waiting. I would
not want it to just return because the bus is busy and force me to write my own
timeout. And I would not want it to hang forever if a second master was beating
the bus to death.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #32 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-26 15:05:59 GMT ---
(In reply to comment #31)
> I think that looping on bus ready is probably ok if there is a SMBus like
> timeout of 25ms to 30ms, and as long as the scheduler can break in switch
> tasks. If the wait can't be interrupted by the scheduler it is more problematic
> when the bus is stuck. 

I2C thread normally shall be pre-empted by threads with higher priority than
itself. With current code this implies that I2C thread has to run with
relatively low priority. Probably it would be better if the priority is
(dynamically) decreased during spinning.

> 
> A worst case scenario might have a task for ALERTB and one for telemetry. The
> second master hogs the bus and the first master is having trouble getting in.
> The the ALERTB comes along and needs immediate service. The ALERTB task may
> flip a GPIO bit and kill power, or it may wait for the timeout and then attempt
> to use the bus.
> 
> All that matters to me is that it the driver waits for the bus to be free,
> times out, and allows a higher priority task do its job while waiting. I would
> not want it to just return because the bus is busy and force me to write my own
> timeout. And I would not want it to hang forever if a second master was beating
> the bus to death.

Timeout may be useful in general and it can be added as well.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #33 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-29 14:23:42 GMT ---
(In reply to comment #23)
> (In reply to comment #22)
> > I am trying to clean up my backlog. Your I2C driver appears to be almost ready
> > for check in. I wander if you have tried the patch proposed in comment 21.
> 
> The patch makes sense to me, though I have not had a chance to test it.

Hi Tomas

If you are too busy, and if you don't have objections, I would like to take
care of the spin optimization (ref. comment 32), of course with all respect and
credit to your contribution.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #34 from Mike Jones <mjones@linear.com> 2013-01-02 03:48:27 GMT ---
Can some one provide some simple code examples? I have the patches installed, a
project ready for code, and hardware with I2C devices known to work using my
hardware and Freescale code tools. I'll read the reference manual for the
target independent API, but I am not sure that applies for the current patch or
if I have to use the i2c_freescale.h stuff, or a combination, etc. And the
i2c_freescale.h API does not describe the parameters, so if I need to use that,
it would help to have names of the parameters and some explanation, especially
for bools. I know I can reverse engineer the code, but I am hoping I don't have
to :-)

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #35 from Mike Jones <mjones@linear.com> 2013-01-02 07:07:14 GMT ---
I read the reference on I2C. I am pretty clear how the standard I2C API works
in terms of making transactions using locking, etc.

I am making a simple app with no tasks, just to run in main. I assume there is
some magic for making a cyg_i2c_bus and then you can make some cyg_i2c_device.
Then call freescale_i2c_init(...) I see a macro for creating the bus, but it
has to pass some function pointers. I assume there is some code for that, or I
have to know all the magic things to put in the macro.

Once there is a device, it is not clear if the standard API can use it. And if
it can, I am not certain if it would behave any different that the Freescale
specific API commands.

So I think I mainly need to know how to get things glued together and
relationship between the generic vs Freescale API.

I don't need to know how to use the API to send and receive data.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #36 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2013-01-03 09:09:27 GMT ---
(In reply to comment #33)
> If you are too busy, and if you don't have objections, I would like to take
> care of the spin optimization (ref. comment 32), of course with all respect and
> credit to your contribution.

Please do with it whatever you deem needs doing :)

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #37 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2013-01-03 09:12:05 GMT ---
(In reply to comment #34)
> Can some one provide some simple code examples? 

The only example I have is the accelerometer driver, which is in the patches
attached to bug 1001419.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #38 from Ilija Kocho <ilijak@siva.com.mk> 2013-01-03 16:27:42 GMT ---
Created an attachment (id=2017)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=2017)
Freescale I2C driver 130103, incremental to attachments 2004/2005

(In reply to comment #36)
> (In reply to comment #33)
> > If you are too busy, and if you don't have objections, I would like to take
> > care of the spin optimization (ref. comment 32), of course with all respect and
> > credit to your contribution.
> 
> Please do with it whatever you deem needs doing :)

Thank you. Here I propose a code that:
   - decreases i2c thread priority prior to wait spin;
   - returns after timeout (as Mike suggests)

The patch is incremental. Apply it on top of attachment 2004 and attachment
2005.

Ilija


Note: I haven't been able to test it with multiple i2c masters.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #39 from Mike Jones <mjones@linear.com> 2013-01-06 05:46:55 GMT ---
I am having trouble slowing down the bus speed.

This is my code for write byte, but it is clocking at over 1Mhz, and my devices
only go up to 400Khz.

Is there something else I need to do?

void smbus_write_byte(uint8_t address, uint8_t command, uint8_t data)
{
    cyg_uint8 buffer[2];
    cyg_i2c_device device = {                        \
            .i2c_bus        = &hal_kinetis_i2c0_bus, \
            .i2c_address    = address,               \
            .i2c_flags      = 0,                     \
            .i2c_delay      = 10000                  \
        };

    buffer[0] = command;
    buffer[1] = data;

    cyg_i2c_transaction_begin(&device);
    if(!cyg_i2c_transaction_tx(&device, true, &buffer[0], 2, false)) {
        diag_printf("Read: fail TX.\n");
    }

    cyg_i2c_transaction_stop(&device);
    cyg_i2c_transaction_end(&device);
}

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #40 from Mike Jones <mjones@linear.com> ---
Created attachment 2112
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2112&action=edit
Control clock rate, fix race condition, bug fix

This patch contains new code for managing clock speed that was added by Ilija.
He can provide a ChangeLog for that if desired. I tested/modified his code and
fixed bugs. There was a race condition that caused the driver to lockup if bits
were not set/cleared at the right time, and a few other small things. I don't
remember the exact changes any more without a fresh diff against the original
patch, but I remember the 3 days on the bench with a scope hunting it down :-)
The original author can diff against this version if desired. The actual bug
fixes are just a few lines of critical code.

I have run this on three PMBus devices extensively including a several day 24
hour telemetry run, meaning constant traffic reading and writing. There were no
errors.

If anyone wants to do more testing, I suggest testing something faster than
PMBus which maxes out at 400Khz.

In the future this could be improved to add API to manage SMBus features such
as the timeout mechanism. I don't have time at the moment to do that.

I suggest the original author (or others( apply the patch and test their
existing application. If nobody has any problems, I feel this code is safe to
commit to CVS.

Some example code:


const cyg_uint32 i2c_bus_time = 10000;

    cyg_i2c_device device = {                        \
            .i2c_bus        = &cyg_i2c0_bus,         \
            .i2c_address    = address,               \
            .i2c_flags      = 0,                     \
            .i2c_delay      = i2c_bus_time           \
        };
    cyg_uint8 buffer[1];
    cyg_uint8 input[1];
    buffer[0] = command;

    cyg_i2c_transaction_begin(&device);
    if(!cyg_i2c_transaction_tx(&device, true, &buffer[0], 1, false)) {
        diag_printf("Read Byte: fail TX.\n");
    } else if(!cyg_i2c_transaction_rx(&device, true, &input[0], 1, true, true))
{
        diag_printf("Read Byte: fail RX.\n");
    }
    cyg_i2c_transaction_end(&device);

    return input[0];

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #41 from Mike Jones <mjones@linear.com> ---
I failed state that my patch called Control clock rate, fix race condition, bug
fix is a patch against CVS, not a patch against a patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned@bugs.ecos.source |ilijak@siva.com.mk
                   |ware.org                    |

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 1001397] New: I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

           Summary: I2C driver for Kinetic microcontrollers
           Product: eCos
           Version: CVS
          Platform: All
        OS/Version: Other
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: low
         Component: I2C
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: tf+bugs.ecos@r-finger.com
                CC: ecos-bugs@ecos.sourceware.org, ilijak@siva.com.mk
             Class: Advice Request


Created an attachment (id=1443)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1443)
Kinetis i2c driver

The attached patch provides a basic (7 bit addressing only for now) i2c driver
for Freescale Kinetis K40 and K60 microcontrollers. The driver is formally
based on the lm3s driver implementation and is irq driven, and been tested on
the TWR-K60N512 board. (The patch applies over the GPIO patch attached to bug
1001388.)

Any feedback would be welcome.

PS. I am in process of sorting out copyright assignment with the FSF.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #1 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-01 15:24:01 GMT ---
Tomas,

Thank you for contribution. I will try to get some time to test your code as
soon as I can, but it may take some time. Anyway we are waiting for copyright
assignment.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ecos-patches@ecos.sourcewar
                   |                            |e.org
          Component|I2C                         |Patches and contributions
         OS/Version|Other                       |Cortex-M

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |1001388

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #2 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2011-12-01 15:58:38 GMT ---
There is no rush, I just wanted to get this out, in case there are any formal
changes required. I am working on an a driver for the MMA7660 accelerometer
(included on the TWR boards), which provides one way of testing i2c in both
directions; it's not ready yet, but not far off. I'll post that into a separate
bug eventually.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |1001419

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #3 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2011-12-23 10:40:41 GMT ---
In the light of the discussion in bug 1001428, I think the hal bits in this
patch should be moved under hal/misc/freescale/i2c?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

John Dallaway <john@dallaway.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |john@dallaway.org.uk

--- Comment #4 from John Dallaway <john@dallaway.org.uk> 2011-12-23 13:27:33 GMT ---
(In reply to comment #3)
> In the light of the discussion in bug 1001428, I think the hal bits in this
> patch should be moved under hal/misc/freescale/i2c?

Tomas, will this package be applicable to _all_ Freescale I2C peripheral blocks
across all Freescale product families? If not, please make the "i2c" directory
name (and corresponding eCos package name) less generic. In an ideal world, all
peripheral blocks which are shared across multiple architectures would have
unique names assigned by the manufacturer.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #5 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-23 14:12:07 GMT ---
(In reply to comment #3)
> In the light of the discussion in bug 1001428, I think the hal bits in this
> patch should be moved under hal/misc/freescale/i2c?

(In reply to comment #3)
> In the light of the discussion in bug 1001428, I think the hal bits in this
> patch should be moved under hal/misc/freescale/i2c?


I assume you are referring to i2c header. They should stay with driver source,
either in driver .c file or (preferably) in a header.
As there is existing devs/i2c directory, I propose following path:
    - devs/i2c/freescale/<freescale_i2c>

where <freescale_i2c> is the name of this particular freescale spi instance
(typically as referred to in ref. man.).

Besides the device structure itself, there is some information provided by HAL
(base address(es), pin configuration or so. They can be be considered variant
or platform dependent and so defined in var_io_devs.h and/or plf_io.h
respectively. If defined in both then plf shall override var.

I hope in future to produce some docs but in meantime pse refer to ENET driver
for example.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #6 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-23 14:15:09 GMT ---
(In reply to comment #5)
> (In reply to comment #3)
> > In the light of the discussion in bug 1001428, I think the hal bits in this
> > patch should be moved under hal/misc/freescale/i2c?
> 
> (In reply to comment #3)
> > In the light of the discussion in bug 1001428, I think the hal bits in this
> > patch should be moved under hal/misc/freescale/i2c?
> 
> 
> I assume you are referring to i2c header. They should stay with driver source,
> either in driver .c file or (preferably) in a header.
> As there is existing devs/i2c directory, I propose following path:
>     - devs/i2c/freescale/<freescale_i2c>
> 
(Just to clarify this is path to device that will have usual subtree
             <version>/cdl
                      /include
                      /src
                      /ChangeLog

> where <freescale_i2c> is the name of this particular freescale spi instance
> (typically as referred to in ref. man.).
> 
> Besides the device structure itself, there is some information provided by HAL
> (base address(es), pin configuration or so. They can be be considered variant
> or platform dependent and so defined in var_io_devs.h and/or plf_io.h
> respectively. If defined in both then plf shall override var.
> 
> I hope in future to produce some docs but in meantime pse refer to ENET driver
> for example.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #7 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2011-12-24 09:21:52 GMT ---
(In reply to comment #5)
> As there is existing devs/i2c directory, I propose following path:
>     - devs/i2c/freescale/<freescale_i2c>

OK, but the situation is exactly the same for the SLCD controller, and so we
end up with bits under devs/ and bits under hal/misc, with no clear logic for
the distinction I can see.


> where <freescale_i2c> is the name of this particular freescale spi instance
> (typically as referred to in ref. man.).

The services have only generic names in the Freescale RMs, so we end up with
very generic names. That is fine if Freescale only make a single i2c controller
and never will design another one, perhaps that is the case?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #8 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-24 11:58:57 GMT ---
(In reply to comment #7)
> (In reply to comment #5)
> > As there is existing devs/i2c directory, I propose following path:
> >     - devs/i2c/freescale/<freescale_i2c>
> 
> OK, but the situation is exactly the same for the SLCD controller, and so we
> end up with bits under devs/ and bits under hal/misc, with no clear logic for
> the distinction I can see.
> 

We need to distinguish/combine two sources of information:
   Device structure (registers, bits)
   Device instantiation (base address, pins, etc).

Device structure is needed only by the driver and should live there (in
driver's .c file or in a header file). Also the naming should be in device name
space, common patterns are: CYGHWR_IO_I2C_FREESCALE_I2C_xxxx or
CYGHWR_DEV_I2C_FREESCALE_I2C_xxx 

Device instants are HAL dependent, hence var_io_devs.h or plf_io.h.

Example:
var_io_devs.h -----------------------------

#define CYGHWR_IO_I2C_FREESCALE_I2C1_P \
((cyghwr_io_i2c_freescale_i2c_t*)0x40067000)

--------------------------------------------

plf_io.h -----------------------------------

#define CYGHWR_IO_I2C_FREESCALE_I2C0_PIN_SDA CYGHWR_HAL_KINETIS_PIN(D, 9, 2, 0)

--------------------------------------------

>
> > where <freescale_i2c> is the name of this particular freescale spi instance
> > (typically as referred to in ref. man.).
> 
> The services have only generic names in the Freescale RMs, so we end up with
> very generic names. That is fine if Freescale only make a single i2c controller
> and never will design another one, perhaps that is the case?

It seem that to be the case with i2c. I checked Coldfire+ seem to have the same
i2c but PXN have some older version looks like some bits match but probably not
all. Maybe we can use i2c now and if there's next entry with generic name we
add the new device name some suffix/prefix.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #9 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2011-12-26 07:32:13 GMT ---
Thanks Ilija, I will prepare an updated patch sometime next week.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Bug 1001397 depends on bug 1001388, which changed state.

Bug 1001388 Summary: Convenience macros for manipulating GPIO pins
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001388

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |CURRENTRELEASE

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1443|0                           |1
        is obsolete|                            |

--- Comment #10 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-01-10 15:40:21 GMT ---
Created an attachment (id=1507)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1507)
Freescale i2c driver

Moved the device driver code under devs/i2c/freescale/i2c and cleaned up the
driver implementation.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #11 from Ilija Kocho <ilijak@siva.com.mk> 2012-01-11 20:32:34 GMT ---
Hi Tomas, thank you for the update and here are my first notes:

ISR priority
Cortex-M NVIC implements priority arbitration with most significant /n/ bits
implemented. Consequently in Kinetis (4 bits) they are bits 7..4 so priorities
range 0x00, 0x10,..,0xf0. But this is not end of the story for eCos. Please
visit this thread for full explanation (both references and follow-ups):
http://ecos.sourceware.org/ml/ecos-devel/2010-11/msg00007.html

It would be good to have configurable ISR priority. I understand that some
drivers do not provide such control, but IMO that ISR priority manipulation may
be important for tuning system behavior. And it is just a cdl per bus.

I2C buses
It would be good that unused buses are inactive - not present. Then user (or
higher level I2C driver) activates the necessary bus by implementing respective
interface. 

Also some parameters such as speed should be private to the bus. I can imagine
two devices using separate buses at different speeds simultaneously.

Also we don't need 2 init functions. Instead you can place parameters such as
CYGHWR_IO_I2C_FREESCALE_I2Cx_PIN_SDA, etc. in the bus structure (or some const
if you wish to save some RAM).

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1507|0                           |1
        is obsolete|                            |

--- Comment #12 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-01-16 14:30:33 GMT ---
Created an attachment (id=1519)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1519)
Freescale i2c driver

Updated patch:

 * made bus presence, bus speed and isr priority configurable,
 * only one kinetis_i2c_init() function.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #13 from Ilija Kocho <ilijak@siva.com.mk> 2012-02-07 20:37:36 GMT ---
Hi Tomas

I'm sorry for this delay.

You have placed things such as IRQ priorities in HAL rather than with device
CDL which is not an usual practice. On second consideration I realized that it
is good to have IRQ priorities at HAL because they are being resolved by HAL
(NVIC). Next step was a logical consequence: Why not consolidate all IRQ
priorities and put them togeteher. Anyway it's not priority what counts but
relative priority differences between IRQs.So I introduced the same to all
drivers and it took some time.
You can find update at bug Bug 1001450.

Now little discussion on your driver. Recently I worked on DSPI so this
quiestion is based on that experience and it may or may not have snse here:
    -Can I2C clock be defined on device basis like at SPI?
Next is general, could it be possible to provide user with option to set I2C
speed (clock) rather than prescaler value?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #14 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-02-08 09:12:22 GMT ---
(In reply to comment #13)
>     -Can I2C clock be defined on device basis like at SPI?

Yes, I think it should be possible to change the bus speed when the device
takes over the mastership of the bus, I shall look into that.

> Next is general, could it be possible to provide user with option to set I2C
> speed (clock) rather than prescaler value?

I can't see any way to translate the SCL/SDA values that are need to calculate
the i2c speed into the MUL + ICR values that need to go into the frequency
divider register; the RM just provides a one big LUT for this.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #15 from Ilija Kocho <ilijak@siva.com.mk> 2012-02-08 09:54:55 GMT ---
(In reply to comment #14)
> (In reply to comment #13)
> >     -Can I2C clock be defined on device basis like at SPI?
> 
> Yes, I think it should be possible to change the bus speed when the device
> takes over the mastership of the bus, I shall look into that.
> 
> > Next is general, could it be possible to provide user with option to set I2C
> > speed (clock) rather than prescaler value?
> 
> I can't see any way to translate the SCL/SDA values that are need to calculate
> the i2c speed into the MUL + ICR values that need to go into the frequency
> divider register; the RM just provides a one big LUT for this.

Yeah, it's kind of reverse calculation. I had similar problem with SPI and I
use a brute force algorithm. But some SPI devices (example MMC) want to set
speed on their own so having this calculation is necessary anyway. In I2C world
it may be not the case (frankly, I have no insight) so it could be just "nice"
but not mandatory.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #16 from Ilija Kocho <ilijak@siva.com.mk> 2012-02-11 00:15:45 GMT ---
Some lines (such as some macros in i2c_freescale.h) are too long. General rule
is that lines are below 78 characters. Somewhat longer lines can be accepted
for macros or in CDL but these are too long. Please reformat.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #17 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-02-11 10:14:24 GMT ---
Will do; also just noticed the priority level for the IRQs is incorrectly
defined in the cdl (should be 0, 0x10, 0x20, etc.), so will fix that as well.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1519|0                           |1
        is obsolete|                            |

--- Comment #18 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-02-12 19:10:38 GMT ---
Created an attachment (id=1571)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1571)
Kinetis i2c driver

Updated patch:

 * rebased on top of bug 1001450,
 * integrated with the new ISR priority scheme,
 * fixed ISR priority values in the cdl, comment 17,
 * fixed the long lines in i2c_freescale.h, comment 16,

I have not yet had the time to address the issues raised in comment 13 (per
device configuratble bus speed; setting clock speed rather than prescaler), but
thought it is worth to update the patch here anyway.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|1001388                     |1001450

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #19 from Ilija Kocho <ilijak@siva.com.mk> 2012-02-23 16:11:04 GMT ---
Please check against current CVS and synchronize if necessary.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Tomas Frydrych <tf+bugs.ecos@r-finger.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1571|0                           |1
        is obsolete|                            |

--- Comment #20 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-03-20 15:16:45 GMT ---
Created an attachment (id=1653)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1653)
Kinetis i2c driver

I have updated the patch against the current cvs, as well as implemented a
per-device bus speed configuration.

However, I left the cdls and API to use the MULT+ICR values (as per 50.3.2 and
50.4.1.10 of the RM) rather then actual frequency -- the ICR value is a
function of multiple parameters, and there seems to be no obvious way to
conjure it up from frequency alone.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #21 from Ilija Kocho <ilijak@siva.com.mk> 2012-03-31 18:17:18 BST ---
(In reply to comment #20)
> Created an attachment (id=1653)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1653) [details]
> Kinetis i2c driver
> 
> I have updated the patch against the current cvs, as well as implemented a
> per-device bus speed configuration.
> 
> However, I left the cdls and API to use the MULT+ICR values (as per 50.3.2 and
> 50.4.1.10 of the RM) rather then actual frequency -- the ICR value is a
> function of multiple parameters, and there seems to be no obvious way to
> conjure it up from frequency alone.

There is a conflict that requires at least one active I2C bus:

18:46:30> ecosconfig new twr_k60n512
U CYGHWR_HAL_CORTEXM, new inferred value M4
U CYGHWR_HAL_CORTEXM_SYSTICK_CLK_SOURCE, new inferred value INTERNAL
C CYGPKG_DEVS_I2C_FREESCALE_I2C, "requires" constraint not satisfied:
CYGINT_HAL_FREESCALE_I2C_BUS
18:46:30>

Probably something like following would be best fix, but unfortunately
configtool crashes when I try to activate an I2C bus:

--- i2c_freescale_org.cdl    2012-03-31 18:15:45.364128606 +0200
+++ i2c_freescale.cdl    2012-03-31 18:53:28.388222427 +0200
@@ -46,10 +46,17 @@
 #####DESCRIPTIONEND####
 # ====================================================================

+cdl_interface CYGINT_HAL_FREESCALE_I2C_BUS {
+    display      "Number of I2C buses"
+}
+    
 cdl_package CYGPKG_DEVS_I2C_FREESCALE_I2C {
     display       "I2C driver for Freescale Kinetis and ColdFire+ chips"
     parent        CYGPKG_IO_I2C
     requires      CYGPKG_IO_I2C
+    
+    active_if     CYGINT_HAL_FREESCALE_I2C_BUS
+    
     include_dir   cyg/io
     description   "
         This package provides a generic I2C device driver for the on-chip
@@ -57,12 +64,6 @@

     compile       -library=libextras.a i2c_freescale.c

-    cdl_interface CYGINT_HAL_FREESCALE_I2C_BUS {
-        display      "Number of I2C buses"
-    }
-
-    requires CYGINT_HAL_FREESCALE_I2C_BUS
-
     cdl_option CYGPKG_DEVS_I2C_FREESCALE_I2C_TRACE {
         display       "I2C trace"
         flavor        bool

Then I tried the following:

--- i2c_freescale_org.cdl    2012-03-31 18:15:45.364128606 +0200
+++ i2c_freescale.cdl    2012-03-31 18:56:09.916229122 +0200
@@ -46,10 +46,17 @@
 #####DESCRIPTIONEND####
 # ====================================================================

+cdl_interface CYGINT_HAL_FREESCALE_I2C_BUS {
+    display      "Number of I2C buses"
+}
+    
 cdl_package CYGPKG_DEVS_I2C_FREESCALE_I2C {
     display       "I2C driver for Freescale Kinetis and ColdFire+ chips"
     parent        CYGPKG_IO_I2C
     requires      CYGPKG_IO_I2C
+
+    requires      CYGPKG_HAL_CORTEXM_KINETIS
+
     include_dir   cyg/io
     description   "
         This package provides a generic I2C device driver for the on-chip
@@ -57,12 +64,6 @@

     compile       -library=libextras.a i2c_freescale.c

-    cdl_interface CYGINT_HAL_FREESCALE_I2C_BUS {
-        display      "Number of I2C buses"
-    }
-
-    requires CYGINT_HAL_FREESCALE_I2C_BUS
-
     cdl_option CYGPKG_DEVS_I2C_FREESCALE_I2C_TRACE {
         display       "I2C trace"
         flavor        bool

I hope you'll find better fix, otherwise If this is good enough I could prepare
updated patch.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Bug 1001397 depends on bug 1001450, which changed state.

Bug 1001450 Summary: Kinetis HAL and driver upgrade
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001450

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |CURRENTRELEASE

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #22 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-21 20:46:39 GMT ---
Tomas

I am trying to clean up my backlog. Your I2C driver appears to be almost ready
for check in. I wander if you have tried the patch proposed in comment 21.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #23 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2012-12-24 09:52:31 GMT ---
(In reply to comment #22)
> I am trying to clean up my backlog. Your I2C driver appears to be almost ready
> for check in. I wander if you have tried the patch proposed in comment 21.

The patch makes sense to me, though I have not had a chance to test it.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1653|0                           |1
        is obsolete|                            |

--- Comment #24 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-24 16:53:06 GMT ---
Created an attachment (id=2004)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=2004)
Freescale I2C driver 121224

Here is the driver with applied patch from comment 21 and synced with CVS as of
2012.12.24.

This patch contains only the I2C driver. HAL and ecos.db are in separated
diffs.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #25 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-24 16:54:24 GMT ---
Created an attachment (id=2005)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=2005)
Freescale I2C Kinetis HAL files

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2005|Freescale I2C Kinetis HAL   |Freescale I2C Kinetis HAL
        description|files                       |121224

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #26 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-24 16:55:52 GMT ---
Created an attachment (id=2006)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=2006)
Freescale I2C driver ecos.db patch 121224

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEEDINFO
     Ever Confirmed|0                           |1

--- Comment #27 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-24 17:14:00 GMT ---
Hi Tomas

Thank you for your reply.

(In reply to comment #23)
> (In reply to comment #22)
> > I am trying to clean up my backlog. Your I2C driver appears to be almost ready
> > for check in. I wander if you have tried the patch proposed in comment 21.
> 
> The patch makes sense to me, though I have not had a chance to test it.

As you may have seen I took the liberty to apply the patch from comment 21. It
compiles fine, but I haven't tested the runtime.

One thing that bothers me is the WAIT_BUS_READY(), it looks like busy waiting,
but I can't estimate implications since I don't have enough insight into
I2C. Can you comment on this please?

Thanks
Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Mike Jones <mjones@linear.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjones@linear.com

--- Comment #28 from Mike Jones <mjones@linear.com> 2012-12-24 17:38:26 GMT ---
I can make a couple of comments based on my experience with K60 and the
Freescale POSIX I2C driver and a real application.

First is the issue of using the interrupt driver vs polling driver. I use the
interrupt driver so that other tasks can operate while waiting. My application
is PMBus, and clock stretching can slow things down, so I figure I don't want
to be in a polling loop.

Second, is the issue of polling on bits. I discovered that the Freescale driver
polls on bits at times, even when using the interrupt driver. This causes some
serious problems in real use. I'll give an example.

When multimastering, it is always possible to have a misbehaving master. I have
one of these, and it basically breaks the rules and asserts the clock or data
line randomly.

Because I can't predict what master might misbehave, I tried to make the
telemetry task immune to this. What I did was create a watchdog. Since most of
my PMBus/I2C transactions are telemetry, it is easy to do. When the misbehaving
master does its deed, I found the I2C driver got stuck in a polling loop on a
bit, even though it was in interrupt driver.

Once in this condition, you are helpless to get out of the loop without
modifying the driver code, because it does not have a counter/timeout on the
loop. The driver assumes the other masters and slaves follow the rules.

So my watchdog kills the task and tries to recover. This means my code has to
clean up the I2C driver state either by reopening the device, or manually. This
proved a bit difficult because in MQX once you are stuck in this loop, you
can't clean up mutexes from a different task. So you have a catch 22. If you
don't kill it, you can't get out of the loop and have it clean up. And if you
kill it, you can't clean up because you don't have permission. 

So I had to create tasks that did not own the resources so I could kill the
task and clean up. Then I restart the task.

This is a pretty nasty way to deal with the problem, but it does in fact work.
In general, a bad master will not be a permanent state of affairs. A bad master
is probably someone putting a USB based dongle on the bus for debug,
programming, etc. A production hardware will probably behave, but even then, in
High Availability systems, one may still want to try to recover.

So potential solutions are to have driver calls that timeout or driver calls
that when stuck, can easily be cleared. If a watchdog can make a call to cause
the driver to break its loop and return an error, one can solve the stuck loop
that was as well. If you use a timeout, it is best to remain compliant with
PMBus/SMBus and set the loops to timeout on a specific time, and best if you
use a hardware timer so the loop does not consume the CPU. From my point of
view this just suggests making a easy way for the watchdog to kill the stuck
loop at clean up so that the time function is under the application's control.
But I would still use an interrupt approach in general for those things where
the I2C peripheral supports it. I don't know why and when people use the
Freescale polled driver. Perhaps there is a case where other uses of interrupts
makes this a problem. So I guess for completeness you could do both.

The Freescale driver code is publicly available for inspection. I am not
familiar with it.

The last thing I will say is that a good I2C driver will open up opportunities
for eCos. eCos would be a good choice for implementing IPMI Board Controllers,
which is my area of interest.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #29 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-25 21:31:55 GMT ---
(In reply to comment #27)

Hi again Tomas

> 
> Thank you for your reply.
> 
> (In reply to comment #23)
> > (In reply to comment #22)
> > > I am trying to clean up my backlog. Your I2C driver appears to be almost ready
> > > for check in. I wander if you have tried the patch proposed in comment 21.
> > 
> > The patch makes sense to me, though I have not had a chance to test it.
> 
> As you may have seen I took the liberty to apply the patch from comment 21. It
> compiles fine, but I haven't tested the runtime.
> 
> One thing that bothers me is the WAIT_BUS_READY(), it looks like busy waiting,
> but I can't estimate implications since I don't have enough insight into
> I2C. Can you comment on this please?

I looked through other I2C drivers and I found out that some of them have
similar macro. So we have a precedent, but I would look for better solution.
For instance what would happen if we replace the macro with something like this
(untested):

bool is_bus_free(freescale_i2c_extra *extra )
{
    bool bus_free;
    if (!extra->i2c_owner) {
        freescale_i2c_t *i2c_s = FREESCALE_I2C_P(extra->i2c_base);
        if(i2c_s->s & FREESCALE_I2C_S_BUSY_M){
            bus_free = false;
        } else {
            extra->i2c_got_nack = 0;
            extra->i2c_owner = 1;
            bus_free = true;
        }
    } else {
        bus_free = true;
    }
    return bus_free;
}

Then something like this:

static cyg_bool
freescale_i2c_handle_xfer(const cyg_i2c_device * dev, int address)
{
  freescale_i2c_device *fdev = (freescale_i2c_device*) dev;
  freescale_i2c_extra  *extra = (freescale_i2c_extra *)
dev->i2c_bus->i2c_extra;
  cyg_uint8             data = *extra->i2c_data.i2c_tx_data;
  freescale_i2c_t      *i2c_s = FREESCALE_I2C_P (extra->i2c_base);

  // Nothing to do
  if (extra->i2c_count == 0)
    return 0;

-  // Take the bus ownership
-  WAIT_BUS_READY (extra);
+  // Check the bus ownership
+  if(!is_bus_free(extra))
+    return 0;

  .....

}

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #30 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-25 21:33:38 GMT ---
Hi Mike, thank you for your considerations.

(In reply to comment #28)
> I can make a couple of comments based on my experience with K60 and the
> Freescale POSIX I2C driver and a real application.
> 
> First is the issue of using the interrupt driver vs polling driver. I use the
> interrupt driver so that other tasks can operate while waiting. My application
> is PMBus, and clock stretching can slow things down, so I figure I don't want
> to be in a polling loop.

Polling is not an issue with eCos I2C driver. It is interrupt driven indeed.

My question is about the macro WAIT_BUS_READY() and my concern is for
multi-master system since, in a single master system it is returns immediately.

Perhaps you could give it a try in your environment. Any feedback is
appreciated.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #31 from Mike Jones <mjones@linear.com> 2012-12-25 23:37:55 GMT ---
I think that looping on bus ready is probably ok if there is a SMBus like
timeout of 25ms to 30ms, and as long as the scheduler can break in switch
tasks. If the wait can't be interrupted by the scheduler it is more problematic
when the bus is stuck. 

A worst case scenario might have a task for ALERTB and one for telemetry. The
second master hogs the bus and the first master is having trouble getting in.
The the ALERTB comes along and needs immediate service. The ALERTB task may
flip a GPIO bit and kill power, or it may wait for the timeout and then attempt
to use the bus.

All that matters to me is that it the driver waits for the bus to be free,
times out, and allows a higher priority task do its job while waiting. I would
not want it to just return because the bus is busy and force me to write my own
timeout. And I would not want it to hang forever if a second master was beating
the bus to death.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #32 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-26 15:05:59 GMT ---
(In reply to comment #31)
> I think that looping on bus ready is probably ok if there is a SMBus like
> timeout of 25ms to 30ms, and as long as the scheduler can break in switch
> tasks. If the wait can't be interrupted by the scheduler it is more problematic
> when the bus is stuck. 

I2C thread normally shall be pre-empted by threads with higher priority than
itself. With current code this implies that I2C thread has to run with
relatively low priority. Probably it would be better if the priority is
(dynamically) decreased during spinning.

> 
> A worst case scenario might have a task for ALERTB and one for telemetry. The
> second master hogs the bus and the first master is having trouble getting in.
> The the ALERTB comes along and needs immediate service. The ALERTB task may
> flip a GPIO bit and kill power, or it may wait for the timeout and then attempt
> to use the bus.
> 
> All that matters to me is that it the driver waits for the bus to be free,
> times out, and allows a higher priority task do its job while waiting. I would
> not want it to just return because the bus is busy and force me to write my own
> timeout. And I would not want it to hang forever if a second master was beating
> the bus to death.

Timeout may be useful in general and it can be added as well.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #33 from Ilija Kocho <ilijak@siva.com.mk> 2012-12-29 14:23:42 GMT ---
(In reply to comment #23)
> (In reply to comment #22)
> > I am trying to clean up my backlog. Your I2C driver appears to be almost ready
> > for check in. I wander if you have tried the patch proposed in comment 21.
> 
> The patch makes sense to me, though I have not had a chance to test it.

Hi Tomas

If you are too busy, and if you don't have objections, I would like to take
care of the spin optimization (ref. comment 32), of course with all respect and
credit to your contribution.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #34 from Mike Jones <mjones@linear.com> 2013-01-02 03:48:27 GMT ---
Can some one provide some simple code examples? I have the patches installed, a
project ready for code, and hardware with I2C devices known to work using my
hardware and Freescale code tools. I'll read the reference manual for the
target independent API, but I am not sure that applies for the current patch or
if I have to use the i2c_freescale.h stuff, or a combination, etc. And the
i2c_freescale.h API does not describe the parameters, so if I need to use that,
it would help to have names of the parameters and some explanation, especially
for bools. I know I can reverse engineer the code, but I am hoping I don't have
to :-)

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #35 from Mike Jones <mjones@linear.com> 2013-01-02 07:07:14 GMT ---
I read the reference on I2C. I am pretty clear how the standard I2C API works
in terms of making transactions using locking, etc.

I am making a simple app with no tasks, just to run in main. I assume there is
some magic for making a cyg_i2c_bus and then you can make some cyg_i2c_device.
Then call freescale_i2c_init(...) I see a macro for creating the bus, but it
has to pass some function pointers. I assume there is some code for that, or I
have to know all the magic things to put in the macro.

Once there is a device, it is not clear if the standard API can use it. And if
it can, I am not certain if it would behave any different that the Freescale
specific API commands.

So I think I mainly need to know how to get things glued together and
relationship between the generic vs Freescale API.

I don't need to know how to use the API to send and receive data.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #36 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2013-01-03 09:09:27 GMT ---
(In reply to comment #33)
> If you are too busy, and if you don't have objections, I would like to take
> care of the spin optimization (ref. comment 32), of course with all respect and
> credit to your contribution.

Please do with it whatever you deem needs doing :)

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #37 from Tomas Frydrych <tf+bugs.ecos@r-finger.com> 2013-01-03 09:12:05 GMT ---
(In reply to comment #34)
> Can some one provide some simple code examples? 

The only example I have is the accelerometer driver, which is in the patches
attached to bug 1001419.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #38 from Ilija Kocho <ilijak@siva.com.mk> 2013-01-03 16:27:42 GMT ---
Created an attachment (id=2017)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=2017)
Freescale I2C driver 130103, incremental to attachments 2004/2005

(In reply to comment #36)
> (In reply to comment #33)
> > If you are too busy, and if you don't have objections, I would like to take
> > care of the spin optimization (ref. comment 32), of course with all respect and
> > credit to your contribution.
> 
> Please do with it whatever you deem needs doing :)

Thank you. Here I propose a code that:
   - decreases i2c thread priority prior to wait spin;
   - returns after timeout (as Mike suggests)

The patch is incremental. Apply it on top of attachment 2004 and attachment
2005.

Ilija


Note: I haven't been able to test it with multiple i2c masters.

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #39 from Mike Jones <mjones@linear.com> 2013-01-06 05:46:55 GMT ---
I am having trouble slowing down the bus speed.

This is my code for write byte, but it is clocking at over 1Mhz, and my devices
only go up to 400Khz.

Is there something else I need to do?

void smbus_write_byte(uint8_t address, uint8_t command, uint8_t data)
{
    cyg_uint8 buffer[2];
    cyg_i2c_device device = {                        \
            .i2c_bus        = &hal_kinetis_i2c0_bus, \
            .i2c_address    = address,               \
            .i2c_flags      = 0,                     \
            .i2c_delay      = 10000                  \
        };

    buffer[0] = command;
    buffer[1] = data;

    cyg_i2c_transaction_begin(&device);
    if(!cyg_i2c_transaction_tx(&device, true, &buffer[0], 2, false)) {
        diag_printf("Read: fail TX.\n");
    }

    cyg_i2c_transaction_stop(&device);
    cyg_i2c_transaction_end(&device);
}

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #40 from Mike Jones <mjones@linear.com> ---
Created attachment 2112
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2112&action=edit
Control clock rate, fix race condition, bug fix

This patch contains new code for managing clock speed that was added by Ilija.
He can provide a ChangeLog for that if desired. I tested/modified his code and
fixed bugs. There was a race condition that caused the driver to lockup if bits
were not set/cleared at the right time, and a few other small things. I don't
remember the exact changes any more without a fresh diff against the original
patch, but I remember the 3 days on the bench with a scope hunting it down :-)
The original author can diff against this version if desired. The actual bug
fixes are just a few lines of critical code.

I have run this on three PMBus devices extensively including a several day 24
hour telemetry run, meaning constant traffic reading and writing. There were no
errors.

If anyone wants to do more testing, I suggest testing something faster than
PMBus which maxes out at 400Khz.

In the future this could be improved to add API to manage SMBus features such
as the timeout mechanism. I don't have time at the moment to do that.

I suggest the original author (or others( apply the patch and test their
existing application. If nobody has any problems, I feel this code is safe to
commit to CVS.

Some example code:


const cyg_uint32 i2c_bus_time = 10000;

    cyg_i2c_device device = {                        \
            .i2c_bus        = &cyg_i2c0_bus,         \
            .i2c_address    = address,               \
            .i2c_flags      = 0,                     \
            .i2c_delay      = i2c_bus_time           \
        };
    cyg_uint8 buffer[1];
    cyg_uint8 input[1];
    buffer[0] = command;

    cyg_i2c_transaction_begin(&device);
    if(!cyg_i2c_transaction_tx(&device, true, &buffer[0], 1, false)) {
        diag_printf("Read Byte: fail TX.\n");
    } else if(!cyg_i2c_transaction_rx(&device, true, &input[0], 1, true, true))
{
        diag_printf("Read Byte: fail RX.\n");
    }
    cyg_i2c_transaction_end(&device);

    return input[0];

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #41 from Mike Jones <mjones@linear.com> ---
I failed state that my patch called Control clock rate, fix race condition, bug
fix is a patch against CVS, not a patch against a patch.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned@bugs.ecos.source |ilijak@siva.com.mk
                   |ware.org                    |

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |1001816

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2004|0                           |1
        is obsolete|                            |
   Attachment #2017|0                           |1
        is obsolete|                            |
   Attachment #2112|0                           |1
        is obsolete|                            |

--- Comment #42 from Ilija Kocho <ilijak@siva.com.mk> ---
Created attachment 2179
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2179&action=edit
I2C driver with removed HAL code 130410

Mike

Here is the driver with HAL executable code removed. Now all code is in driver
and HAL borrows only data (pins, clock, etc).

I reworked clock rate calculator so it now deals with period rather than
frequency. The reason is replacing some division with multiplication (in a
loop). Once being with with the clock, I implemented your idea about frequency
set-point as user convenience. So now CDL accepts Hz but everything else works
with ns (actually for increasing of accuracy with 1/100ns.

I also removed some warnings, by patching high level I2C driver, so please try
also bug 1001816.

In case you submit patches please put driver, HAL, etc. and especially ecos.db
in separate patches. It makes it easier for management. FYI, simultaneously
with submitting you can obsolete one or several old patches.

Ilija

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2005|0                           |1
        is obsolete|                            |

--- Comment #43 from Ilija Kocho <ilijak@siva.com.mk> ---
Created attachment 2180
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2180&action=edit
Kinetis HAL with I2C definitions. 130410

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug 1001397] I2C driver for Kinetic microcontrollers

[bugzilla-daemon]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2006|0                           |1
        is obsolete|                            |

--- Comment #44 from Ilija Kocho <ilijak@siva.com.mk> ---
Created attachment 2181
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2181&action=edit
Freescale I2C driver ecos.db patch 130410

-- 
You are receiving this mail because:
You are on the CC list for the bug.