periph/i2c: remodeling part 2: introduce new API #6576

haukepetersen · 2017-02-08T12:35:58Z

EDIT
~~this PR is reabsed on #6575~~

This PR introduces the actual new i2C API as factored out in #4926.

kaspar030

Looks good to me, but I don't have enough experience with i2c to properly review. Will reassign.

kaspar030 · 2017-02-08T12:39:57Z

drivers/include/periph/i2c.h

+ * should enable the pin's internal pull-up resistors. There are however some
+ * use cases for which the internal pull resistors are not strong enough and the
+ * I2C bus will show faulty behavior. This can for example happen when
+ * connecting a logic analyzer which will raise the capacitance of the bus. In


capacitance?

capacity =) as in electrical. Not increase the number of supported nodes, or clock speed =)

kaspar030 · 2017-02-08T12:45:54Z

drivers/include/periph/i2c.h

+/**  @} */
+
+/**
+ * @brief   Read bit needs to be set when reading


Don't defines need @name?

DipSwitch

Looks like a good and complete API to me :)

A way to avoid a to long parameter list could could be made by provide a config struct pointer, since per connected I2C device there is always a fixed address & flags portion. Could save some space with multiple function calls for the same device I guess. But it's not a must. :)

DipSwitch · 2017-02-15T19:27:04Z

drivers/include/periph/i2c.h

+                 void *data, size_t len, uint8_t flags);
+
+/**
+ * @brief   Write data from/to the given I2C device


from/to this could be changed to only to I guess. :)

DipSwitch · 2017-02-15T19:32:26Z

drivers/include/periph/i2c.h

+              const void *data, size_t len, uint8_t flags);
+
+/**
+ * @brief   Convenience function for writing a single byte onto the bus


for writing a single byte onto the bus to for writing a single byte to a device ? or something like that? But I guess you also would like to use this for general call purposes?

immesys · 2017-02-24T07:21:17Z

drivers/include/periph/i2c.h

+    I2C_ADDR10  = 0x01,     /**< use 10-bit device addressing */
+    I2C_REG16   = 0x02,     /**< use 16-bit register addressing */
+    I2C_NOSTOP  = 0x04,     /**< do not issue a STOP condition after transfer */
+    I2C_NOSTART = 0x08      /**< skip START sequence, ignores address field */


How do you do a repeated start? NOSTOP on the previous transaction followed by no flags on the next?

that's what I had in mind, should work, right?!

Yep that should work. Thanks for working on this!

haukepetersen · 2017-02-28T09:05:48Z

Just for info: The I2C remodeling is still high up on my prio list, but I most likely will not get anything done before the end of the 'Embedded World' fair mach 16th... So please be patient :-)

dylad · 2017-09-15T08:06:39Z

Any news about this PR ?

Mario0x1 · 2017-10-06T13:47:30Z

drivers/include/periph/i2c.h

+typedef enum {
+    I2C_ADDR10  = 0x01,     /**< use 10-bit device addressing */
+    I2C_REG16   = 0x02,     /**< use 16-bit register addressing */
+    I2C_NOSTOP  = 0x04,     /**< do not issue a STOP condition after transfer */


So I2C_NOSTOP is START + Address/Data and I2C_NOSTART only a STOP without Address/Data?

Might I suggest instead adding a third Option for I2C_RAW and changing the other two to I2C_START and I2C_STOP? That way you would be good for all eventualities for weird hardware specific commands.

To give you an example, besides the project mentioned by @immesys with its repeated START I am currently working on a port for the ATECC508a. All of the members of Atmels crypto-coprocessors need a wake command which is specified as SDA and SCL LOW for around 100us. That can be achieved by a START Signal followed by nothing for a 100us sleep and sending the address/data 0x00 and a stop afterwards.

My point being, I dont know how many of those non standard combinations for specific hardware are out there and START, STOP and RAWDATA seem like the cleanest solution to me?

edit: Never mind got a bit of a tunnel vision and forgot SAM0s sercom with no separate START. Sorry for that

Actually, I think this might make sense. I think we need 3-4 sample implementations for different vendors to get a feeling if this is doable throughout the line of supported CPUs...

Will get on this full-time after the release to finally get this remodeling done!

comments addressed.

haukepetersen · 2017-11-27T16:36:29Z

I rebased this PR and removed all changes not related to the interface, to make it easier to focus on the actual interface changes for now.

dylad · 2018-01-26T21:42:28Z

drivers/include/periph/i2c.h

 */
-int i2c_init_master(i2c_t dev, i2c_speed_t speed);


Why did we drop the speed param here ?

Because we found that it is not actually needed in the interface and it is more efficient to statically configure this value in the board configuration. As I2C is not like SPI, where each device on the bus can be interfaces with a different clock speed, the bus clock for I2C buses needs to be assigned globally for all devices on the bus, so we might as well define it as part of the I2C config in the boards periph_conf.h.

Per default, the i2c_init_master() is to be called during system initialization (same as spi_init() in from periph_init()). In this context, it is much easier to pass the clock speed as configuration parameter instead of a function parameter...

smlng · 2018-08-02T15:10:00Z

done in #9548

haukepetersen added Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Feb 8, 2017

haukepetersen added this to the Release 2017.04 milestone Feb 8, 2017

haukepetersen assigned jnohlgard, DipSwitch, aabadie, kaspar030 and PeterKietzmann Feb 8, 2017

This was referenced Feb 8, 2017

periph/i2c: tracking the remodeling #6577

Closed

periph/i2c: remodeling part 1: deprecate existing API #6575

Closed

kaspar030 previously requested changes Feb 8, 2017

View reviewed changes

dylad mentioned this pull request Feb 10, 2017

SAM L21 i2c support #6332

Merged

DipSwitch reviewed Feb 15, 2017

View reviewed changes

immesys reviewed Feb 24, 2017

View reviewed changes

smlng mentioned this pull request Feb 28, 2017

drivers/veml6070: add implementation for UV sensor + saul adaptation #6665

Merged

aabadie mentioned this pull request Mar 17, 2017

boards: fix I2C_APBCLK value in stm32-based boards #6759

Merged

kaspar030 modified the milestone: Release 2017.04 Apr 21, 2017

aabadie modified the milestone: Release 2017.10 Jun 23, 2017

miri64 mentioned this pull request Jul 4, 2017

ds1307: initial import of a driver for the DS1307 RTC #7312

Merged

Mario0x1 reviewed Oct 6, 2017

View reviewed changes

smlng modified the milestones: Release 2017.10, Release 2018.01 Nov 16, 2017

PeterKietzmann mentioned this pull request Nov 27, 2017

stm32f1: I2C read functions return bogus values with unconnected devices #7846

Closed

haukepetersen added 2 commits November 27, 2017 17:29

drivers/periph: introduced new I2C interface

b8e5ceb

drivers/periph_common: included I2C initialization

3e6da18

haukepetersen force-pushed the opt_periph_i2c_if branch from ace964f to 3e6da18 Compare November 27, 2017 16:32

smlng modified the milestones: Release 2018.01, Release 2018.04 Jan 12, 2018

dylad reviewed Jan 26, 2018

View reviewed changes

haukepetersen mentioned this pull request Jan 31, 2018

cpu/nrf52/i2c: use static mutex initializtion #8486

Merged

haukepetersen mentioned this pull request Feb 26, 2018

drivers/periph: reworked I2C driver #4926

Closed

PeterKietzmann mentioned this pull request Mar 1, 2018

boards: Jiminy-mega256rfr2: initial support #8643

Merged

kaspar030 removed this from the Release 2018.04 milestone Apr 16, 2018

dylad mentioned this pull request May 22, 2018

I2C: introduce new API #9162

Merged

This was referenced May 22, 2018

tools/bpt_interface: Interface for a hardware test device #9163

Closed

tests/periph_i2c: Update to new I2C API #9168

Merged

dylad mentioned this pull request Jul 14, 2018

I2C: introduce and adapt new I2C interface (2nd attempt) #9548

Merged

smlng closed this Aug 2, 2018

gschorcht mentioned this pull request Aug 9, 2018

periph/i2c: Possibility to set I2C bus speed is missing in new I2C #9753

Open

gschorcht mentioned this pull request Dec 29, 2018

periph/i2c: return values of i2c_aquire and i2c_release #10673

Closed

periph/i2c: remodeling part 2: introduce new API #6576

periph/i2c: remodeling part 2: introduce new API #6576

Uh oh!

Conversation

haukepetersen commented Feb 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaspar030 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DipSwitch Feb 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DipSwitch left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Feb 28, 2017

Uh oh!

dylad commented Sep 15, 2017

Uh oh!

Mario0x1 Oct 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Nov 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Aug 2, 2018

Uh oh!

Uh oh!

haukepetersen commented Feb 8, 2017 •

edited

Loading

DipSwitch Feb 15, 2017 •

edited

Loading

DipSwitch left a comment •

edited

Loading

Mario0x1 Oct 6, 2017 •

edited

Loading