stm32l1/i2c.c: adapt for new i2c periph driver interface by ReneHerthel · Pull Request #4096 · RIOT-OS/RIOT

ReneHerthel · 2015-10-15T14:03:26Z

Adaptation of the stm32l1/I2C driver for the optimized Interface

Original PR where the i2c periph driver interface was optimized.
Fixed the issue, that there works only the first I2C bus, now both should work.
This should be able to work with the new default I2C device access macro.
Removes some redundant defines/macros.
Added adaptation for nucleo-l1

thomaseichinger · 2015-10-16T12:17:29Z

@ReneHerthel Great, would you mind also taking care of nucleo-l1?

ReneHerthel · 2015-10-19T16:16:10Z

@thomaseichinger This board should work with the same driver, or am I wrong? Or did you just mean I should test it with a nucleo-l1? Then I need one for testing.

thomaseichinger · 2015-10-19T16:34:09Z

@ReneHerthel I'm sorry I was not clear, I meant to also adopt nucleo-l1/include/periph_conf.h as you did for limifrog-v1.

ReneHerthel · 2015-10-19T18:25:52Z

@thomaseichinger Ah, okey :-). Only for I2C? I see there are only definitions for I2C_0 but not for I2C_1. I think I can adapt that, but I need a nucleo-l1 anyway for testing.

thomaseichinger · 2015-10-19T18:42:09Z

That would be really great if you could add I2C_1 too. If you don't have access to a nucleo-l1 I could take the testing part.

ReneHerthel · 2015-10-20T17:49:29Z

@thomaseichinger Think you can test it now.

thomaseichinger · 2015-10-21T10:08:29Z

Ok great, one proposal: What do you think about changing the current approach to something like the UART configuration for the samr21-xpro board. Defining something comparable to uart_conf_t for I2C to hold the configuration defined in periph_conf.h. This way we'd save all these wrapping functions of if statements and replace it with array accesses. This would also limit maintenance to one file, periph_conf.h, if someone would like to change/add/remove a i2c device. So omit all _get_*(i2c_t dev) functions. What do you say?

ReneHerthel · 2015-10-22T09:59:11Z

@thomaseichinger Sounds really interesting! I'll look at it later :-) think it could be a good idea

ReneHerthel · 2015-10-25T18:25:05Z

@thomaseichinger Adapt, think this change makes sense :-)

PeterKietzmann · 2015-10-26T07:19:52Z

~~@ReneHerthel what's the outcome of your thinking?~~
Edit: forget about my latest comment :-), it was by accident

PeterKietzmann · 2015-10-26T07:41:12Z

@thomaseichinger may I assign you?

PeterKietzmann · 2015-10-26T13:44:45Z

boards/limifrog-v1/include/periph_conf.h

Could you add a small comment that says which pin is which?

ReneHerthel · 2015-10-27T19:48:14Z

Rebase.

ReneHerthel · 2015-10-28T17:10:49Z

@thomaseichinger I don't know how to add a mutex to the constant i2c_cfg array, any ideas? I adapt the other things.

BytesGalore · 2015-10-28T17:33:22Z

cpu/stm32l1/include/periph_cpu.h

@ReneHerthel you could add:

mutex_t mtx; /**< i2c config mutex */

here, and change the initialization of the config array [1] by appending the static mutex initializer MUTEX_INIT, e.g.:

{I2C2, GPIO_PIN(PORT_B, 10), GPIO_PIN(PORT_B, 11), 4, I2C2_EV_IRQn, I2C2_ER_IRQn, RCC_APB1ENR_I2C2EN, MUTEX_INIT},

(though this is untested, so don't know if this works)

[1] https://github.com/RIOT-OS/RIOT/pull/4096/files#diff-b7ca3df665fccc20bb1375fdb885cafbR246

@BytesGalore Yes I already tried this, but I got an error like: the i2c_cfg is constant and the Mutex is not.

@ReneHerthel The following is untested but semantically it should be what we want:

typedef struct { const I2C_TypeDef *dev; /**< pointer to the used I2C device */ const gpio_t scl_pin; /**< pin used for SCL */ ... mutex_t mutex; } i2c_conf_t;

and

static i2c_conf_t i2c_cfg[] = { {I2C2, GPIO_PIN(PORT_B, 10), GPIO_PIN(PORT_B, 11), 4, I2C2_EV_IRQn, I2C2_ER_IRQn, RCC_APB1ENR_I2C2EN, MUTEX_INIT}, ... }

@thomaseichinger hmm okey. I'll Change that :)

PeterKietzmann · 2015-10-30T07:23:31Z

@thomaseichinger can you please have a look at this? It would be really comfortable to merge this asap so we can test #3495 and #3263

thomaseichinger · 2015-10-30T08:34:34Z

@PeterKietzmann I will have a look on the code, but I can't test for the next one and a half weeks as I don't have access to l1 board. Are #3495 and #3263 urgent?

PeterKietzmann · 2015-10-30T08:39:38Z

Well, once your're happy with the code, we can test it here. The mentioned PRs are not urgent but I don't want Rene to lose his motivation ;-)

ReneHerthel · 2015-11-01T21:48:27Z

@thomaseichinger Hmm I can compile that, don't know why, but okey! :-), think I did something wrong.. Thanks, it should work now.

BTW: I had to include mutex.h in periph_cpu.h, dont know if that is okey.

thomaseichinger · 2015-11-02T09:09:38Z

@ReneHerthel It did compile because you where not using the mutex in i2c_cfg. But I figured this won't work as I though first. So please move back to ReneHerthel@269f35b and we should be able to get this through soon.

ReneHerthel · 2015-11-02T19:11:39Z

@thomaseichinger Check.

PeterKietzmann · 2015-11-04T16:54:04Z

@katezilla you need this PR for testing #3495 and #3263. Will you voluntary also take over this one :-D ? Thomas already did a code review. It's more about testing

katezilla · 2015-11-05T10:08:08Z

yes, i can test it also

ReneHerthel · 2015-11-28T12:33:15Z

@katezilla any news? :-) I thought you can test the limifrog drivers only with this here?

PeterKietzmann · 2015-12-02T12:26:06Z

@ReneHerthel as we discussed yesterday: please squash! AFAIK you and @katezilla successfully tested your PR

ReneHerthel · 2015-12-02T16:31:36Z

rebase.

ReneHerthel · 2015-12-02T16:33:53Z

@PeterKietzmann @katezilla It is squashed now.

PeterKietzmann · 2015-12-04T07:22:45Z

We needed to put this again to the periph_conf, to satisfy travis:

 #define I2C_NUMOF           (2U)
 #define I2C_0_EN            1
 #define I2C_1_EN            1

@thomaseichinger ( @haukepetersen ) are you fine this way?!

PeterKietzmann · 2015-12-04T08:06:26Z

BTW: Travis is green. So if you agree, we can merge this instantly

thomaseichinger · 2015-12-04T10:57:53Z

ACK & Go

stm32l1/i2c.c: adapt for new i2c periph driver interface

haukepetersen · 2015-12-04T14:41:18Z

I wonder if this PR works? Seems broken: The pins are defined as GPIO_PIN() -> #define GPIO_PIN(x, y) ((GPIOA_BASE + (x << 10)) | y):

i2c_cfg[] = {... GPIO_PIN(PORT_B, 8)}

now the _pin_config() and _toggle_pin()` functions are called like this:

_pin_config(dev, GPIOB, i2c_cfg[dev].scl_pin, i2c_cfg[dev].sda_pin);

So i2c_cfg[dev].scl_pin is of type gpio_t, while the _pin_config() function expects just the pin number...
Further, the port is hard-coded here -> that should not be!

PeterKietzmann · 2015-12-04T14:48:23Z

@ReneHerthel, @katezilla you told me your tests were successful, didn't you? Anyway, what you say seems logic, I will check...

katezilla · 2015-12-04T15:15:07Z

Yes we tested it with the nucleo-l1 and a ultrasound Sensor, it worked fine.

thomaseichinger added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: drivers Area: Device drivers labels Oct 16, 2015

thomaseichinger added this to the Release NEXT MAJOR milestone Oct 16, 2015

thomaseichinger assigned PeterKietzmann Oct 16, 2015

PeterKietzmann reviewed Oct 26, 2015
View reviewed changes

thomaseichinger assigned thomaseichinger and unassigned PeterKietzmann Oct 27, 2015

ReneHerthel force-pushed the stm32l1_i2c_opt branch from fbf24c9 to 4d18bea Compare October 27, 2015 19:47

BytesGalore reviewed Oct 28, 2015
View reviewed changes

ReneHerthel mentioned this pull request Oct 30, 2015

LIS3MDL driver implementation for LimiFrog V1 #3495

Merged

thomaseichinger added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 1, 2015

ReneHerthel force-pushed the stm32l1_i2c_opt branch from 59f7522 to cf0c49f Compare December 2, 2015 16:31

ReneHerthel force-pushed the stm32l1_i2c_opt branch from cf0c49f to f4c7cbe Compare December 2, 2015 16:33

PeterKietzmann added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Dec 2, 2015

ReneHerthel force-pushed the stm32l1_i2c_opt branch from eaae573 to a7a0922 Compare December 3, 2015 18:49

stm32l1/i2c.c: adapt for new periph driver interface

e9215a6

ReneHerthel force-pushed the stm32l1_i2c_opt branch from a7a0922 to e9215a6 Compare December 3, 2015 18:50

thomaseichinger added a commit that referenced this pull request Dec 4, 2015

Merge pull request #4096 from ReneHerthel/stm32l1_i2c_opt

55f4013

stm32l1/i2c.c: adapt for new i2c periph driver interface

thomaseichinger merged commit 55f4013 into RIOT-OS:master Dec 4, 2015

PeterKietzmann mentioned this pull request Dec 7, 2015

Revert "stm32l1/i2c.c: adapt for new periph driver interface" #4428

Merged

ReneHerthel deleted the stm32l1_i2c_opt branch December 28, 2015 13:20

Conversation

ReneHerthel commented Oct 15, 2015

Adaptation of the stm32l1/I2C driver for the optimized Interface

Uh oh!

thomaseichinger commented Oct 16, 2015

Uh oh!

ReneHerthel commented Oct 19, 2015

Uh oh!

thomaseichinger commented Oct 19, 2015

Uh oh!

ReneHerthel commented Oct 19, 2015

Uh oh!

thomaseichinger commented Oct 19, 2015

Uh oh!

ReneHerthel commented Oct 20, 2015

Uh oh!

thomaseichinger commented Oct 21, 2015

Uh oh!

ReneHerthel commented Oct 22, 2015

Uh oh!

ReneHerthel commented Oct 25, 2015

Uh oh!

PeterKietzmann commented Oct 26, 2015

Uh oh!

PeterKietzmann commented Oct 26, 2015

Uh oh!

PeterKietzmann Oct 26, 2015

Choose a reason for hiding this comment

Uh oh!

ReneHerthel Oct 28, 2015

Choose a reason for hiding this comment

Uh oh!

ReneHerthel commented Oct 27, 2015

Uh oh!

ReneHerthel commented Oct 28, 2015

Uh oh!

BytesGalore Oct 28, 2015

Choose a reason for hiding this comment

Uh oh!

ReneHerthel Oct 30, 2015

Choose a reason for hiding this comment

Uh oh!

thomaseichinger Oct 30, 2015

Choose a reason for hiding this comment

Uh oh!

ReneHerthel Oct 30, 2015

Choose a reason for hiding this comment

Uh oh!

PeterKietzmann commented Oct 30, 2015

Uh oh!

thomaseichinger commented Oct 30, 2015

Uh oh!

PeterKietzmann commented Oct 30, 2015

Uh oh!

ReneHerthel commented Nov 1, 2015

Uh oh!

thomaseichinger commented Nov 2, 2015

Uh oh!

ReneHerthel commented Nov 2, 2015

Uh oh!

PeterKietzmann commented Nov 4, 2015

Uh oh!

katezilla commented Nov 5, 2015

Uh oh!

ReneHerthel commented Nov 28, 2015

Uh oh!

PeterKietzmann commented Dec 2, 2015

Uh oh!

ReneHerthel commented Dec 2, 2015

Uh oh!

ReneHerthel commented Dec 2, 2015

Uh oh!

PeterKietzmann commented Dec 4, 2015

Uh oh!

PeterKietzmann commented Dec 4, 2015

Uh oh!

thomaseichinger commented Dec 4, 2015

Uh oh!

haukepetersen commented Dec 4, 2015

Uh oh!