This is not a bug although some folks might want these things turned on by default. It might also be a bit confusing and hard to find in code or documentation, but the default for alot of these peripheral config variables are by default 0. You need to turn them on if you want to use a certain peripheral. There are two basic reasons for this: in some cases there are not enough pins to go around, so enabling everything simply wont work. The other reason is to create a smaller image by default.
I am not sure I understand the #if vs #ifdef thing. #if MYNEWT_VAL(X) should work just fine if you ask me. Will > On Jun 6, 2017, at 4:59 AM, Fabio Utzig <ut...@linux.com> wrote: > > On Tue, Jun 6, 2017, at 08:01 AM, Sigve Sebastian Farstad wrote: >> Hi all, >> >> *Short version: *I2C didn't work, and I think I've narrowed it down to >> a bug with #if directives and the MYNEWT_VAL(..) macro in the core >> that potentially affects many boards.> >> *Long version:* >> I started looking at mynewt for work, and I'm trying to use I2C with >> an nRF52 (both nRF52dk and telee02 boards/bsps). It doesn't work out-of-the- >> box as far as I can tell, so I did some snooping around in the mynewt >> code base. Here's why it didn't work, and some suggestions for how it >> could be fixed. Of course, I'm not very familiar with mynewt, so I >> might be completely off-target.> >> First, here is the way it is set up today: In syscfg.yml, I2C_0 is >> declared so that it can be used by the bsp (e.g. in hal_i2c.c):> Inline >> image 1 >> (incubator-mynewt-core/hw/bsp/nrf52dk/syscfg.yml) >> >> These declarations are magically converted into header defines by the >> newt tool, which will end up looking something like this:> Inline image 2 >> (bin/targets/TARGET_NAME/generated/include/syscfg/syscfg.h in a newt >> project)> >> There is a convenient that macro is used to access these defines: >> Inline image 1 >> (bin/targets/TARGET_NAME/generated/include/syscfg/syscfg.h in a newt >> project)> >> This macro is used in the bsp to include I2C if it is declared in >> syscfg.yml:> Inline image 2 >> (incubator-mynewt-core/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c) >> >> Herein lies the problem. MYNEWT_VAL_I2C_0 is defined to 0. The #if >> directive evaluates truthiness, and the truthiness of the value 0 is >> false. Hence, I2C code is not included even though it should be.> >> So, there are perhaps two bugs in here. 1) why is MYNEWT_VAL_I2C_0 0 >> and not '0', as it is defined in syscfg.yml? And 2) I2C code is not >> included properly.> >> For 1), CfgEntry structs in the newt tool's source only support string >> entries, so I imagine something gets when going from yml entries to C >> literals.> >> One way of solving 2) is to use #ifdef instead of #if when checking if >> a value is defined. However, this doesn't work with the MYNEWT_VAL(..) >> macro, as far as I can tell, so one would need to use MYNEWT_VAL_I2C_0 >> directly instead.> >> Currently, I've worked around the issue by setting the I2C_0 value in >> syscfg.yml to something truthy (e.g. true), but it seems to me that >> the intention is for it to be e.g. 0 for I2C_0 and 1 for I2C_1, since >> the code in hal_i2c.c seems to want to use these numbers as array >> indexes as well.> >> Lastly, this probably affects more than just I2C as the #if >> MYNEWT_VAL(..) pattern is used many places in the core.> >> >> Thoughts? > > I think the "value" in the syscfg.yml files usually means a flag to > enable/disable something. I2C_0 with value 0 means it's disabled, 1 > means it is enabled. Also looking at > "hw/mcu/nordic/nrf52xxx/src/hal_i2c.c", it seems to only use the value > as an enable/disable flag, not as an indexing value. > Cheers, > Fabio Utzig
