Skip to content

cc26x2_cc13x2: trim device registers on cpu_init.#13816

Merged
benpicco merged 10 commits intoRIOT-OS:masterfrom
btcven:2020_04_02-setup-trim-device
May 1, 2020
Merged

cc26x2_cc13x2: trim device registers on cpu_init.#13816
benpicco merged 10 commits intoRIOT-OS:masterfrom
btcven:2020_04_02-setup-trim-device

Conversation

@jeandudey
Copy link
Copy Markdown
Contributor

Contribution description

This is a very stripped-down port from the TI SDK of the
SetupTrimDevice function which must be called on CPU initialization.

It sets configurations from the CCFG (Customer Configuration Area), most
of the code is already implemented in the ROM and we use that API.

More importantly, these ROM functions aren't present on cc26x0 devices
and must be implemented manually on these to just implement
SetupTrimDevice, so I opted out on writing a more general
implementation as the code for that is quite big and the review effort
for that kind of code is simply too much (more than writing it IMHO), it
also happens that I don't have one of these to test.

This function is important because it allows us to change the SCLK_HF
clock source to the external HF XOSC, without this, the MCU simply halts
when performing the switch due to a bad state on the MCU registers.

Also this function sets some FLASH registers to a working state, so we
can write in the future a MTD driver for it.

There's room for improvement in this port, mainly refactorings, but as
documentation is scarse I tried to do a one-to-one port without changing
the logic too much, even some variables don't make sense, or some code
blocks. Unbelievably, it's much worse on the TI SDK, of course we have
the opportunity to change that on RIOT 😉.

Feel free to criticize the code, as I personally think that can be improved,
but I need some opinions though.

Testing procedure

  • Green travis/murdock.
  • Any test/example should run as normal, there isn't a change in the functionality.

Issues/PRs references

Needed for #13295.
Fixes one item on #13635.

@jeandudey jeandudey requested a review from smlng as a code owner April 4, 2020 15:13
@benpicco benpicco added Area: cpu Area: CPU/MCU ports Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 4, 2020
@jeandudey jeandudey force-pushed the 2020_04_02-setup-trim-device branch from fef6f74 to 313a92b Compare April 5, 2020 20:07
Copy link
Copy Markdown
Contributor

@benpicco benpicco 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 at a first glance.
I suppose all those register offset calculations are just like that in the vendor code?

@jeandudey
Copy link
Copy Markdown
Contributor Author

I suppose all those register offset calculations are just like that in the vendor code?

Yes, they're the same

@cgundogan cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 5, 2020
@jeandudey
Copy link
Copy Markdown
Contributor Author

I have some issues, with the code right now, I'm investigating. I'll mark it as WIP.

@jeandudey jeandudey changed the title cc26x2_cc13x2: trim device registers on cpu_init. [WIP] cc26x2_cc13x2: trim device registers on cpu_init. Apr 6, 2020
@tcschmidt tcschmidt added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Apr 11, 2020
This function is needed to setup the AUX operational mode at startup,
also used for managing low-power states.

Signed-off-by: Jean Pierre Dudey <[email protected]>
- Fixes padding.
- Updates documentation.
- Removes documentation longer than 80-chars for the registers values.

Signed-off-by: Jean Pierre Dudey <[email protected]>
- Updated documentation
- Fixed register bank name
- Added missing field

Signed-off-by: Jean Pierre Dudey <[email protected]>
- Changed "meh" to "Reserved".
- Renamed CTL to CFG to match SDK/TRM name.
- Added constants for VIMS and FLASH necessary to trim registers.

Signed-off-by: Jean Pierre Dudey <[email protected]>
These are necessary to trim some registers at startup.

Signed-off-by: Jean Pierre Dudey <[email protected]>
Add some register values needed to trim registers.

Signed-off-by: Jean Pierre Dudey <[email protected]>
- Updated documentation.
- Fixed offset of JTAGUSERCODE.
- Added necessary register values to perform startup trims.

Signed-off-by: Jean Pierre Dudey <[email protected]>
- Added ADI instruction offsets
- Added register banks and address bases for masked access (writes).

Signed-off-by: Jean Pierre Dudey <[email protected]>
This function trims the necessary registers for the device to operate
normally.

Signed-off-by: Jean Pierre Dudey <[email protected]>
@jeandudey jeandudey force-pushed the 2020_04_02-setup-trim-device branch from 932b0b8 to 951a99d Compare April 29, 2020 23:34
@jeandudey jeandudey changed the title [WIP] cc26x2_cc13x2: trim device registers on cpu_init. cc26x2_cc13x2: trim device registers on cpu_init. Apr 29, 2020
@jeandudey
Copy link
Copy Markdown
Contributor Author

@benpicco

This is ready for review again, works correctly and matches original implementation 1 to 1. Some registers were fixed in the process (probably the reason of the brick I got previously) and all of the new are correctly defined (and on the correct address).

@benpicco benpicco removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Apr 30, 2020
@cgundogan cgundogan removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 30, 2020
@cgundogan cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 30, 2020
Copy link
Copy Markdown
Contributor

@benpicco benpicco 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 to me.
As far as I understand, those functions are not used yet but follow-up PRs make use of them?

@jeandudey
Copy link
Copy Markdown
Contributor Author

Looks good to me.
As far as I understand, those functions are not used yet but follow-up PRs make use of them?

Yes you're right, in this PR only setup_trim_device is called on cpu_init, and the rest of the registers are used on follow-up PRs.

Thanks for the review!

@benpicco benpicco merged commit c95e4c3 into RIOT-OS:master May 1, 2020
@jeandudey jeandudey deleted the 2020_04_02-setup-trim-device branch May 1, 2020 14:17
@miri64 miri64 added this to the Release 2020.07 milestone Jun 24, 2020
@lutgaru lutgaru mentioned this pull request Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR 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.

5 participants