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

feat(PeriphDrivers): Add PinMux tool supporting functions #997

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

lorne-maxim
Copy link
Contributor

Add PinMux Tool Support

Description

Tools like a PinMux Configuration tool need a single entry point or function for configuring all the necessary pins. The current paradigm in the peripheral drivers is to initialize any pins associated with a particular peripheral at the time the peripheral's initialization function is called. This could override any settings previously set by the PinMux tool. This PR addresses this problem in the following way:

  • Add a global variable to allow locking the pin cofiguration state (Alternate function selection, input/output mode, pull strength/direction selection, voltage level selection, and drive strength). Default state of this variable is UNLOCKED.
  • Add MXC_GPIO_Set/GetConfigLock() functions to read and modify this variable.
  • Add code to the MXC_GPIO_Config function that checks the global variable to see if it should actually perform the configuration. If the configuration is LOCKED, the function simply returns E_NO_ERROR.
  • Implement a weak function (called PinInit) that is called as part of SystemInit just prior to Board_Init that does nothing. The PinMux tool can then output code that overrides this function. The new code would initialize all pins to their appropriate settings and call MXC_GPIO_SetConfigLock(LOCKED) prior to exiting.

Advantages

  • Backwards compatible with any existing customer code. Anyone that hasn’t used the PinMux tool’s code will notice no difference.
  • Only requires changing the GPIO_Config and SystemInit functions. No need to touch the Init function of all the peripherals. (Although, we should update the function documentation of each to describe which parameters, e.g. mapping, are ignored when the GPIO configuration is locked).
  • Code still has the option of reconfiguring pins after the PinMux code is run. They just need to unlock the gpio config before attempting to do so. This doesn’t happen frequently, but there are times when customers need to switch between two configurations.

Disadvantages

  • Some function parameters in the periph’s init functions will be ignored. This could look a bit clunky to users.

@github-actions github-actions bot added MAX32520 Related to the MAX32520 (ES17) MAX32570 Related to the MAX32570 (ME13) MAX32650 Related to the MAX32650 (ME10) MAX32655 Related to the MAX32655 (ME17) MAX32660 Related to the MAX32660 (ME11) MAX32665 Related to the MAX32665 (ME14) MAX32670 Related to the MAX32670 (ME15) MAX32672 Related to the MAX32672 (ME21) MAX32675 Related to the MAX32675 (ME16) MAX32680 Related to the MAX32680 (ME20) MAX32690 Related to the MAX32690 (ME18) MAX78000 Related to the MAX78000 (AI85) MAX78002 Related to the MAX78002 (AI87) MAX32662 Related to the MAX32662 (ME12) MAX32572 Related to the MAX32572 (ME55) labels Apr 19, 2024
@lorne-maxim
Copy link
Contributor Author

/clang-format-run

@@ -109,6 +109,11 @@ int MXC_GPIO_Config(const mxc_gpio_cfg_t *cfg)

MXC_GPIO_Init(1 << port);

if (MXC_GPIO_GetConfigLock() == MXC_GPIO_CONFIG_LOCKED) {
// Configuration is locked. Ignore any attempts to change it.
return E_NO_ERROR;
Copy link
Contributor

Choose a reason for hiding this comment

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

@lorne-maxim what do you think about returning E_BAD_STATE instead of no error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had considered it, but the problem is our PeriphInit functions check the return value of this function. So someone that has used the PinMux tool will have their configuration locked. They will then have no way of calling the PeriphInit function that won't return an error.

@lorne-maxim lorne-maxim merged commit 6cb136f into main Apr 19, 2024
1 check passed
@lorne-maxim lorne-maxim deleted the feat/pin_mux_support branch April 19, 2024 20:49
@perkinsmg
Copy link
Contributor

Is PinInit the correct name? We're hoping to extend the configuration tools to do clock configuration, and then later we'll do peripheral initialization. I think the ideal would be to have all this done by a single entry-point hook (and break down into functions underneath in the generated code) in which case a more generic name will be required. We were using "soc_init" until now, but you might have other preferences, especially if you want to use CamelCase.

@Jake-Carter
Copy link
Contributor

Is PinInit the correct name? We're hoping to extend the configuration tools to do clock configuration, and then later we'll do peripheral initialization. I think the ideal would be to have all this done by a single entry-point hook (and break down into functions underneath in the generated code) in which case a more generic name will be required. We were using "soc_init" until now, but you might have other preferences, especially if you want to use CamelCase.

@perkinsmg As we extend the functionality of the pin config tool we can continue to add calls into SystemInit, which is our single entry-point available for all micros. See here for example. The function is weak, so even if you wanted to have the pin config tool auto-generate a completely new SystemInit you can override the existing one.

@lorne-maxim
Copy link
Contributor Author

The current call stack on reset is:

PreInit()        /* weak function */
SystemInit()     /* weak function */
    PalSysInit() /* weak function - for parts that have a BLE radio. */
    PinInit()    /* weak function - could arguably be a part of Board_Init, adding it to SystemInit was easier so I took the lazy way. */
    Board_Init() /* weak function */
main()

Therefore, I agree with @Jake-Carter here. We can add additional functions to this list as needed. We could also replace PinInit with a more generic function like @perkinsmg suggests, but I'm struggling to find an appropriate name. soc_init, feels wrong since we already have SystemInit. The name should suggest this is where all the configuration that was selected by an external tool is actually applied: ApplyConfigurationSelections()? Doesn't feel quite right, but along the lines of what I'm looking for.

Jake-Carter pushed a commit that referenced this pull request Apr 29, 2024
EricB-ADI pushed a commit that referenced this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MAX32520 Related to the MAX32520 (ES17) MAX32570 Related to the MAX32570 (ME13) MAX32572 Related to the MAX32572 (ME55) MAX32650 Related to the MAX32650 (ME10) MAX32655 Related to the MAX32655 (ME17) MAX32660 Related to the MAX32660 (ME11) MAX32662 Related to the MAX32662 (ME12) MAX32665 Related to the MAX32665 (ME14) MAX32670 Related to the MAX32670 (ME15) MAX32672 Related to the MAX32672 (ME21) MAX32675 Related to the MAX32675 (ME16) MAX32680 Related to the MAX32680 (ME20) MAX32690 Related to the MAX32690 (ME18) MAX78000 Related to the MAX78000 (AI85) MAX78002 Related to the MAX78002 (AI87)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants