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

Place data from .uicrREGOUT0 into UICR.REGOUT0 flash word. #139

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

geeksville
Copy link

The REGOUT0 flash register to set VCC voltage upon power-up. This adds support to allow optionally setting that register in the generated hex file (similar to the other UICR words you already support).

If not used, there is no change to the output files.

Example usage from my board's pinconfig.c file:

 __attribute__ ((section(".uicrREGOUT0"))) volatile uint32_t m_uicr_regout0 = 0xfffffff4;

(These are a few commits I thought might be useful after bringing up an open-source radio board - I can send in the changes for that board if you want it (once the next spin comes back and we test it)?)

Used to set VCC voltage upon power-up.
Copy link
Member

@hathach hathach left a comment

Choose a reason for hiding this comment

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

Look good, since the REGOUT0 also exists with nrf52833 as well, would you mind copying this and paste to the linker of 52833 as well. I am sure people will find it useful

@rafacouto
Copy link

Warning: REGOUT0 also sets the voltage level for GPIO output. It could be problematic when reflashing the MCU by SWD pins...

@geeksville
Copy link
Author

@hathach, sure I'll do that.

@rafacouto, yep - this can definitely break things, but it is also absolutely required on some boards because vcc powers other important things. I'll add a warning to not use this unless you are careful (and example usage) in a comment.

reflashing is still okay if the user connects their vref input on their ICE to the VCC rail.

@hathach
Copy link
Member

hathach commented Jun 16, 2020

Warning: REGOUT0 also sets the voltage level for GPIO output. It could be problematic when reflashing the MCU by SWD pins...

I ddin't know it is that dangerous. I will double check the manual. As long as the output binary is the same without using the section. It should be still good/safe to go. @geeksville Maybe you could drop a line of comment for warning in the linker script. Since existing boards don't use it at all.

@geeksville
Copy link
Author

yep - warning is in that linker script. Yeah - when not populated, nothing is written to that address so the flash stays at the default 0xffffffff.

@Nicell
Copy link
Contributor

Nicell commented Jan 30, 2021

@geeksville thanks for adding this! This is immensely useful for boards that run with VDDH but need a higher GPIO logic level than 1.8V (which is the default). I've hacked this together in the past, but this is a much more proper implementation.

I don't really see how this would be dangerous. If anything, the default 1.8V is the dangerous piece. I've spoken with a few people that have been unable to program their boards due to that default logic level. @hathach could you take a second look at this? I think it's a great improvement to this project.

@lyusupov
Copy link
Contributor

lyusupov commented Jan 30, 2021

This PR functionality is already superseded by merged #177

Just simply add:


#define UICR_REGOUT0_VALUE UICR_REGOUT0_VOUT_3V3

into your board.h file.

But be aware of this particular PR as well: #186

@hathach
Copy link
Member

hathach commented Feb 1, 2021

I really like the solution that is implemented to have define in the board.h. Therefore this PR could be safely close by now.

#define UICR_REGOUT0_VALUE UICR_REGOUT0_VOUT_3V3

@hathach hathach closed this Feb 1, 2021
@geeksville
Copy link
Author

geeksville commented Feb 2, 2021

@hathach, @Nicell and @lyusupov I think you might want to think twice about that other PR. It seems to me that writing UICR later after the first boot (as that other change does) will not work for many applications.

Because some boards have critical components powered by the VCC output rail from the CPU. And without that rail powered at the proper voltage the first boot might not be possible at all. Which is part of why I suspect nordic put this in the flash config registers.

@lyusupov
Copy link
Contributor

lyusupov commented Feb 2, 2021

There is nothing new in the #177 / #186 method.
This is typical solution for developers to use suggested by Nordic DevZone forum reporters.
Moreover, Zephyr project is using this method together with Nordic reference design USB dongle

@hathach hathach reopened this Feb 4, 2021
@hathach hathach closed this Feb 4, 2021
@hathach
Copy link
Member

hathach commented Feb 4, 2021

@geeksville would you mind testing to see if that first boot, method work with your board. I kind of like e it more because of its simplicity and configurable right within the board.h . Sorry for late response, I have been occupied by other works

@geeksville
Copy link
Author

geeksville commented Feb 6, 2021

@hathach alas - my queue is kinda full right now with other things also (and the board I used this on still hasn't been shipped by the mfg). But there is a reason that nordic put this setting in a special flash register (rather than just a normal CPU register), which is "the problem of critical components being powered by VCC." The solution in 177 is sub-optimal (and might make some boards impossible to properly power sequence) because it guarantees that at least one boot (the first one) has incorrect VCC voltage before trying to run code.

Nordic put REGOUT0 in flash so that the processor could properly configure VCC before running even the first instruction.

The other patch is not 'wrong' but I'm just letting you know it might not be sufficient for all boards. IMO a cleaner solution is to let the linker put the correct word into the flash image directly. And it is quite elegant because it shows up as just another optional constant you can put into your C code.

You could also just use it based on a constant from board.h. Without the nasty second boot hack and writing flash from the bootloader.

__attribute__ ((section(".uicrREGOUT0"))) volatile uint32_t m_uicr_regout0 = UICR_REGOUT0_VALUE;

@hathach
Copy link
Member

hathach commented Feb 8, 2021

Thanks @geeksville , to be honest, I am not entirely sure on this matter, but didn't have time to test this as well. The pico and other works take higher priority now. I will re-open this, maybe we could come back later when having more time and/or as reference/solution for others that get into trouble that you just mentioned. Thanks again

@hathach hathach reopened this Feb 8, 2021
@dotnfc
Copy link

dotnfc commented May 8, 2021

I really like the solution that is implemented to have define in the board.h. Therefore this PR could be safely close by now.

#define UICR_REGOUT0_VALUE UICR_REGOUT0_VOUT_3V3

create a Makefile.user with:

CFLAGS += -DUICR_REGOUT0_VALUE=UICR_REGOUT0_VOUT_3V3

for pca10059, 3.3V output OK.

this will not touch the board.c

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.

None yet

6 participants