Hi Eddie (and the rest of the Rockchip team),

First let me reiterate how happy I am to see that Rockchip is interested in
supporting Yocto/OE builds, this is great! I hope this trend continues.

Second, thank you so much for your patience while I reviewed these patches. I
don't like my review to be only visual, I want to see the code running on my
device(s), so reviewing is going to take a bit of time. In this case I hadn't
flashed my firefly board in a long time, and the flashing procedure was quite
a bit different now from what I last remember.

For future reference, these patches have the string "PATH" in the subject line
instead of "PATCH". Also, if you're creating a v2 or a v3 etc please put a
space between "PATCH" and "v2". But I won't ask you for a "PATCH v3", I'll
just fix these things up as I apply the patches (I hope you don't mind).

You've put "morty" in the subject lines which to me means you're hoping these
patches will be applied against the morty branch of meta-rockchip. Morty was
released in October of 2016 which, at this point, is almost 5 months ago. At
this point patches should only go against morty to fix critical issues. These
patches look more like they're adding functionality, so I'll apply them to
master instead. The next release is Pyro which is expected around April 2017.

On Tue 2017-02-14 @ 01:54:20 PM, Eddie Cai wrote:
> This patch set add main line kernel support for meta-rockchip
> 
> Eddie Cai (6):
>   machine: Use cortexa17hf-neon-vfpv4 as default tune for rk3288.inc

As I've mentioned before, Yocto/OE is a _distribution_ builder, not just an
image builder. As such most people in the community consider DEFAULTTUNES to
be a DISTRO-level policy. In other words, this is not something that should be
set by the BSP. One distro might want to use softfloat, while another wants
hard-float. It will be easier for people who make distributions using Yocto/OE
to use this layer when these types of things are not set at this level. Users
are free to use these distributions (by adding their layers) or they can set a
DEFAULTTUNE in their conf/local.conf file.

I'll add a note to the README pointing this out so people can be aware of this
configuration tweak, but I won't take this patch into the BSP. I've been able
to build and run images where the DEFAULTTUNE is not set and they run just
fine. Another patch (which I'll be happy to create) will need to look for this
tune in recipes that provide things like mali support.

>   machine: Use SOC specific assignements

The above patch is fine, but I am going to adjust the synopsis line to include
the machine to which this patch refers. I.e. instead of:

        machine: Use SOC specific assignments

I'll say:

        rk3288: Use SOC specific assignments

>   machine: separate rk3188 inc file from rk3066

The above patch is okay, but I'm going to remove the DEFAULTTUNE line.

>   machine: Use SOC specific assignements

Adjusted to specify machine (rk3188).

>   machine: Use SOC specific assignements

Adjusted to specify machine (rk3066).

>   recipes-kernel: linux: Add mainline kernel

If Romain is happy with this patch I will apply it.

Best regards,
        Trevor
-- 
_______________________________________________
yocto mailing list
yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/yocto

Reply via email to