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.

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

hikey960: Initial commit v2 #1684

Merged
merged 4 commits into from
Jul 18, 2017
Merged

hikey960: Initial commit v2 #1684

merged 4 commits into from
Jul 18, 2017

Conversation

vchong
Copy link
Contributor

@vchong vchong commented Jul 12, 2017

Replaces #1681.

Please ignore first commit which is #1683.

Thanks!

@vchong vchong mentioned this pull request Jul 12, 2017
@vchong
Copy link
Contributor Author

vchong commented Jul 13, 2017

Comments addressed.

#define PL011_UART6_BASE 0xFFF32000
#if defined(CFG_CONSOLE_UART) && (CFG_CONSOLE_UART == 5)
#define CONSOLE_UART_BASE PL011_UART5_BASE
#elif !defined(CFG_CONSOLE_UART) || (CFG_CONSOLE_UART == 6)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can (and should) be #elif defined(CFG_CONSOLE_UART) && (CFG_CONSOLE_UART == 6) since CFG_CONSOLE_UART now has a default value set in conf.mk. The problem with the current code is that it will silently ignore any value different from 5 and 6 and use 6 in any case. Please fix line 41 (hikey-hikey) also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks! In that case, can we simplify this to just #elif CFG_CONSOLE_UART == 6 and etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only thing with the simplification is that if CFG_CONSOLE_UART is not defined, then it will default to uart0 for hikey instead of printing an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, !defined(CFG_CONSOLE_UART) is useless if we make sure it is always defined obviously.

@jforissier
Copy link
Contributor

Thanks @vchong, just one more comment, once it is addressed:
Reviewed-by: Jerome Forissier <[email protected]>

Ah, yes, one more question, maybe for @etienne-lms. Since Hikey960 has both A53 and A73 cores, is it OK to have -mcpu=cortex-a53 (set in file cortex-armv8-0.mk)?

Signed-off-by: Victor Chong <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
@vchong
Copy link
Contributor Author

vchong commented Jul 17, 2017

Rebased, squashed and tags applied. Ready for merge. Thanks!

@jforissier You're welcome! Please make sure you're ok with the CONSOLE_UART_BASE section one last time before merging. Thanks!

Signed-off-by: Victor Chong <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
@vchong
Copy link
Contributor Author

vchong commented Jul 17, 2017

Sorry, please hold. One of the commit message needs to be changed.

@vchong
Copy link
Contributor Author

vchong commented Jul 17, 2017

Commit message fixed. Thanks!

Victor Chong added 2 commits July 17, 2017 18:48
The HiKey 960 development platform is based around the Huawei Kirin 960
octa-core ARM big.LITTLE processor with four ARM Cortex-A73 and four
Cortex-A53 cores with 3GB of LPDDR4 SDRAM memory, 32GB of UFS 2.0 flash
storage, and the latest generation Mali G71 MP8 graphics processor.

See https://www.96boards.org/product/hikey960 for more details.

Signed-off-by: Victor Chong <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Signed-off-by: Victor Chong <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
@jforissier jforissier merged commit 3ff350a into OP-TEE:master Jul 18, 2017
@vchong
Copy link
Contributor Author

vchong commented Jul 19, 2017

Thanks!

@vchong vchong deleted the hikey960v2 branch November 24, 2017 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants