Converting them to macros certainly reduces the amount of code that
needs to change.
Thanks for the link Nathan, I will take a look at that.
John
On 3/25/20 11:45 AM, Nathan Hartman wrote:
On Wed, Mar 25, 2020 at 11:38 AM Xiang Xiao <xiaoxiang781...@gmail.com>
wrote:
On Wed, Mar 25, 2020 at 11:33 PM John Rippetoe
<jrippe...@roboticresearch.com> wrote:
1. For each chip, files that include mpu support need to add a
conditional to two included files
#ifdef CONFIG_ARM_MPU
# include "mpu.h"
# include "<chip>_mpuinit.h"
#endif
Wouldn't it make more sense to put #ifdef CONFIG_ARM_MPU inside of
mpu.h and <chip>_mpuinit.h. That way, the problem is fixed in one
place for all time.
I hadn't considered that, but that does make more sense.
Another option would be to remove the inline qualifier and move the
functions into a .c file.
I'm equally fine with this, but I can see the appeal for wanting to keep
the small functions inlined. Of the 12 funcitons in mpu.h, 9 are just
wrappers around a call to mpu_configure_region that look like
static inline void mpu_priv_intsram(uintptr_t base, size_t size)
{
/* The configure the region */
mpu_configure_region(base, size,
MPU_RASR_TEX_SO | /* Ordered */
MPU_RASR_C | /* Cacheable */
/* Not Bufferable */
MPU_RASR_S | /* Shareable */
MPU_RASR_AP_RWNO /* P:RW U:None */
/* Instruction access */);
}
Like others have said, there is no guarantee these will be inlined.
Furthermore, the build system doesn't currently enforce a minimum C99
standard to ensure the inline keyword is even available. So I am in
favor of moving the functions to a .c file in the name of clarity.
Consolidating them into up_mpu.c seems like a good choice. If the
overhead of an additional function call is problematic, we could
predefine the flags and call mpu_configure_region() directly
#define MPU_PRIV_INTSRAM MPU_RASR_TEX_SO | /* Ordered */
MPU_RASR_C | /* Cacheable */
/* Not Bufferable */
MPU_RASR_S | /* Shareable */
MPU_RASR_AP_RWNO /* P:RW U:None */
/* Instruction access */);
mpu_configure_region(base, size, MPU_PRIV_INTSRAM);
Or change mpu_priv_intsram to macro?
That's a valid approach. Macros are C89 compliant, guaranteed to be inlined
no matter what, result in 0 code if unused, and don't cause problems in
header files.
Reminder: When implementing a function-like macro, always write it as do {
... } while (0) -- see
https://stackoverflow.com/a/257425
Cheers,
Nathan
CONFIDENTIALITY NOTICE: This communication may contain private, confidential
and privileged material for the sole use of the intended recipient. If you are
not the intended recipient, please delete this e-mail and any attachments
permanently.
