cpu/samd21: implement gpio_toggle properly

[LudwigKnuepfer]

edit:
implement gpio_toggle properly, the original method used gpio_read which is not possible if pins are configured as output.

old description:
With this change, gpio_init_out configures pins to buffer the input value,
which in turn allows gpio_read to work if a pin has been configured as
output only.

gpio_toggle relies on gpio_read, so it can't work if gpio_read is not
working.

Tested with tests/periph_gpio which didn't work before and works now.

[jnohlgard]

gpio_toggle relies on gpio_read, so it can't work if gpio_read is not working.

I'm surprised that the samd21 doesn't have a hardware toggle register.

[LudwigKnuepfer]

I didn't read the datasheet, just looked what gpio_init_in does.

[jnohlgard]

I'm not opposed to this PR, and I don't have a samd21 anyway, but I think
the gpio toggle function should use the hardware register for toggling the
pin instead of reading and then writing back. I found a register called
PORT_OUTTGL in the header which might be of interest.
On Mar 23, 2015 6:47 PM, "Ludwig Ortmann" notifications@github.com wrote:

I didn't read the datasheet, just looked what gpio_init_in does.

—
Reply to this email directly or view it on GitHub
#2696 (comment).

[LudwigKnuepfer]

@gebart there I fixed it.

[jnohlgard]

@LudwigOrtmann nice work, sorry if I sounded snarky before, I didn't mean to cause you extra work.

[LudwigKnuepfer]

@gebart I didn't feel offended in any way and I'm grateful for your hint.

[PeterKietzmann]

Here you see the output of GPIO_0, GPIO_1, GPIO_2, GPIO_3, GPIO_8running the tests/periph_gpio. GPIO_3 looks weird

[jnohlgard]

Why do you need to take the address and dereference it?

[LudwigKnuepfer]

copy paste

[LudwigKnuepfer]

@PeterKietzmann looks like something else is driving the pin as well

[jnohlgard]

@PeterKietzmann Run in GDB and set a breakpoint on the line after the closing brace of the for loop toggling the pin. OrIf that doesn't yield any clues, remove the hwtimer_wait inside the toggle loop and single step through it with the logic analyzer connected to see where it goes wrong.
Watchpoints on the output register might help too.

[LudwigKnuepfer]

From my perspective, all other pins look weird on your screenshot. This is what it looks here:

I guess it has to do with some electrical effect of reading pins that are configured as input?

[jnohlgard]

Do you have any pull resistors on any of the lines or are you reading floating inputs on the logic analyzer?

[PeterKietzmann]

I have some strange behavior here that I need to handle first. But I just realized that GPIO_3 (PA19) is connected to the LED.

[LudwigKnuepfer]

My analyzer is floating.

[PeterKietzmann]

I gave it a try again. I assume my analyzer has floating inputs but don't now how to find this out. It is a cheap Saleae 24MHz 8Ch. Here are some things I figured out:

• I guess the unexpected behavior occurs because of LED connected to this pin. I measured the same strange thing on the stm32f4discovery on pins where LEDs are connected to
• the last low-to-high transition (compare channel D3 in this pic) is caused by gpio_init_in(). I guess this is not wanted for the tests/periph_gpio
• using the internal pull-down resistors here doesn't solve the problem
• using an external 10k resistor solves the problem
• using the internal pull-up resistors here leads to a picture similar to the on from @LudwigOrtmann

[LudwigKnuepfer]

OK, so shall I squash?

[PeterKietzmann]

To be honest I'm not sure if this behavior is reasonable or "acceptable". Any opinions from @gebart? Anyway, this is not caused by your PR so I'd say squash and we'll open an issue or so?

[jnohlgard]

The issue that you are seeing is that you are measuring a floating input, which can give you either a 1, 0, none or both, depending on anything. You shouldn't be able to expect to see a deterministic behaviour across different boards when you are measuring the signal out from an input pin, this is to be expected. I agree with @PeterKietzmann , squash and merge. Disclaimer: I have no samd21 board to test this on so I trust your experiment results, @LudwigOrtmann and @PeterKietzmann .

[LudwigKnuepfer]

squashed.

@PeterKietzmann
I don't see a reason to open an issue for undefined behavior of floating pins either.

[PeterKietzmann]

Dammit now it finally "clicked" for me that I measured an INPUT pin. Still it is a bit confusing that I never see a low level on this one pin (connected to the LED) even when it is initialized as GPIO_PULLDOWN. Still ACK and giving Travis a chance until tomorrow morning

[PeterKietzmann]

I feel like travis will never do the job. Anyone against merging without waiting for travis?

[jnohlgard]

I went through Travis' build queue and killed off a bunch of outdated PRs
which were queued for build, but there are still a lot left for Travis to
go through.. I don't like merging without Travis, but it is possible to run
the tests locally by hand. Do you mind running the compile test and static
tests on your side?
On Mar 26, 2015 10:40 AM, "Peter Kietzmann" notifications@github.com
wrote:

I feel like travis will never do the job. Anyone against merging without
waiting for travis?

—
Reply to this email directly or view it on GitHub
#2696 (comment).

[LudwigKnuepfer]

I'd open an issue / follow up PR for addressing #2696 (comment) in all the places it occurs.

[jnohlgard]

@LudwigOrtmann do you know if it is specific to the samd21 port or if it is widespread?

[LudwigKnuepfer]

Not yet.

[PeterKietzmann]

I ran the compile tests and the static tests locally, there was nothing related to this PR so ACK.

[LudwigKnuepfer]

Issue created: #2726