ARM-software / tf-issues

Issue tracking for the ARM Trusted Firmware project
37 stars 16 forks source link

question: #include <arm_def.h> not required in arm_bl1_setup.c and causing issue for non-ARM platform. #321

Closed Mathieu-Brcm closed 9 years ago

Mathieu-Brcm commented 9 years ago

for non-ARM platform that want to be aligned with ARM and re-use the palt/arm/common/arm_bl1_setup.c then arm_def.h include needs to be removed. this would allow non-ARM platform to use arm_bl1_setup.c with different SRAM/DRAM base address for example.

In fact, BL1 coompile fine without this include for JUNO board so this include is not required (it seems).

Thanks, -Mathieu

Mathieu-Brcm commented 9 years ago

Same comment for BL2 and BL31. (In process to get agreement for pull requests to facilitate this change proposal).

danh-arm commented 9 years ago

arm_bl1_setup.c uses more than just the SRAM/DRAM base addresses from arm_def.h so removing this include is not an option. I'm sure that this gets indirectly included even if you remove the explicit include from arm_bl1_setup.c.

I guess you are saying you have an "almost" ARM standard platform. Potentially we could add some flexibility by splitting arm_def.h into a) ARM standard platform stuff and b) common ARM Ltd development platform stuff, however that split might be somewhat subjective. For example, platforms that conform to this ARM white paper will all have DRAM in the same place, but it's difficult to know how many platforms actually conform to this.

I think any such split is only really feasible if the non-ARM Ltd standard platform(s) are available in the upstream repository (or at least public).

I'm glad to hear that you are in the process of signing the CLA!

Mathieu-Brcm commented 9 years ago

"arm_bl1_setup.c uses more than just the SRAM/DRAM base addresses from arm_def.h" correct but like you said it gets included by other files that are ARM platform specific somewhere else. This way the arm_bl1_setup.c can be re-used by non-ARM platform that are almost like ARM platform.

For example, so far, the only difference for my BL1 is the addition of some UART clock init in bl1_early_plafrom_setup. The rest, uses arm_bl1_setup.c. This removes the need for code duplication and makes it easier for SoC vendor in long term when the ATF will evolve. If not then the SoC vendor will endup having to copy/past the arm_blx_setup.c files into his SoC directory and will have to maintain that code as APIs/features/drivers evolve...

danh-arm commented 9 years ago

I understand very well the advantages of having as much common code as possible, and we would like to support your use case. The problem is knowing what code really is common platform code. For your platform, the only difference might be the UART init code, but there might be more or less differences in other SoC vendors' platforms. We don't really have a hope of satisfying all possible platforms out there, which is why I'd prefer to limit the scope of this to upstream platforms.

Mathieu-Brcm commented 9 years ago

Thanks Danh, I understand. -Mathieu

danh-arm commented 9 years ago

I'm going to close this. If a future upstream platform requires further splitting of the plat/arm/common code for improved code re-use, this should be raised as a separate issue with more concrete requirements.