Skip to content
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

zephyr: io: trivial fixes after I/O functions separation (if MCUBOOT_INDICATION_LED enabled) #1874

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

pepe2k
Copy link
Contributor

@pepe2k pepe2k commented Dec 5, 2023

This short series includes 3 warning/error fixes found when building with MCUBOOT_INDICATION_LED and LOG enabled:

  • zephyr: rename 'led_init()' to 'io_led_init()'
  • zephyr: io: include 'bootutil_log.h' and declare log module membership
  • zephyr: io: add 'io_led_set()'

All issues were introduced in 433b848.

The last fix (zephyr: io: add 'io_led_set()') should be treated as more a proposal as there might be other solutions/approaches preferred.

@nordicjm nordicjm requested a review from de-nordic December 5, 2023 15:40
@pepe2k pepe2k marked this pull request as draft December 6, 2023 06:04
@pepe2k
Copy link
Contributor Author

pepe2k commented Dec 6, 2023

@nordicjm @de-nordic thanks for review and approval but it seems bootutil_log.h is not enough to fix logging related issue. I forgot to test it with LOG enabled. It still breaks but now with (no clue for now but I'm looking into it):

In file included from zephyr/include/zephyr/logging/log.h:11,
                 from zephyr/include/zephyr/usb/usb_device.h:43,
                 from bootloader/mcuboot/boot/zephyr/io.c:26:
mcuboot/boot/zephyr/io.c: In function 'io_led_init':
zephyr/include/zephyr/logging/log_core.h:151:20: error: '__log_level' undeclared (first use in this function)
  151 |         (_level <= __log_level) &&                                          \
      |                    ^~~~~~~~~~~

I have changed this PR to draft to make sure it won't get merged until this is sorted out.

This fixes below warning when building with 'MCUBOOT_INDICATION_LED'
enabled:

  mcuboot/boot/zephyr/main.c:410:5:
    warning: implicit declaration of function 'led_init';
             did you mean 'io_led_init'? [-Wimplicit-function-declaration]
      410 |     led_init();
          |     ^~~~~~~~
          |     io_led_init

Fixes: 433b848 ("zephyr: Move IO functions out of main to separate file")
Signed-off-by: Piotr Dymacz <[email protected]>
This fixes below error when building with 'MCUBOOT_INDICATION_LED' and
'LOG' enabled:

  In file included from zephyr/include/zephyr/logging/log.h:11,
                   from zephyr/include/zephyr/usb/usb_device.h:43,
                   from bootloader/mcuboot/boot/zephyr/io.c:26:
  mcuboot/boot/zephyr/io.c: In function 'io_led_init':
  zephyr/include/zephyr/logging/log_core.h:151:20: error:
    '__log_level' undeclared (first use in this function)
    151 |         (_level <= __log_level) &&                                          \
        |                    ^~~~~~~~~~~

Fixes: 433b848 ("zephyr: Move IO functions out of main to separate file")
Signed-off-by: Piotr Dymacz <[email protected]>
The static declaration of 'led0' was moved to 'io.c' which broke
building with the 'MCUBOOT_INDICATION_LED' enabled:

  mcuboot/boot/zephyr/main.c:380:22: error:
    'led0' undeclared (first use in this function)
      380 |     gpio_pin_set_dt(&led0, 1);
          |                      ^~~~

This adds simple function 'io_led_set()' for changing LED's value.

Fixes: 433b848 ("zephyr: Move IO functions out of main to separate file")
Signed-off-by: Piotr Dymacz <[email protected]>
@pepe2k pepe2k force-pushed the main__io-separate-leftovers-fixes branch from 248246b to 1f3b103 Compare December 6, 2023 06:43
@pepe2k pepe2k marked this pull request as ready for review December 6, 2023 06:44
@pepe2k
Copy link
Contributor Author

pepe2k commented Dec 6, 2023

Sorted out, BOOT_LOG_MODULE_DECLARE was missing. I have included it, reworded related commit and rebased on main. Sorry for the noise!

@pepe2k pepe2k requested review from nordicjm and de-nordic December 6, 2023 06:47
@nordicjm nordicjm merged commit 2a74a2b into mcu-tools:main Dec 6, 2023
55 checks passed
@pepe2k pepe2k deleted the main__io-separate-leftovers-fixes branch December 7, 2023 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants