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

soc: atmel: sam: samx7x: fold atmel_samx7x_config() into early init hook #84142

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

henrikbrixandersen
Copy link
Member

Fold atmel_samx7x_config() into soc_early_init_hook(), which is the only caller of this function.

Fold atmel_samx7x_config() into soc_early_init_hook(), which is the only
caller of this function.

Signed-off-by: Henrik Brix Andersen <[email protected]>
Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

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

Hi @henrikbrixandersen ,

This is not part of existential part of boot process to be mixed inside soc.c.
Why do you want to do this? What we gain doing this?

@henrikbrixandersen
Copy link
Member Author

This is not part of existential part of boot process to be mixed inside soc.c.

Not sure what you mean here? The code currently in soc_config.c is only called from within soc.c. This construct stems from commit 51c771e which switched the SAM E70/V71 from SYS_INIT() to soc_early_init_hook().

Why do you want to do this? What we gain doing this?

Simplification. Reduced overhead, both compile-unit-wise and function-call wise.

@nandojve
Copy link
Member

This is not part of existential part of boot process to be mixed inside soc.c.

Not sure what you mean here? The code currently in soc_config.c is only called from within soc.c. This construct stems from commit 51c771e which switched the SAM E70/V71 from SYS_INIT() to soc_early_init_hook().

The function configure some pins but it should be part of pinctrl, not part of soc init. If you want remove the soc_config.c unit then move as a static function. Let the compiler do the job : )

@henrikbrixandersen
Copy link
Member Author

henrikbrixandersen commented Jan 17, 2025

The function configure some pins but it should be part of pinctrl, not part of soc init. If you want remove the soc_config.c unit then move as a static function. Let the compiler do the job : )

This is a refactoring of existing code, simply moving the function body of one function (atmel_samx7x_config()) into the only caller of that function (soc_early_init_hook()), which doesn't do much else than call the first function. Not sure where pinctrl fits into this small refactoring?

@nandojve
Copy link
Member

The function configure some pins but it should be part of pinctrl, not part of soc init. If you want remove the soc_config.c unit then move as a static function. Let the compiler do the job : )

This is a refactoring of existing code, simply moving the function body of one function (atmel_samx7x_config()) into the only caller of that function (soc_early_init_hook()), which doesn't do much else than call the first function. Not sure where pinctrl fits into this small refactoring?

The refactor into pinctrl is not so easy. For me keeping the function is better. As far I know, the compiler will suppress the call when the function is defined as static inline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Board/SoC configuration platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants