[meta-intel] [PATCH 0/1] [PATCH] meta-valleyisland: new BSP layer for Intel Valley Island platform

[Darren Hart]

On Tue, 2014-01-28 at 19:03 +0800, rebecca.swee.fun.chang at intel.com
wrote:
> From: "Chang, Rebecca Swee Fun" <rebecca.swee.fun.chang at intel.com>
> 
> Hi all,
> 

Hi Rebecca,

We'll start the review with this cover letter - and get into more detail
in the patch itself.

> This pull request is the first phase submission for meta-valleyisland.
> As the kernel patches for valley island are still in the progress of
> review, I have only submitted the recipes without recipes-kernel.

OK - but this will not be buildable as is since there isn't a
virtual/kernel provider for the MACHINE, right? We can review, but it
needs to build before it can go in.

> This BSP was proven to be a working BSP as we have released it
> through the Amazon host site. The functionality of the BSP was
> sanity tested. The plan for the second phase submission
> which includes recipes-kernel and BSP README file will be send out once
> all the kernel patches had been pulled into linux-yocto-3.8. The
> estimated time for the next submission will be in work week 6/7.
>
> This pull request is meant to be pulled into meta-intel/master branch
> and meta-intel/dylan branch.

It's a bit unconventional to add a BSP to the 1.4 release, skip the 1.5
release, and add it to the 1.6 (master) cycle. I'd suggest adding it to
1.5 (dora) as well. The kernel need not be updated, but the BSP should
be supported across the releases.

In general, this is good to have out for review, but we should pull it
in as a single patch *series* after the kernel bits are in. Note the
*series*, not just a single patch. I'll elaborate more below...

More below in the diffstat...

> 
> Thanks all.
> 
> Rebecca
> 
> The following changes since commit 8f426abb4ecb08116a84a73eae005c24411dc680:
> 
>   sys940x: Set PACKAGE_ARCH to MACHINE_ARCH explicitly (2014-01-27 16:56:15 -0800)
> 
> are available in the git repository at:
> 
>   git://git.yoctoproject.org/meta-intel-contrib rebeccas/meta-valleyisland-dylan
>   http://git.yoctoproject.org/cgit.cgi/meta-intel-contrib/log/?h=rebeccas/meta-valleyisland-dylan
> 
> Chang, Rebecca Swee Fun (1):
>   meta-valleyisland: new BSP layer for Intel Valley Island platform
> 
>  meta-isg/MAINTAINERS                               |    4 +
>  meta-isg/meta-valleyisland/COPYING.MIT             |   17 +
>  meta-isg/meta-valleyisland/README.sources          |   18 +
>  meta-isg/meta-valleyisland/conf/layer.conf         |   14 +
>  .../conf/machine/valleyisland32.conf               |   23 +
>  .../conf/machine/valleyisland64.conf               |   25 +
>  .../formfactor/valleyisland32/machconfig           |    3 +
>  .../formfactor/valleyisland64/machconfig           |    3 +

It's a minor thing, but for consistency valleyisland-32 and
valleyisland-64 would be preferred. It will help keep consistent naming
throughout the repository.

>  .../recipes-bsp/formfactor/formfactor_0.0.bbappend |    3 +
>  .../syslinux/files/cross-build-4.04.patch          |   65 ++

This is odd...

>  .../libinstaller-Avoid-using-linux-ext2_fs.h.patch |  975 ++++++++++++++++++++
>  .../recipes-devtools/syslinux/files/no-strip.patch |   31 +
>  .../recipes-devtools/syslinux/syslinux_4.04.bb     |   65 ++

You are adding patches to syslinux in two places, and you adding a minor
syslinux version to the repository. What is the reason for adding
another version of syslinux? We have a later one in poky.

If this is absolutely needed we should first talk to HPA as he needs to
know and probably would have a solution. Second, this recipe would need
to be added as a separate patch and justified. Really, we should not
need this. It complicates the build and fragments the support of the
Intel machines.


-- 
Darren Hart
Yocto Project - Linux Kernel
Intel Open Source Technology Center