Skip to content

cpu/native: Allow Access to Hardware GPIO Pins on Linux #12451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 25, 2020

Conversation

benpicco
Copy link
Contributor

Contribution description

This adds GPIO support for native using the new Linux GPIO API.

Testing procedure

Compile tests/periph_gpio for the native board on any SBC, e.g. a Raspberry Pi.
Run bin/native/tests_periph_gpio.elf --gpio=/dev/gpiochip0

Please consult your board's manual for the pin mapping. For the Raspberry Pi, this would be
Raspi Pin Map

Here, GPIO20 and GPIO21 can be conveniently bridged with a jumper.

Setting and Reading GPIOs should work with this PR already:

> init_in 0 20
> init_out 0 21
> read 0 20
GPIO_PIN(0.20) is LOW
> set 0 21
> read 0 20
GPIO_PIN(0.20) is HIGH

For interrupts to work, #12428 is needed:

> init_int 0 20 2
GPIO_PIN(0, 20) successfully initialized as ext int
> init_out 0 21
> set 0 21
> INT: external interrupt from pin 20

Issues/PRs references

depends on #12428
supersedes #7530

@benpicco benpicco added Platform: native Platform: This PR/issue effects the native platform Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Oct 14, 2019
@benpicco benpicco requested a review from bergzand October 14, 2019 20:24
@maribu
Copy link
Member

maribu commented Oct 15, 2019

This was on my to do list for quite some time, but I never came to id. Thanks for tackling this!

@fhessel
Copy link
Contributor

fhessel commented Oct 16, 2019

I'd appreciate that feature, too! Just a short note here, with #11352 being merged now:

I think, the definition of SPI_HWCS(x) will need to be adjusted, as the hardware CS number space collides with the one used for GPIOs. The same goes for the functions using spi_cs_t parameters in spidev_linux.c. If I can help with that, just let me know.

@benpicco benpicco force-pushed the native-gpio branch 3 times, most recently from 08f0de4 to 2f10b6d Compare October 16, 2019 15:11
@benpicco
Copy link
Contributor Author

If you already have a solution in mind, feel free to send a PR to my native-gpio branch.
I don't quite understand yet how your !use_hwcs case works. All those /dev/spidev0.x files are hardware CS from the perspective of RIOT, no?

@fhessel
Copy link
Contributor

fhessel commented Oct 16, 2019

By now, RIOT can only control hardware CS lines, as Linux' userspace SPI API only allows those hard-wired connections. In case of the Raspberry Pi, this is pin CE0 (GPIO8) on spidev0.0 and CE1 (GPIO7) on spidev0.1.

However, there is a flag in the SPI API called SPI_NO_CS, which corresponds to "1 dev/bus, no chipselect" on system level. My idea would be, if a valid GPIO pin is used for the cs parameter in RIOT, to use the first Linux spidev for that bus, set this NO_CS flag and control the chipselect line through RIOT's GPIO module. This was not possible without an implementation on top of the kernel's GPIO API.

If that works, it would allow arbitrary GPIOs for CS and also more than two devices. If not, I'd at least separate the number space, so that SPI_HWCS(0) != GPIO(0, 0). This allows to differentiate between actual GPIOs and values for HWCS, so that the developer can get proper feedback on what went wrong.

I'll have a look at it and see if that flag behaves like expected.

@fhessel
Copy link
Contributor

fhessel commented Oct 24, 2019

Sorry that it took me so long, testing this with limited access to actual SPI hardware was not that easy.

benpicco#4 contains a suggestion to solve the numbering issue between SPI_HWCS and GPIO_PIN and to allow arbitrary GPIOs for the chip select line.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 4, 2020
@benpicco benpicco added State: waiting for other PR State: The PR requires another PR to be merged first and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 4, 2020
@benpicco benpicco requested review from smlng and kaspar030 March 6, 2020 12:35
@mfp20
Copy link

mfp20 commented May 10, 2020

No support yet?

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

I haven't checked all the details but it seems to me that this PR is in quite a good shape. There's just the problem that I see with the module dependency that I think should be changed.

@miri64
Copy link
Member

miri64 commented Jun 25, 2020

@aabadie your comments seem to be addressed. Care to take another look, please?

@aabadie
Copy link
Contributor

aabadie commented Jun 25, 2020

your comments seem to be addressed. Care to take another look, please?

Problem is that it depends on #12428 which cannot be easily tested on OSX.

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

This ends-up in lots of not directly related changes unfortunately (because now new drivers/packages can be build on native).
I'm wondering if we should split them in separate PRs to have more focused reviews.

USEMODULE += periph_spidev_linux
ifeq ($(OS),Linux)
ifneq (,$(filter periph_gpio,$(USEMODULE)))
DEFAULT_MODULE += periph_gpio_linux
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DEFAULT_MODULE += periph_gpio_linux
USEMODULE += periph_gpio_linux

I would not use DEFAULT_MODULE for this.

If it's because of the failing tests, then in the tests, instead of using DISABLE_MODULE, add native to CI_BOARD_BLACKLIST.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it could still be useful if someone wants to use the mock GPIO implementation instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

How is the mock named?, maybe something like this can be used?

Suggested change
DEFAULT_MODULE += periph_gpio_linux
ifneq (,$(filter periph_gpio_mock,$(USEMODULE)))
USEMODULE += periph_gpio_linux
endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mock is just periph_gpio.
I can make that a pseudomodule and move the implementation to gpio_mock.c.

Then in the tests we can do

ifeq (native,$(BOARD))
  USEMODULE += periph_gpio_mock
endif

@benpicco benpicco added the State: waiting for other PR State: The PR requires another PR to be merged first label Jul 30, 2020
@benpicco
Copy link
Contributor Author

I moved all the unrelated commits to a separate PR: #14663

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 31, 2020
@benpicco benpicco removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 21, 2020
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Looks good now. Please squash.

ACK!

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 25, 2020
@aabadie
Copy link
Contributor

aabadie commented Aug 25, 2020

Murdock found a couple of problems. Can you have a look @benpicco ?

@benpicco
Copy link
Contributor Author

Rebased to fix the warnings introduced by tests/driver_at86rf2xx_aes.

@aabadie aabadie merged commit 9f28e1d into RIOT-OS:master Aug 25, 2020
@benpicco benpicco deleted the native-gpio branch August 25, 2020 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.