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

[sw/lib] move register and bit definitions to according module header files #542

Merged
merged 8 commits into from
Mar 11, 2023
Merged

Conversation

emb4fun
Copy link
Collaborator

@emb4fun emb4fun commented Mar 8, 2023

The idea behind this is to also define the information of a device in its header file.

Using the GPIO as an example, I moved the definition of the structure from neorv32.h
to neorv32_gpio.h. Only the define for NEORV32_GPIO_BASE remains in the neorv32.h file.

Later you can then summarize all the BASE addresses again in the neorv32.h file.

What do you think of this proposal?

@stnolting stnolting marked this pull request as draft March 8, 2023 20:14
@stnolting stnolting added enhancement New feature or request SW software-related coding-style Related to the HW/SW coding style labels Mar 8, 2023
@stnolting
Copy link
Owner

I think I like this idea!
The neorv32.h has become quite a monster... Your proposal has the potential to make things cleaner: all HAL-related things for one module can be found in a single header file. 👍

@andkae
Copy link

andkae commented Mar 8, 2023

In my opinion looks really good!. The Hardware adresses are in the neorv32.h header and the rest of the sw layer is in the driver itself.

Question how to deal with Bit definition in other modules?

@stnolting
Copy link
Owner

Question how to deal with Bit definition in other modules?

What do you mean exactly?
The register bits are defined using simple C enumerations. So we could move them to the according *.h file as well.

@andkae
Copy link

andkae commented Mar 8, 2023

What do you mean exactly?

Yes exactaly the enums, but for sure they can also move to the *.h files.

@stnolting
Copy link
Owner

@emb4fun looks good so far! Thank you very much!

The list of base addresses might cause some issues with Doxygen. But that is not critical. I can fix that after merging this.

In your Pr we have three new files: for the system information memory (SYSINFO), the debug module (DM) and the bus monitor (BUSKEEPER). Having a new file for SYSINFO is just fine. Same for the BUSKEEPER. But what about the DM?

The debug module cannot be accessed by "normal" software running on the core. All the register and bit definition were just listed for "completeness". So I am not sure what we should do with this file?

By the way, I will modify your PR and add the new files to the software sections file list. 😉

@stnolting stnolting changed the title GPIO: Better grouping of information [sw] move register and bit definitions to according module header files Mar 10, 2023
@emb4fun
Copy link
Collaborator Author

emb4fun commented Mar 11, 2023

The DM file was listed for completeness too. Because all structure definitions of a "device" was moved to its own file.

@emb4fun emb4fun marked this pull request as ready for review March 11, 2023 08:03
@stnolting
Copy link
Owner

The question is do we really want to keep this file. 🤔

Anyway, I'll have a look at your modifications. 👍

@stnolting stnolting changed the title [sw] move register and bit definitions to according module header files [sw/lib] move register and bit definitions to according module header files Mar 11, 2023
@stnolting
Copy link
Owner

Seems ready. @emb4fun what do you think?

@emb4fun
Copy link
Collaborator Author

emb4fun commented Mar 11, 2023

Yes it is ready. @stnolting please merge.

@stnolting stnolting merged commit 2a9089e into stnolting:main Mar 11, 2023
@emb4fun emb4fun deleted the gpio_group branch March 11, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coding-style Related to the HW/SW coding style enhancement New feature or request SW software-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants