cpu/fe310/gpio: use gpio_irq feature

[haukepetersen]

See #9992

[miri64]

The binary size doesn't change with this PR, regardless if I compile with or without FEATURES_OPTIONAL += periph_gpio_irq, also the binary is larger than in current master :-/. So I can't use the shortcut for testing I used in other PRs in this family. @kaspar030 if I remember correctly you have a hifive1. Can you have a look?

[kaspar030]

@kaspar030 if I remember correctly you have a hifive1. Can you have a look?

The hifive1 is on my desk at FU. I won't be there for a while, maybe you could use that for testing?

[miri64]

Actually, this problem was caused because I was building with docker (see #10001 (comment)). I'll retry with setting the variables within the Makefile instead.

[miri64]

It still grows for some reason by 8 bytes compared to master without periph_gpio_irq, but I was able to compare the build sizes and diffs like I did in e.g. #10002:

As far as I can tell (and checked with colordiff -u <(git show | grep "^-" | sed 's/^-//g') <(git show | grep "^+" | sed 's/^+//g')) this just moves and ifdefs some code around. After a local rebase to include #9981 I also was able to build this with tests/periph_gpio both with periph_gpio_irq manually added or removed to FEATURES_OPTIONAL. […]

As with #9997, this PR needed another level of diffing, because the move wasn't that obvious, but I checked this as well:

colordiff -u \
    <(diff -u <(git show | grep "^-" | sed 's/^-//g') \
              <(git show | grep "^+" | sed 's/^+//g') | grep "^-" | sed 's/^-//g') \
    <(diff -u <(git show | grep "^-" | sed 's/^-//g') \
              <(git show | grep "^+" | sed 's/^+//g') | grep "^+" | sed 's/^+//g')

[gschorcht]

When I execute the make command directly within docker, it works as expected and the image size reduces when DISABLE_MODULE=periph_gpio_irq is used at make command line. When I run docker from host with BUILD_IN_DOCKER, I get the same problem.

Is that something I should look at? You said it seems to be specific for esp8266 platform.

[miri64]

Is that something I should look at? You said it seems to be specific for esp8266 platform.

I asked @cladmi about it and it is a known, but not easy (or even practicable) to fix problem. Basically, the docker container would need to get every possible variables handed via the environment filtered from irrelevand variables. This sounded to me rather like a nice to have, than something sensible to implement or maintain, so I didn't open an issue for that.

But anyway... this is completely unrelated from this PR

[cladmi]

It's easy to fix on a per variable basic, it means adding it in here:

RIOT/makefiles/docker.inc.mk

Line 27 in 4e92c2a

export DOCKER_ENV_VARS = \

But what I meant it is more a global issue of knowing what should be exported globally than just adding one variable there. A solution to make it flexible could be to use DOCKER_ENV_VARS += so you could do

DOCKER_ENV_VARS=ANY_VARIABLE  ANY_VARIABLE=value make BUILD_IN_DOCKER=1

[miri64]

It's easy to fix on a per variable basic, it means adding it in here:

The per variable basis was the thing that sounded to me as "something [that is not] sensible to implement or maintain" ;-).

I'm not sure your proposed solution is really one. You would need to know it the same way you would need to know that some variables are not provided to the docker container's environment.

[cladmi]

You would need to know indeed, but currently even if you know you can't do it from the command line.

It would just allow setting some non handled variables.
And there are application/board specific variables that will never be globally exported.

And even if you don't know, you could basically always add it to DOCKER_ENV_VARS even if it is already in to be sure :D