Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

plat-marvell: Add initial support for ARMADA3700 #1946

Merged
merged 1 commit into from
Nov 21, 2017
Merged

plat-marvell: Add initial support for ARMADA3700 #1946

merged 1 commit into from
Nov 21, 2017

Conversation

moswangwen
Copy link
Contributor

Only test 64bit mode with default configuration

  1. Build command
    make PLATFORM=marvell-armada3700 CFG_ARM64_core=y CFG_ARM_GICV3=y
  2. Pass xtest

Signed-off-by: wangwen wangwen@marvell.com

jforissier
jforissier approved these changes Nov 16, 2017
View reviewed changes
Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

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

Hi @moswangwen,

Please see some minor comments below.
Also, I'd like to have Kevin's Acked-by: since you added him as a maintainer. Thanks!

core/arch/arm/plat-marvell/conf.mk Outdated
ifeq ($(PLATFORM_FLAVOR),armada3700)
include core/arch/arm/cpu/cortex-armv8-0.mk
platform-debugger-arm := 1
$(call force,CFG_MVEBU_UART,y)
endif
Copy link
Contributor

Choose a reason for hiding this comment

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

How about enable CFG_ARM64_core and CFG_ARM_GICV3 here? (since your'e saying in the commit log that those are needed).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, that's good.

core/arch/arm/plat-marvell/conf.mk Outdated
@@ -18,7 +25,7 @@ $(call force,CFG_WITH_ARM_TRUSTED_FW,y)

$(call force,CFG_GENERIC_BOOT,y)
$(call force,CFG_GIC,y)
$(call force,CFG_8250_UART,y)

Copy link
Contributor

Choose a reason for hiding this comment

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

Useless empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

core/arch/arm/plat-marvell/sub.mk Outdated

ifeq ($(PLATFORM_FLAVOR_armada3700),y)
srcs-y += armada3700/hal_sec_perf.c
endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Better written as:

src-$(PLATFORM_FLAVOR_armada3700) += armada3700/hal_sec_perf.c

(same for 7k8k above BTW).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

.shippable.yml Outdated
@@ -152,6 +152,7 @@ build:
- _make PLATFORM=rockchip-rk322x CFG_TEE_CORE_LOG_LEVEL=4 DEBUG=1
- _make PLATFORM=sam
- _make PLATFORM=marvell-armada7k8k CFG_ARM64_core=y
- _make PLATFORM=marvell-armada3700 CFG_ARM64_core=y CFG_ARM_GICV3=y
Copy link
Contributor

Choose a reason for hiding this comment

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

_make PLATFORM=marvell-armada3700 should be enough, that is, if you support only one configuration for now, make it the default (see below).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

.travis.yml Outdated
@@ -255,6 +255,9 @@ script:
# Marvell ARMADA 7K 8K
- $make PLATFORM=marvell-armada7k8k CFG_ARM64_core=y

# Marvell ARMADA 3700
- $make PLATFORM=marvell-armada3700 CFG_ARM64_core=y CFG_ARM_GICV3=y
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@jforissier
Copy link
Contributor

One more thing: please review the checkpatch warnings, they shouldn't be hard to fix.

@jforissier
Copy link
Contributor

Thanks!
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

@kevin-peng-hao
Copy link

Looks good to me.

Acked-by: Kevin Peng <kevinp@marvell.com>

@jforissier
Copy link
Contributor

@moswangwen please squash the commits.

Note: you shouldn't add a Acked-by: or similar tag to a commit unless the person has actually posted it publicly to the pull request (which @kevin-peng-hao has just done now, so all is good).

@moswangwen
plat-marvell: Add initial support for ARMADA3700
686e355 
Only test 64bit mode with default configuration

1. Build command
    make PLATFORM=marvell-armada3700
2. Pass xtest

Signed-off-by: wangwen <wangwen@marvell.comi>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Kevin Peng <kevinp@marvell.com>
@jforissier
Copy link
Contributor

Thanks @moswangwen .
FYI, the CI error seems to be caused by an incorrect usage of SHIPPABLE_COMMIT_RANGE in .shippable.yml -- not related to this PR. I'll check that.

@jforissier jforissier merged commit 24bb751 into OP-TEE:master Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jforissier jforissier jforissier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@moswangwen @jforissier @kevin-peng-hao