Support lt version of mx51evk hwpack along with non-lt version

[Eric Miao]

The landing team version of mx51evk hwpack has to include the packaged kernel based on Freescale's BSP, as well as other packages. Due to the kernel flavor name difference (linaro-mx51 vs linaro-lt-mx5), linaro-image-tools is currently unable to support both hwpacks.

Lool suggested a preferred list of kernels:

<lool> ericm|ubuntu: Here's the rationale: currently we use a static filename or regexp to find the kernel to install, if we find more than one linaro-image-tools break (that's a reported bug I think); if you have multiple kernels installed, the behavior is also not specified
<lool> ericm|ubuntu: What I would propose is that we keep list of preferred kernels and process them in order
<lool> for instance, "omap4, omap" would cause omap4 to be tried first, then omap to be tried
 we would use comparable lists in linaro-image-tools and flash-kernel
 For your LT case, the Mx51BoardConfig would say "kernel_flavors = lt-mx51, mx51, mx5" or something like that

[Loïc Minier (lool)]

Would also fix bug #645536

[Mattias Backman (mabac)]

Hi,

I made a small change to _get_file_matching to have it take a prefix and a suffixes string as parameters. The intention is to avoid breaking anything, current configs should still work. Now it's possible to specify a comma separated list of regexps in kernel_suffix or just leave a single regexp as it is before the change.

See lp:~mabac/linaro-image-tools/bug-744857

If this is a good way to solve this, I could adapt code outside of this small change, like adding a kernel_flavors config parameter.

Thanks,

Mattias

[Eric Miao (eric.y.miao)]

@Mattias, hi - same idea. I'm fine with your approach. Please let me know when that gets merged.

[James Westby (james-w)]

Hi Mattias,

Thanks for looking at this.

The direction of the change looks good to me, but I'm a little unsure about the specifics.

It's not clear to me why you added the prefix parameter. It seems obscure to me, and if
code wants to have a common prefix that they don't release, they can do that in code
somehow. Maybe I'm missing something here though.

The other thing that's not clear to me is why to prefer comma-separated strings, rather
than a list of strings. The latter is more obvious, and doesn't have a problem if someone
wants a comma in the regex.

As I said, I may be missing something, so please tell me if so.

I suggest that you propose your changes for merging as I'm sure we can come to a
resolution quickly for this change.

Thanks,

James

[Mattias Backman (mabac)]

Hi James,

It seems like both Eric and I got the same idea, but there's no real reason behind doing it exactly like this.

The prefix parameter came from '%s/vmlinuz-*-' (and '%s/initrd.img-*-') being concatenated with all the regexps. I chose between letting _get_file_matching handle the concatenation or having make_uImage and make_uInitrd do it before calling that function. I could just as well supply _get_file_matching with a list of pre-concatenated regexps and that probably makes more sense when reading the code.

About the comma separated string. Well, I just got stuck on lool's suggestion in the description and kind of thought that the board configs are plain text config files even when reading the code. Of course it should be a list of strings.

I'll push an update in the morning and propose a merge.

Thanks,

Mattias

[James Westby (james-w)]

On Tue, 29 Mar 2011 19:33:39 -0000, Mattias Backman <email address hidden> wrote:
> Hi James,
>
> It seems like both Eric and I got the same idea, but there's no real
> reason behind doing it exactly like this.
>
> The prefix parameter came from '%s/vmlinuz-*-' (and '%s/initrd.img-*-')
> being concatenated with all the regexps. I chose between letting
> _get_file_matching handle the concatenation or having make_uImage and
> make_uInitrd do it before calling that function. I could just as well
> supply _get_file_matching with a list of pre-concatenated regexps and
> that probably makes more sense when reading the code.

Ah ok. That I don't mind too much then, I assumed they were coming
straight from the BoardConfig classes.

I'm happy with which ever way you think is better.

> I'll push an update in the morning and propose a merge.

Great.

Thanks,

James

[Loïc Minier (lool)]

So both implementations are on the right path, but please consider implementing by replacing
kernel_suffix = 'linaro-omap'

with:
kernel_suffixes = ('linaro-omap4', 'linaro-omap')

(Now that I look at the implementation, it's painfully clear that it wont be enough to kill bug #645536 as multiple versions of the same flavor might be installed; so let's just keep that other one open.)

[Eric Miao (eric.y.miao)]

Loic, wouldn't split the string into list be better than explicitly specifying the list, using python syntax. And eventually we are going to encode this into hwpack v2 config file, and a string would be more straight forward?

[Mattias Backman (mabac)]

Hi,

I just realized that this discussion is still ongoing, but went ahead and proposed the merge https://code.launchpad.net/~mabac/linaro-image-tools/bug-744857/+merge/55492.

Thanks,

Mattias

[Loïc Minier (lool)]

On Wed, Mar 30, 2011, Eric Miao wrote:
> Loic, wouldn't split the string into list be better than explicitly
> specifying the list, using python syntax. And eventually we are going to
> encode this into hwpack v2 config file, and a string would be more
> straight forward?

 I don't think we want a list in hwpack; instead the hwpack will contain
 a single kernel and that will be the one flavor we're expected to use;
 the hwpack creation might support finding the right kernel from
 multiple locations though.

 I personally find it's best to use the closest Python structure for
 code, so that you use a list to contain a list of things; we can
 convert the string representation from the hwpack when we read/write
 hwpacks. Perhaps we will infer kernel flavors from multiple hwpacks
 for instance, and then build a list from that. So I wouldn't worry
 too much about the way it's serialized in data files outside of
 linaro-image-tools.

--
Loïc Minier