All,
I can pitch in where needed. Glad to see that finally we are seeing some movement to refine ARMPlatformPkg into maintainable code.
Just my 2 cents below....... Not a big fan of pre-compiler conditional statements as they get messy over the course of time as more and more subsystems/platforms get created over time and it will be hard to maintain eventually. I experienced this first hand while working at Qualcomm, so many chipsets with their weird Quirks that had to be accommodated by so many pre-compiler conditionals. Finally over time, it was difficult to figure out which pre-compiler conditional was for which chipset/feature.
Not a short term goal but long term goal, I think we should go with run time conditional rather than compiler time conditional. We should create a generic abstract class/structure and have some kind of virtual/polymorphic function/behavior capability in C. http://www.embedded.com/electronics-blogs/programming-pointers/4391967/Virtu...
so a new platform port should just be adding .c and .h file for that particular platform with a lot of get and set functions. The generic files should be able to get the data it needs for that particular platform (run time). This is similar to abstract factory design pattern.
Thanks, Supreeth
-----Original Message----- From: James King Sent: Tuesday, December 22, 2015 3:18 AM To: Leif Lindholm; ryan.harkin@linaro.org Cc: Ard Biesheuvel; Supreeth Venkatesh; Linaro UEFI Mailman List Subject: RE: Refactoring ARM Platform support
I agree that it is best to just start doing the gradual patch migration and see what work chunks we can ask others to help with. I think the message here is for everyone to try and be helpful in providing prompt feedback and looking for ways to help.
This is however also a decision that could be affected by your amount of free time. If this was put higher up on your priority list, would you still consider it problematic?
There is no escape from the normal monthly release cycle from the ARM-LT, but after that this work is already the highest priority task that ARM is asking Ryan to do.
Thanks, James
-----Original Message----- From: Leif Lindholm [mailto:leif.lindholm@linaro.org] Sent: 21 December 2015 17:13 To: ryan.harkin@linaro.org Cc: Ard Biesheuvel; Supreeth Venkatesh; Linaro UEFI Mailman List; James King Subject: Re: Refactoring ARM Platform support
On Mon, Dec 21, 2015 at 02:55:18PM +0000, Ryan Harkin wrote:
I am starting on the task of refactoring ARM Platform support. Initially in the ARM Ltd sense, but also in the general ARM-based SoC case.
\o/
Right, seems like you've taken on a bit more than I was expecting, including a few things I had been expecting to have to do myself. We should be able to share this workload somewhat.
The existing platform code, mostly under ArmPlatformPkg is fragmented, duplicated and hard to maintain. Adding a new platform often involves copy/paste of a lot of code that the platform owner neither understands nor really needs to care about.
The basic concept is to have as much common/generic code as possible and have as little in the platform specific code. Thus making porting to a new platform as simple as can be.
A new board using code that’s already in the tree should just be able to provide a .dsc file with some defines in it and include the common.dsc.inc file.
Absolutely.
The common.dsc.inc file would then define everything, with conditional statements including various drivers or features.
For example, juno.dsc would define that it is using the CFI NOR flash driver and provide the defines for the flash config. The common.dsc.inc file would only include the CFI NOR flash driver in the build if it is specified in its parent .dsc file.
The other concept is to move the platform code from the EDK2 tree to the OpenPlatformPkg tree.
In my experiments, I have included OpenPlatformPkg as a sub-tree of the EDK2 git tree. This allows me to “git mv” files between the two repos. In reality, we would have to “git add” to the OpenPlatformPkg repo and “git rm” from EDK2 as separate patches.
Yeah, I'm afraid so. And it _is_ worth mentioning here that there is finally some movement with regards to changing upstream processes and components - so anything moved onto OpenPlatformPkg now _may_ need to transition again. _But_, all of the actual hard work will be in this initial process - changing of paths in configuration files at a later date can be done with a coccinelle script.
Failed attempts
I had several plays with the code.
In one attempt, I moved all the code in ArmPlatformPkg that was used by Juno and attempted to get this working in the new location. Unfortunately, in my opinion, the build system is so obtuse that this becomes a tangled web of build failures. I aborted this attempt very quickly.
In another attempt, I thought I would start from scratch by creating a new BSP. But I realised that my understanding of much of the code/config was severely lacking. There are some components and sections of the DSC and FDF files where I have absolutely no idea what they are for. I guess many people are in this position and it’s probably a reason why so many BSPs are boilerplate copied from ArmPlatformPkg.
Yeah. And actually, there are a lot of ARM-specific things in ArmPkg.dec/ArmPlatformPkg.dec that just shouldn't be there. That exist only to support code doing things differently on ARM.
Getting started
I decided to start by splitting the Juno BSP up into common and platform specific code. The first step is to create a new juno.dsc file in OpenPlatformPkg.
Initially, this just #included ArmJuno.dsc from edk2.
Next, I copied ArmJuno.dsc over to OpenPlatformPkg/Platforms/common/common.dsc.inc and included that in the new juno.dsc.
Then, I migrated Juno specific stuff from common.dsc.inc to juno.dsc, eg. “PLATFORM_NAME = ArmJuno”.
From here on, I started walking through the common.dsc.inc file and moving code across from ArmPlatformPkg to OpenPlatformPkg/Platforms/common. Bit by bit, I moved, renamed and fixed up components used by the Juno platform.
At this point, the “common” code is still very Juno specific. Function names, symbols and various other pieces still reference Juno. This needs to be fixed.
One part (of the many) that may cause controversy is that I have kept to a “simple” file naming scheme. Instead of everything having the words Arm, Juno, Platform and Pkg in multiple combinations, I’ve tried to keep things simple.
The main issue are with things that may have a global scope. As long as those can be kept unique, it won't upset me much.
I also used all lower case filenames, because we’re generally not DOS users and all these mixed case filenames drive me insane. I expect this to go down badly with some, but hope I can get away with it ;-)
Sorry, don't think you can get away with it. As much as I hate camel case, it is the explicitly required naming standard for EDK2, where we are hoping to get the end result adopted.
Once Juno is migrated to common
After all the parts that Juno uses have been moved to OpenPlatformPkg, I intend to add another platform. I’ll use FVP: it’s similar enough to Juno that most of the common code will remain there, even if it is still quite ARM platform specific, but it will allow me to demonstrate two platforms using the common code.
That makes a lot of sense.
What it won’t do yet is split out any functionality that is “Versatile Express” in nature, but neither Juno nor FVP specific; i.e. any code that is not really common to all platforms, but is common to ARM Ltd's platforms.
Yeah, but that sort of flushes itself out as you add more platforms.
After this, I was thinking that first I need to go around cleaning up the code to remove all the Juno references. Then, adding another platform, such as HiKey or D01/02 into the common framework might be a brave next step to see how much we can keep common and how much needs to be split back into Juno, FVP or Versatile Express specific areas.
For now, D02 is looking like the best option to work with.
After that, I would be quite tempted to include one of the Minnowboards, to help strip out ARM-specific assumptions.
My GIT branch
I’ve created a demo branch with my experimental changes here:
https://git.linaro.org/landing-teams/working/arm/edk2.git/shortlog/ref s/heads/openplat
This is only intended to demonstrate a concept, so don’t take too much of it to heart. I haven’t gone all the way in migrating the code either, I’ve just picked the essentials, followed by the NOR flash and ACPI parts. I picked those because NOR is common and ACPI is Juno specific.
Thanks, I'll have a look.
So what next??
In order to push this code to OpenPlatformPkg, I think there are two ways of doing it:
Big hit: go away, do all the work in isolation and submit a patch series.
Gradual migration: submit small patch series as they are ready and remove the code from ArmPlatformPkg as we move along.
I’m a bit worried about doing it in one big hit. Looking at the amount of free time I have to work on this and how long my experiments have taken me, I worry that by the time the work is “done”, it will need some serious fixing again to catch up to the upstream code changes. I also think it leaves my changes less open to scrutiny by others.
This is however also a decision that could be affected by your amount of free time. If this was put higher up on your priority list, would you still consider it problematic?
Unfortunately, a gradual migration would possibly take a longer elapsed time to complete. I don’t think it would be realistic to create the new code in OpenPlatformPkg without removing (some of?) the code from ArmPlatformPkg as we go. If the migrated code is still in ArmPlatformPkg, people will continue to use it and patch it, leaving two diverging code bases that will need effort to sync. At the very least, removing the ArmJuno.dsc from ArmPlatformPkg would help keep things on track without removing the rest of the code in ArmPlatformPkg.
Regardless of the above, if you start working on a gradual migration, that is a task where it would be easier for others to pitch in and help out.
But indeed, it is already a pain for me to manually synchronize edk2/OpenPlatformPkg for the platforms that are not fully migrated.
At this point, I’m soliciting feedback on my approach and hoping to get some sane critique on what can be done better/easier.
To be brutally honest, I think just getting on with it will be better than not getting on with it :)
We can always change course later if it starts looking like a trainwreck.
/ Leif
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.