Comment 6 for bug 889179

Revision history for this message
Mattias Backman (mabac) wrote : Re: [Bug 889179] [NEW] Add vexpress_a9 platform to linaro-android-media-create

On Tue, Nov 15, 2011 at 8:50 AM, Tixy (Jon Medhurst)
<email address hidden> wrote:
> On Mon, 2011-11-14 at 15:41 +0000, Mattias Backman wrote:
>> On Mon, Nov 14, 2011 at 8:27 AM, Tixy (Jon Medhurst)
>> <email address hidden> wrote:
>> > On Fri, 2011-11-11 at 17:09 +0000, Dave Martin wrote:
>> >> >   Add vexpress_a9 platform to linaro-android-media-create
>> >>
>> >> Should the name "vexpress" be used instead, for consistency with
>> >> linaro-media-create?  Or does l-a-m-c use an independent set of board
>> >> names?
>> >>
>> >> Other platforms based on vexpress would need to be more fully qualified
>> >> in any case.
>> >
>> > For linaro-media-create we can specify all of the required board
>> > parameters in the hardware pack so the tool doesn't need a command-line
>> > board parameter to distinguish between vexpress flavours.
>> >
>> > linaro-android-media-create however needs everything hardcoded into the
>> > tools, so we need to distinguish between vexpress core-tiles to pass the
>> > correct memory addresses to U-Boot.
>>
>> Still, I agree that we should keep the naming consistent with the
>> names in l-m-c for code maintenance reasons and for not confusing the
>> end user with different names for the same board. If it's important to
>> change the name of the BoardConfig and --dev option we should also
>> make the same change in l-m-c. The --dev option is left in l-m-c since
>> it will be needed when there is support for multiple boards in one
>> hwpack.
>
> OK, how about if create a sub-class of VexpressConfig in l-m-c called
> VexpressA9Config and add a 'vexpress-a9' entry for it in board_configs.
> My AndroidVexpressA9Config class for l-a-m-c  could then derive from
> this?

That sounds good. Having a Vexpress superclass will be good since I
gather from the earlier comments that there may be other incarnations
of the Vexpress. So please go ahead with that change.

>
>> > -    boot_script = None
>> > +    boot_script = 'boot.scr'
>>
>> This change should go in the AndroidVexpressConfig so it does not
>> break support for existing Vexpress hwpacks. That line is still in
>> boards.py only to maintain support for old hwpacks. and will not take
>> effect for new hwpacks anyway.
>>
>
> It shouldn't break anything by adding this as the VExpress U-Boot
> currently doesn't read boot.scr - so the file will just get ignored.

Good, the I won't worry about that.

>
> I was adding this because I plan on fixing U-Boot very soon so it does
> read boot.scr, and when I do, both Android and V1 hardware packs will
> need to create this. I can add this as a separate patch in a week or so.

If we need it, we might as well change it now. I was hoping that we
wouldn't create any more hwpacks using the V1 hwpack format though. If
we use the V2 hwpack format, l-m-c has no need to know about boot
script paths etc.

>
>> Would you push your branch to launchpad and create a merge proposal so
>> we can keep track of this change?
>
> So I should create a Bazaar branch with my changes in then? Will this
> need to be on Launchpad, e.g. under https://code.launchpad.net/~tixy?

Yes please, then we get the complete change history and can comment
away while you push new revisions if needed. When we're happy with it,
we simply merge the branch into trunk and we're done. And yes, the
branch goes in your name space under the linaro-image-tools project,
like so:
       bzr push lp:~tixy/linaro-image-tools/vexpress-a9-support

> How do I create a 'merge proposal'? (As you may have gathered, I've
> never done this sort of thing before ;-)

Not a problem. :) There's a pretty good tutorial that actually uses
linaro-image-tools as an example here:
    https://wiki.linaro.org/Resources/HowTo/BZR

If you're used to git, there are a few bullets about the differences
between bzr and git on the bottom of the page.

Basically, you just push your branch to launchpad as above and then
navigate to it in the launchpad web ui. There you find the link
labelled "Propose for merging" and trunk of l-i-t should be default as
the target. Enter a short description about the change and you're
done. While there are options to change, you don't need any of them
for this change.

Then we will get an email about the proposed change and can go ahead
with the code review.

Just let me know if you run into any problems and I'll do my best to help out!

Cheers,

Mattias