Skip to content

cpu/stm32f1: modernized peripheral GPIO driver#3535

Merged
PeterKietzmann merged 4 commits intoRIOT-OS:masterfrom
haukepetersen:opt_f1_gpio
Aug 6, 2015
Merged

cpu/stm32f1: modernized peripheral GPIO driver#3535
PeterKietzmann merged 4 commits intoRIOT-OS:masterfrom
haukepetersen:opt_f1_gpio

Conversation

@haukepetersen
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen commented Jul 31, 2015

One more CPU on the bright side :-)

Code size before (tests/periph_gpio, iot-lab_M3):

   text    data     bss     dec     hex
  16492     120    2888   19500    4c2c

and after:

   text    data     bss     dec     hex
  15244     120    2888   18252    474c

Savings ~1,2k

@haukepetersen haukepetersen 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 Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Jul 31, 2015
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Jul 31, 2015

nice!

@haukepetersen haukepetersen mentioned this pull request Jul 31, 2015
Merged
PeterKietzmann
PeterKietzmann reviewed Aug 3, 2015
View reviewed changes
cpu/stm32f1/include/periph_cpu.h Outdated
Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann Aug 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stm32f1 family

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Aug 3, 2015

The changes look valid to me at a first glance. I don't have all boards here to test, just the iot-lab_M3. Will do now

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Aug 3, 2015

The transceiver driver seems so work correctly on the iot-lab_M3 node. Does anyone know the pin-setup of the iot-lab connector? Can anyone test this on spark-core and fox?

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Aug 4, 2015

Ok, how do we proceed? @haukepetersen did you test it on spark-core and/or fox boards? We don't have those...

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Aug 4, 2015

I will try it out and fix the comments tomorrow.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Aug 5, 2015

addressed comments and added missing implementation of the init_af function. Will make a follow up PR to adjust the other peripheral drivers accordingly.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Aug 5, 2015

I am un-able to test the spark-core, my board seems to be electrically broken...

@PeterKietzmann PeterKietzmann mentioned this pull request Aug 5, 2015
Merged
@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Aug 5, 2015

found one more bug in the _port_pin() function, a -2 offset was missing...

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Aug 5, 2015

Why is this offset needed?
I tested this PR again on iot-lab_M3 with tests/driver_at86rf2xx and tests/periph_gpio. It seems to be fine.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Aug 5, 2015

Without the offset, Port A would lead to 2 -> which breaks the clock initialization. This just did not show, as the clocks were also activated hard-coded by other peripheral drivers...

@PeterKietzmann PeterKietzmann added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 5, 2015
@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Aug 5, 2015

two more fixes:

  • removed the GPIO_xx defines from the iot-labs periph_conf
  • enable GPIO clock in gpio_init_af function

@haukepetersen haukepetersen mentioned this pull request Aug 5, 2015
Merged
PeterKietzmann
PeterKietzmann reviewed Aug 5, 2015
View reviewed changes
cpu/stm32f1/include/periph_cpu.h
Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann Aug 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you shortly explain these values? Maybe referring to the reference manual (sec.9.2.1, p. 171 or table 20, p.161)? I think it's much faster than figuring this out by myself

Copy link
Copy Markdown
Contributor Author

@haukepetersen haukepetersen Aug 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure: To understand them, have a look at the gpio_init function and how they are used. I made a bit-map table with the configuration values for the CR[L|H] register and then tried to map the values of DIR and PP to this table. The values you see here is what you get.

Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann Aug 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I totally overlooked if (dir == GPIO_DIR_OUT) there

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Aug 6, 2015

@haukepetersen on iot-lab_M3 I tested tests/periph_gpio, tests/driver_at86rf2xx, examples/default with success. Travis gives us green light. I think no one else will test this on fox and spark-core. So this is an ACK. Please squash your latest commits (or at least remove the "s")

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Aug 6, 2015

squashed. Waiting for Travis once more to make sure I did not break anything while squashing...

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Aug 6, 2015

Let's do this. ACK and go

PeterKietzmann added a commit that referenced this pull request Aug 6, 2015
@PeterKietzmann
Merge pull request #3535 from haukepetersen/opt_f1_gpio
68d0519 
cpu/stm32f1: modernized peripheral GPIO driver
@PeterKietzmann PeterKietzmann merged commit 68d0519 into RIOT-OS:master Aug 6, 2015
@haukepetersen haukepetersen deleted the opt_f1_gpio branch August 6, 2015 12:42
@PeterKietzmann PeterKietzmann mentioned this pull request Aug 11, 2015
Merged
@PeterKietzmann PeterKietzmann mentioned this pull request Sep 3, 2015
Closed
@mayman99 mayman99 mentioned this pull request Sep 17, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@PeterKietzmann PeterKietzmann

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@haukepetersen @kaspar030 @PeterKietzmann