[PATCH] libmm patches after fixups

Joel Sherrill Joel.Sherrill at OARcorp.com
Fri Aug 30 16:31:58 UTC 2013


I know you know the naming rules and the leading underscore means it is private. So if the bsp or app init is to perform it, it needs a public style name

Gedare Bloom <gedare at gwmail.gwu.edu> wrote:


Does the high level score interface need to include
"_Memory_management_Initialize"? Is MMU/MPU initialization only going
to be invoked in low-level BSP startup code?

The other primary issue I see (other than the hard-coded values in the
tests) is the use of a global SMP lock. I don't know if that is the
right way we want to use locks, or if the lock should be encapsulated
inside of some kind of MM structure (even if that structure is only a
wrapper for the lock).

-Gedare

On Wed, Aug 28, 2013 at 12:11 PM, Hesham Moustafa
<heshamelmatary at gmail.com> wrote:
> Hey all,
>
> Please review the new patches after fixups according
> to your reviews.
>
> Thanks,
> Hesham
>
> On Tue, Aug 27, 2013 at 6:39 PM, Gedare Bloom <gedare at gwmail.gwu.edu> wrote:
>> For 0003-libmm-libcpu-arm-shared.patch, you should not specify the
>> exception handler is inline, since you register a pointer for it. At
>> any rate can you use the ARM's default exception handler (Sebastian
>> pointed it out in another email). Also, you should probably get rid of
>> the arm_cp15_print_fsr.c file or make it part of your test code if you
>> want to keep it for debugging purposes. There are unused variables in
>> _CPU_Memory_management_Initialize(), and a lot of unused variables in
>> _CPU_Memory_management_Set_attributes(). Get rid of unused variables
>> and get rid of unnecessary initializations of variables.
>>
>> In the arm-cp15.h file it looks like you have a spurious paste:
>> +
>> +/** @} */
>> +
>> +/** @} */
>> +
>> I think you only want one of those.
>>
>> 0004-libmm-armbsps-realview-xilinx-raspberry.patch:
>> * mm_config_table.h: The global variables in these 3 header files
>> should not be given the static attribute. It does not make a lot of
>> sense to me for a variable to be static in a header file.
>>
>> The tests in patch 0005 still need some fixes to deal with workspace
>> allocation, and eventually to make them flexible and not use
>> hard-coded addresses.
>>
>> -Gedare
>>
>> On Mon, Aug 26, 2013 at 8:23 PM, Hesham Moustafa
>> <heshamelmatary at gmail.com> wrote:
>>> Hey all,
>>>
>>> I have fixed up some issues according to reviews on my
>>> latest set of patches. Fixups include:
>>>
>>> - Deleting translation table and created a new translation macro.
>>> (According to Sebastian Review) with introducing flags at high-level
>>> for attributes.
>>> - s/dummy_data_abort_excpetion_handler/default_data_abort_excpetion_handler/
>>> and made it call bsp_generic_fatal
>>> - Created new xxx_fsr_print_description C file (According to Gedare review)
>>> - Moved FSR Regiser Defines to arm-cp15.h (According to Gedare review)
>>> - Deleted hard coded values.
>>> - Remove blanks and white spaces.
>>>
>>> Regards,
>>> Hesham
_______________________________________________
rtems-devel mailing list
rtems-devel at rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel




More information about the devel mailing list