Skip to content

layerscape: align layerscape source code to layerscape lsdk#1221

Closed
yangbolu1991 wants to merge 5 commits intolede-project:masterfrom
yangbolu1991:devel-rebase
Closed

layerscape: align layerscape source code to layerscape lsdk#1221
yangbolu1991 wants to merge 5 commits intolede-project:masterfrom
yangbolu1991:devel-rebase

Conversation

@yangbolu1991
Copy link
Contributor

@yangbolu1991 yangbolu1991 commented Jul 17, 2017

This patch-set is to update LEDE layerscape source code with NXP Layerscape LSDK.
https://lsdk.github.io
https://github.com/qoriq-open-source

Attached verification logs for current 5 layerscape platforms.

layerscape-log.zip

@yangbolu1991
Copy link
Contributor Author

Any comments on this patch-set?
:)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the source URL, it should refer to a public one that does not require a github account.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nbd168 The public u-boot images git tree for 32b ARMv8 platforms for distros is in plan, but it needs several months.
Could we use this one temporarily? I will update it with public one once it's set up.

Thanks a lot :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is:
https://github.com/yangbolu1991/u-boot-lede.git instead of git@github.com:yangbolu1991/u-boot-lede.git.
The first one allows public download, the second one requires a ssh key for github access.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... Sorry for my misunderstanding. I will update the link in next version.

$(INSTALL_DIR) $(STAGING_DIR_IMAGE)
$(CP) $(PKG_BUILD_DIR)/ls1043ardb-uboot.bin $(STAGING_DIR_IMAGE)/ls1043ardb-armv8_32b-uboot.bin
$(CP) $(PKG_BUILD_DIR)/ls1046ardb-uboot.bin $(STAGING_DIR_IMAGE)/ls1046ardb-armv8_32b-uboot.bin
$(CP) $(PKG_BUILD_DIR)/ls1012ardb-uboot.bin $(STAGING_DIR_IMAGE)/ls1012ardb-armv8_32b-uboot.bin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you build these binaries from source code inside of LEDE?
For GPL compliance you anyway have to provide the source code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always used 64b u-boot image to support both 32b and 64b kernel and rootfs. It's convenient.
So this uboot-layerscape-armv8_32b here used same image/code with uboot-layerscape-armv8_64b.
The reason we used image is that LEDE could only provide 32b toolchain for 32b target.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggestion here? Or should I add a note to explain in Makefile?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is ok with me

PKG_SOURCE_VERSION:=c275e91392e2adab1ed22f3867b8269ca3c54014

PKG_BUILD_DIR=$(BUILD_DIR)/$(PKG_NAME)-$(PKG_VERSION)-$(BUILD_VARIANT)/$(PKG_NAME)-$(PKG_VERSION)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The License tag is wrong:
PKG_LICENSE:=GPL-2.0 GPL-2.0+This is not the GPL: https://github.com/qoriq-open-source/fm-ucode/blob/integration/Freescale-Binary-EULA

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right... These're binaries developed by Freescale/NXP.
Let me remove the error PKG_LICENSE.

May I know whether PKG_LICENSE is required in Makefile? Could I just ignore it.
fman-ucode is neither GPL or BSD, it's just Freescale-Binary-EULA.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok to me now

PKG_SOURCE_VERSION:=6eed8fdfc07920178b70fba199fa20fb42adb20b

PKG_BUILD_DIR=$(BUILD_DIR)/$(PKG_NAME)-$(PKG_VERSION)-$(BUILD_VARIANT)/$(PKG_NAME)-$(PKG_VERSION)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package is licensed under some BSD license and not GPL:
https://github.com/qoriq-open-source/rcw/blob/integration/LICENSE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will correct it in next version. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove the "define Build/Compile" block completely, this is more or less what "define Build/Compile/Default" does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will remove this in next version.


ARCH:=arm
BOARDNAME:=ARMv8 32-bit based boards
CPU_TYPE:=cortex-a9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What type of CPUs are supported by this target?
Is it a cortex A9 or some other CPU like a cortex A53?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Layerscape CPU is cortex A53 or cortex A72 now, but we want it to run on 32-bit mode which is compatible with ARMv7.
So we set CPU_TYPE as cortex-a9 to select 32b toolchain.

Copy link
Contributor

@chunkeey chunkeey Jul 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has actually come up before. And there was a promise to fix this:
#864 (comment)
"@chunkeey Thanks a lot for pointing out this. I will have a check on our platforms. If the CPU_TYPE is not correct for some platforms, we need a plan to fix that. I will let you know the result. Thanks :)"

you are kind of screwing the users and yourself out of neon v2 and vfpv4.
Can you at least explain why you want to stick with "specifically" A9? From what I know the A7 or A15 would be a better fit. Not to mention that you'll have to fix the BOARDNAME, because it does contain the "ARMv8" name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chunkeey
Sorry for my late response. I just come back to office today from holiday.
I didn't insist A9 ... In current layerscape platforms, some are with A53 cpus and some are with A72 cpus. More details,
http://www.nxp.com/products/microcontrollers-and-processors/arm-processors/qoriq-layerscape-arm-processors:QORIQ-ARM

I don't know which CPU_FLAGS is proper for all of them and doesn't introduce any potential issue, but at least platforms worked fine with current A9.
You suggested to use A7 or A15, which one exactly should I use in your view?
Should I also add CPU_SUBTYPE:=neon-vfpv4 ?
I really would like to fix it with proper value.

Regarding to BOARDNAME, I think it seems proper because they are indeed ARMv8 32-bit.

@hauke @nbd168 @fsl-jyt
Do you have any suggestion on CPU_TYPE?
Thanks a lot :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the layerscape 32 biot target is not so popular I would suggest choosing something which is already used by some other target, see here: https://downloads.lede-project.org/snapshots/packages/

I would suggest this:
CPU_TYPE:=cortex-a15
CPU_SUBTYPE:=neon-vfpv4
This is probably a better fit than the A9

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for @chunkeey @hauke suggestion!
It's ok for me. I will fix this in next version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run "make kernel_oldconfig CONFIG_TARGET=subtarget" when this subtarget is selected to reduce the size of this configuration file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for letting me know this method.
No problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run "make kernel_oldconfig CONFIG_TARGET=subtarget" when this subtarget is selected to reduce the size of this configuration file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for letting me know this method.
No problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the CFLAGS setting really needed? LEDE should use similar compiler options by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried to compile without that. There was no problem. Let me remove it in next version. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yangbolu1991 ,
Please double check the libnl-tiny-0.1
Without that CFLAGS, it will arise compile error, please have a look:
#344

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of compile error? I don't see an error log anywhere in there...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fsl-jyt Thanks for providing the information.
I just double checked the compiling(make distclean and make, for NXP target) and no problem. Maybe the old issue had been fixed.

So I think there should be no problem to remove it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes please remove this

# See /LICENSE for more information.
#

ARCH:=aarch64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the SoC supported by this target all have an cortex A53 CPU, please add:
CPU_TYPE:=cortex-a53

Copy link
Contributor Author

@yangbolu1991 yangbolu1991 Jul 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, ls1012a/ls1088a/ls1043a is cortex A53 cpu while ls1046a/ls2088a is cortex A72 cpu.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok with this, then you can share the packages with the armvirt target.

@@ -0,0 +1,114 @@
From 7ece66e296972744ec471d521b8ca07990e2494f Mon Sep 17 00:00:00 2001
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit and probably many more are already in kernel 4.9, could you please update this target to kernel 4.9.
The next LEDE release will be based on kernel 4.9 anyway so you have to update this target when the layerscape target should be included in the next release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NXP LSDK will release kernel v4.9 on September 2017. We will update our targets to v4.9 in LEDE on September.
But currently customers still use v4.4, this patch-set is still needed.

May I know whether LEDE will still maintain v4.4 after v4.9 is released?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like all these patches being added you are ending up with 835 patches on top of kernel 4.4. Please do the migration to kernel 4.9 then you can probably get rid of many patches.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hauke ,
Almost all these patches are for layerscape IP modules. We are also working on kernel 4.9 now. Even we migrate to kernel 4.9, I'm afraid the number of patches would not shrunk so much.

The large number patches mainly comes from dts (155 patches), mc-bus (139 patches), sec (138 patches), qspi (80 patches). What I can do is to merge each IP module patches into one. We can provide only one patch for each module/component. Then we will only have dozens of patches finally.

It's also easy for maintaining, becasue we have each module as a feature branch when backport patches.

Do you think is it ok? Let me generate a new version for your reveiwing.
Thank you very much.

@yangbolu1991
Copy link
Contributor Author

@hauke @nbd168 @fsl-jyt
I updated the patch-set with below changes to address your comments.
Please help to review. Thanks a lot :)

layerscape: take over maintainership
Changes for v2:
- None

layerscape: rename subtargets and update makefile files
Changes for v2:
- None

layerscape: update packages with LSDK git trees
Changes for v2:
- Fixed u-boot-layerscape-armv8_32b url with public url.
- Added note to explain why used u-boot images for 32b targets, and removed wrong license info.
- Removed wrong license info of fman_ucode.
- Corrected rcw license info with BSD.
- Removed useless Build/Compile definition for rcw.

layerscape: update memory layout according to LSDK
Changes for v2:
- None

layerscape: update linux to LSDK linux
Changes for v2:
- Reduced size of kernel config for 32b/64b targets.
- Removed useless CFLAGS for 64b targets.

@yangbolu1991
Copy link
Contributor Author

Any comments?
Thanks a lot.

@yangbolu1991
Copy link
Contributor Author

Just rebased this patch-set.

@yangbolu1991
Copy link
Contributor Author

Could we consider to merge this pull request if there is no other change request?
This patch-set is mainly to update all packages code and kernel patches to Layerscape SDK.
Regarding to CPU_TYPE discussion, we can still continue or start a email thread after this PR was closed, and I can send a seperate patch for that if need.

Thanks a lot :)

Copy link
Member

@hauke hauke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The layerscape target now has 835 kernel patches without this pull request it had 249 patches. I am unable to review them and many of them are backports from the upstream kernel. I am unsure if we want to add so many patches into LEDE for a kernel 4.4 target.


ARCH:=arm
BOARDNAME:=ARMv8 32-bit based boards
CPU_TYPE:=cortex-a9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the layerscape 32 biot target is not so popular I would suggest choosing something which is already used by some other target, see here: https://downloads.lede-project.org/snapshots/packages/

I would suggest this:
CPU_TYPE:=cortex-a15
CPU_SUBTYPE:=neon-vfpv4
This is probably a better fit than the A9

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes please remove this

# See /LICENSE for more information.
#

ARCH:=aarch64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok with this, then you can share the packages with the armvirt target.

$(INSTALL_DIR) $(STAGING_DIR_IMAGE)
$(CP) $(PKG_BUILD_DIR)/ls1043ardb-uboot.bin $(STAGING_DIR_IMAGE)/ls1043ardb-armv8_32b-uboot.bin
$(CP) $(PKG_BUILD_DIR)/ls1046ardb-uboot.bin $(STAGING_DIR_IMAGE)/ls1046ardb-armv8_32b-uboot.bin
$(CP) $(PKG_BUILD_DIR)/ls1012ardb-uboot.bin $(STAGING_DIR_IMAGE)/ls1012ardb-armv8_32b-uboot.bin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is ok with me

PKG_SOURCE_VERSION:=c275e91392e2adab1ed22f3867b8269ca3c54014

PKG_BUILD_DIR=$(BUILD_DIR)/$(PKG_NAME)-$(PKG_VERSION)-$(BUILD_VARIANT)/$(PKG_NAME)-$(PKG_VERSION)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok to me now

@yangbolu1991
Copy link
Contributor Author

I have updated the patch-set with v3 version.
See below changes for v3,

layerscape: take over maintainership
Changes for v3:

  • None

layerscape: rename subtargets and update makefile files
Changes for v3:

  • None

layerscape: update packages with LSDK git trees
Changes for v3:

  • None

layerscape: update memory layout according to LSDK
Changes for v3:

  • None

layerscape: update linux to LSDK linux
Changes for v3:

  • fixed CPU_TYPE issue for 32-bit.
  • merged each IP module kernel patches into one.

@yangbolu1991
Copy link
Contributor Author

@hauke
v3 patch-set fixed the CPU_TPYE problem, and merge each IP module patches into one. Now there are only 36 patches.
Do you think is it ok?

@yangbolu1991
Copy link
Contributor Author

Hi @hauke ,
Sorry for disturbing. Did you have a chance to have a look at this?
Thanks.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Rename subtargets 32b/64b with armv8_32b/armv8_64b which are
more proper, and update makefile files. There also will be other
subtargets added in the future, like armv7.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
NXP Layerscape LSDK had set up its own open source web site
and github for release.

https://lsdk.github.io
https://github.com/qoriq-open-source

This patch is to update rcw/fman_ucode/u-boot packages with LSDK
git trees. Also add some patches of packages to support LEDE.
Since ARMv8 32-bit u-boot images are same with ARMv8 64-bit images
but 64-bit toolchain couldn't be used for 32-bit targets, we still
use a private tree for ARMv8 32-bit u-boot images. This is in plan
to move this private tree to NXP Layerscape github.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
The uniform memory layout of NOR/QSPI/NAND/SD media on all Layerscape platforms:
+-----------------------------+---------|--------------|-----------------+
|Firmwre Definition           | MaxSize | Flash Offset | SD Start Block #|
|-----------------------------|---------|--------------|-----------------|
|RCW+PBI                      | 1MB     | 0x00000000   | 0x00008         |
|-----------------------------|---------|--------------|-----------------|
|Boot firmware(U-Boot,UEFI)   | 2MB     | 0x00100000   | 0x00800         |
|-----------------------------|---------|--------------|-----------------|
|Boot firmware Environment    | 1MB     | 0x00300000   | 0x01800         |
|-----------------------------|---------|--------------|-----------------|
|PPA firmware                 | 2MB     | 0x00400000   | 0x02000         |
|-----------------------------|---------|--------------|-----------------|
|Secure boot headers          | 3MB     | 0x00600000   | 0x03000         |
|-----------------------------|---------|--------------|-----------------|
|DPAA1 FMAN ucode             | 256KB   | 0x00900000   | 0x04800         |
|-----------------------------|---------|--------------|-----------------|
|QE/uQE firmware              | 256KB   | 0x00940000   | 0x04A00         |
|-----------------------------|---------|--------------|-----------------|
|Ethernet PHY firmware        | 256KB   | 0x00980000   | 0x04C00         |
|-----------------------------|---------|--------------|-----------------|
|Scripts                      | 256KB   | 0x009C0000   | 0x04E00         |
|-----------------------------|---------|--------------|-----------------|
|DPAA2 MC firmware            | 3MB     | 0x00A00000   | 0x05000         |
|-----------------------------|---------|--------------|-----------------|
|DPAA2 DPL                    | 1MB     | 0x00D00000   | 0x06800         |
|-----------------------------|---------|--------------|-----------------|
|DPAA2 DPC                    | 1MB     | 0x00E00000   | 0x07000         |
|-----------------------------|---------|--------------|-----------------|
|Device tree(needed by uefi)  | 1MB     | 0x00F00000   | 0x07800         |
|-------------+---------------|---------|--------------|-----------------|
|Kernel       |               | 16MB    | 0x01000000   | 0x08000         |
|-------------| kernel.itb    |---------|--------------|-----------------|
|Ramdisk rfs  |               |32MB     | 0x01100000   | 0x08800         |
+-------------+---------------+---------|--------------|-----------------+

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
@yangbolu1991
Copy link
Contributor Author

@hauke @chunkeey @nbd168 @fsl-jyt
Hi all,

Rebased this patch-set again since Hauke's below patch introduced conflicts. Actually this changes had already in this patch-set as we discussed.
a5d26d7 layerscape: do not add custom CFLAGS.

Could we speed up the reviewing? I think all current comments had been addressed, right?
Thanks a lot.

@yangbolu1991
Copy link
Contributor Author

I splited patch "layerscape: update linux to LSDK linux" into 5 patches.
I think it will be easier for you to review.

See #1327

@yangbolu1991 yangbolu1991 deleted the devel-rebase branch October 10, 2017 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Comments