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

plat-hikey: Make DRAM1_BASE configurable and replace deprecated function #6672

Merged
merged 2 commits into from
Feb 19, 2024

Conversation

zwb-233
Copy link
Contributor

@zwb-233 zwb-233 commented Feb 6, 2024

This pull request aims to enhance the plat-hikey platform configuration by introducing the CFG_DRAM1_BASE configuration switch in platform_config.h. This change enables the configuration of DRAM1_BASE and ensures platform consistency.Additionally, deprecated register_dynamic_shm() calls are replaced with register_ddr(). These modifications align with previous discussions and address issues #5146, #4768, and #6160.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

For all commits, please use your real name instead of zwb233 in S-o-b tag as per developer certificate of origin requirements.

Suggestion for commit message of "plat-hikey: Update platform_config.h to use CFG_DRAM1_BASE":

 plat-hikey: Update platform_config.h to use CFG_DRAM1_BASE
 
-Add conditional compilation directives to lines 110-114 of
-tee4/optee_os/core/arch/arm/plat-hikey/platform_config.h
-to utilize the CFG_DRAM1_BASE configuration switch for defining DRAM1_BASE.
-This ensures that the platform configuration is consistent with the newly introduced
-configuration switch.
+Use CFG_DRAM1_BASE in core/arch/arm/plat-hikey/platform_config.h.
 
 Signed-off-by: ...

Wrap also the long lines the commit message of the last commit of the series (72 chars max)

@@ -107,7 +107,11 @@
#define DRAM0_BASE 0x00000000
#define DRAM0_SIZE 0x3F000000
#define DRAM0_SIZE_NSEC 0x3E000000
#define DRAM1_BASE 0x40000000
#ifdef CFG_DRAM1_BASE
Copy link
Contributor

Choose a reason for hiding this comment

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

As per previous commit in this series CFG_DRAM1_BASE is always defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestions and review. I have made the necessary changes to the commits based on your feedback, including updating the commit names and formats, and removing unnecessary #ifdef directives.

@zwb-233 zwb-233 force-pushed the master branch 2 times, most recently from 96f972a to 56ede31 Compare February 7, 2024 03:05
@jenswi-linaro
Copy link
Contributor

Please fix the checkpatch issues.

@zwb-233 zwb-233 force-pushed the master branch 2 times, most recently from aa7220a to ab1b6af Compare February 8, 2024 03:28
@zwb-233
Copy link
Contributor Author

zwb-233 commented Feb 8, 2024

Please fix the checkpatch issues.

I have fixed the issues, thank you.

@jenswi-linaro
Copy link
Contributor

Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

Commit "plat-hikey: make DRAM1_BASE configurable":
Remove the following sentence from the commit message.

This change aligns with previous discussions and resolves issues

I think commits "plat-hikey: make DRAM1_BASE configurable" and
"plat-hikey: Update platform_config.h to use CFG_DRAM1_BASE" should be squashed into a single commit.

Commit "plat-hikey: Replace register_dynamic_shm() with register_ddr()":
The commit message doees not explain why the change is made. I would replace it with:

-The register_ddr() function is used to define RAM areas where the
-non-secure world can allocate dynamic shared memory.
-This memory can be utilized with zero-copy memory buffers passed
-as memref in TA invocations.
+Use register_ddr() instead of register_dynamic_shm() that is deprecated.

Acked-by: Etienne Carriere <etienne.carriere@foss.st.com> once addressed.

This commit introduces the CFG_DRAM1_BASE configuration switch
in the plat-hikey platform.

Signed-off-by: Wen Bin <a1231512a@163.com>
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Use register_ddr() instead of register_dynamic_shm() that is
deprecated.

Signed-off-by: Wen Bin <a1231512a@163.com>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
@zwb-233
Copy link
Contributor Author

zwb-233 commented Feb 10, 2024

I think commits "plat-hikey: make DRAM1_BASE configurable" and
"plat-hikey: Update platform_config.h to use CFG_DRAM1_BASE" should be squashed into a single commit.

I have merged the two commits and made the other changes you mentioned. Thank you.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

look all good to me

@jforissier jforissier merged commit 57ad009 into OP-TEE:master Feb 19, 2024
7 checks passed
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.

4 participants