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

arm: imx: add i.MX7D support #1639

Merged

jforissier merged 1 commit into OP-TEE:master from MrVan:imx7d

Jun 28, 2017

Contributor

MrVan commented Jun 27, 2017

Add i.MX7D support.

Add register definition
Add gpcv2 to powerup and powerdown cpu
Introduce soc runtime detection
Add PSCI cpu/off/affinity

The scripts to build 7dsdb image.
make PLATFORM=imx-mx7dsabresd ARCH=arm CFG_PAGEABLE_ADDR=0
CFG_NS_ENTRY_ADDR=0x80800000 CFG_DT=y DEBUG=y CFG_TEE_CORE_LOG_LEVEL=4
CFG_PSCI_ARM32=y CFG_MX7=y CFG_DDR_SIZE=0x40000000
CFG_TZC380=y
./mkimage -A arm -O linux -C none -a 0xbdffffe4 -e 0xbe000000
-d out/arm-plat-imx/core/tee.bin uTee-7d

Signed-off-by: Peng Fan <peng.fan@nxp.com>

Contributor

jforissier commented Jun 27, 2017 •

edited

Loading

The scripts to build 7dsdb image.
make PLATFORM=imx-mx7dsabresd ARCH=arm CFG_PAGEABLE_ADDR=0
CFG_NS_ENTRY_ADDR=0x80800000 CFG_DT=y DEBUG=y CFG_TEE_CORE_LOG_LEVEL=4
CFG_PSCI_ARM32=y CFG_MX7=y CFG_DDR_SIZE=0x40000000
CFG_TZC380=y

Why not set (most of) the CFG_ properly in plat-imx/conf.mk when PLATFORM=imx-mx7dsabresd?

Edit: To clarify: make PLATFORM=imx-mx7dsabresd should be enough to result in a bootable image.

Contributor Author

MrVan commented Jun 27, 2017

@jforissier yeah, that could simplify the build command. I'll add it in next version after collecting more comments. Hope this patch could catch up 2.5.0 release.

jforissier reviewed

View reviewed changes

core/arch/arm/plat-imx/config/config_imx7.h Outdated

+               * POSSIBILITY OF SUCH DAMAGE.
+               */
+              #ifndef CFG_TEE_CORE_NB_CORE

Contributor

jforissier Jun 27, 2017

#ifndef not needed unless it makes sense to allow override at build time

Contributor Author

MrVan Jun 27, 2017

i.MX7Dual has two cores, but i.MX7Solo only has one core. So allow override at build time.

Contributor

jforissier Jun 27, 2017

OK, I see. IMO this should be addressed via $(PLATFORM) (more specifically, the platform "flavor"), because relying on a proper combination of command line parameters is error-prone, and setting the right values in conf.mk based on $(PLATORM_xyz) is easy enough. This applies to CFG_DDR_SIZE below, too.

core/arch/arm/plat-imx/config/config_imx7.h Outdated

+              #endif
+              #ifndef CFG_DDR_SIZE
+              #error "DRAM0_SIZE not defined"

Contributor

jforissier Jun 27, 2017

I guess that should be #define CFG_DDR_SIZE 0x40000000.
Same as above re. #ifndef, OK if it makes sense to override it from e.g., the command line.

Contributor Author

MrVan Jun 27, 2017

I'll correct the "error" message. I could move this conf.mk. Orignally I want to use one code base to support all i.MX7 boards, using different command line parameter to generate different image for different boards, but still needs some work to do.

core/arch/arm/plat-imx/gpcv2.c

		@@ -0,0 +1,90 @@
		/*

Contributor

jforissier Jun 27, 2017

Any reason not to put this under core/drivers/?

Contributor Author

MrVan Jun 27, 2017

This actually is not a driver, no driver framework. This is just to handle gpc registers and directly provide functions for imx psci code. I prefer keep it in plat-imx.

Contributor

jforissier Jun 27, 2017

OK, fair enough.

core/arch/arm/plat-imx/gpcv2.c Outdated

+              #include <mm/core_memprot.h>
+              #include <platform_config.h>
+              #include <imx.h>
+              #include <stdint.h>

Contributor

jforissier Jun 27, 2017

Several includes are not used

Contributor Author

MrVan Jun 27, 2017

Will clean up.

core/arch/arm/plat-imx/imx-common.c Outdated

+              {
+              	imx_soc_type();
+              	return soc_type == SOC_MX6UL;

Contributor

jforissier Jun 27, 2017

return imx_soc_type() == SOC_MX6UL; and same below.

Contributor Author

MrVan Jun 27, 2017

Fix in next version.

core/arch/arm/plat-imx/imx-common.c Outdated

+              static uint32_t soc_type;
+              static uint32_t soc_rev;
+              static uint32_t soc_rev_major;
+              static uint32_t soc_rev_minor;

Contributor

jforissier Jun 27, 2017

I'd rather cache only the register value and have helper functions like so:

static uint32_t imx_digproc()
{
	static uint32_t reg;

	if (!reg) {
		/* ... read register value ... */
	}
	return reg;
}

uint32_t imx_soc_type()
{
	return (imx_digproc() >> 16) & 0xff;
}

/* Etc. */

Contributor Author

MrVan Jun 27, 2017

I'll check to use your proposed way.

core/arch/arm/plat-imx/imx-common.c Outdated

+              bool soc_is_imx7s(void)
+              {
+              	int val = core_mmu_get_va(OCOTP_BASE + 0x450, MEM_AREA_IO_SEC);

Contributor

jforissier Jun 27, 2017

core_mmu_get_va() returns a vaddr_t

Contributor Author

MrVan Jun 27, 2017

fix in next version

core/arch/arm/plat-imx/imx-common.c Outdated

+              	int val = core_mmu_get_va(OCOTP_BASE + 0x450, MEM_AREA_IO_SEC);
+              	if (soc_is_imx7ds()) {
+              		if (val & 1)

Contributor

jforissier Jun 27, 2017

Missing read32() or something?

Contributor Author

MrVan Jun 27, 2017

oh. yeah. Thanks. Fix in next version.

core/arch/arm/plat-imx/imx-common.c Outdated

+              }
+              bool soc_is_imx7d(void)
+              {

Contributor

jforissier Jun 27, 2017

Same comments as for soc_is_imx7s()

Contributor Author

MrVan Jun 27, 2017

Fix in next version

MrVan force-pushed the imx7d branch from 64a33c4 to b6cd00c Compare

June 27, 2017 13:43

Contributor Author

MrVan commented Jun 27, 2017

@jforissier Updated with most CFG_xx into conf.mk, following your comments to detect soc type, cleanup including files in gpc code, fix a bug when detecting 7d/s soc as you pointed.

Contributor

jforissier commented Jun 27, 2017

@MrVan thanks, LGTM. Three minor things:

Would you mind updating README.md ("Platforms supported") and MAINTAINERS.md?
.travis.yml should probably have $make PLATFORM=imx-mx7dsabresd
(Very minor!) In the commit message, if you put a backslash then it's easier to copy/paste the build commands without missing one line (and it's easier to understand too):

...
The scripts to build 7dsdb image:
  make PLATFORM=imx-mx7dsabresd
  ./mkimage -A arm -O linux -C none -a 0xbdffffe4 -e 0xbe000000 \
      -d out/arm-plat-imx/core/tee.bin uTee-7d

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>

MrVan force-pushed the imx7d branch from b6cd00c to fcc4ad5 Compare

June 28, 2017 01:31

Contributor Author

MrVan commented Jun 28, 2017

@jforissier Updated. README.md, MAINTAINERS.md updated, .travis.yml updated, tags applied, I did not add backslash in commit log.

jforissier reviewed

View reviewed changes

README.md Outdated

               | [FSL i.MX6 Quad SABRE Lite Board](https://boundarydevices.com/product/sabre-lite-imx6-sbc/) |`PLATFORM=imx`| Yes |
               | [FSL i.MX6 Quad SABRE SD Board](http://www.nxp.com/products/software-and-tools/hardware-development-tools/sabre-development-system/sabre-board-for-smart-devices-based-on-the-i.mx-6quad-applications-processors:RD-IMX6Q-SABRE) |`PLATFORM=imx`| Yes |
               | [FSL i.MX6 UltraLite EVK Board](http://www.freescale.com/products/arm-processors/i.mx-applications-processors-based-on-arm-cores/i.mx-6-processors/i.mx6qp/i.mx6ultralite-evaluation-kit:MCIMX6UL-EVK) |`PLATFORM=imx`| Yes |
+              | [NXP i.MX7Dual SabreSD Board](http://www.nxp.com/products/software-and-tools/hardware-development-tools/sabre-development-system/sabre-board-for-smart-devices-based-on-the-i.mx-7dual-applications-processors:MCIMX7SABRE) |`PLATFORM=imx`| Yes |

Contributor

jforissier Jun 28, 2017

PLATFORM=imx-mx7dsabresd (other i.MX6 platforms are incomplete but that should be fixed in a separate patch, I'll take care of that).

Contributor Author

MrVan Jun 28, 2017

@jforissier ok. I'll resend this patch to use PLATFROM=imx-mx7dsabresd.

Contributor

jenswi-linaro commented Jun 28, 2017

It could be worth mentioning how far the soc runtime detection reaches. For instance if it's possible to support both i.MX6 and i.MX7 based socs with the same image, etc.

Contributor Author

MrVan commented Jun 28, 2017

@jenswi-linaro Currently still need build different images for different platforms. Finally I would like to support the i.MX family in one image, or support i.MX6 using one image, support i.MX7 using one image. Now we do not have a good framework to get information from dtb.

MrVan force-pushed the imx7d branch from fcc4ad5 to f39bf7c Compare

June 28, 2017 07:49

Contributor

jenswi-linaro commented Jun 28, 2017

Please elaborate on "Introduce soc runtime detection" in the commit message. What can it be used for?

Contributor Author

MrVan commented Jun 28, 2017

@jenswi-linaro How about adding the following:
"Introduce soc runtime detection, the final goal is to support i.MX family using one image, but still far from it. Now using the runtime detection apis, we could remove the CFG_MX[6,7][x] to simplify the code, such as in imx psci cpu on/off using one function to support 6Q/7D without CFG_X.
"
The main block issue for me moving formward to use one image for i.MX family is how to register TEE memory runtime. I could not move TEE to low memory, because armv7 kernel has some restriction, I have to put TEE at high memory, but different boards may have different dram size. So If we could register TEE memory runtime with memory info from dts, the biggest blocking issue resolved:)

Contributor

jenswi-linaro commented Jun 28, 2017

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


          arm: imx: add i.MX7D support

539acfe

Add i.MX7D support.
 - Add register definition
 - Add gpcv2 to powerup and powerdown cpu
 - Introduce soc runtime detection, the final goal is to support i.MX
   family using one image, but still far from it. Now using the runtime
   detection, we could remove the CFG_MX[6,7][x] to simplify the code,
   such as in imx psci cpu on/off using one function to support 6Q/7D
   without CFG_[X].
 - Add PSCI cpu/off/affinity

The scripts to build 7dsdb image.
make PLATFORM=imx-mx7dsabresd \
mkimage -A arm -O linux -C none -a 0xbdffffe4 -e 0xbe000000 \
 -d out/arm-plat-imx/core/tee.bin uTee-7d

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

MrVan force-pushed the imx7d branch from f39bf7c to 539acfe Compare

June 28, 2017 08:16

Contributor Author

MrVan commented Jun 28, 2017

Per @jenswi-linaro comments, add more commits log and tags applied

jforissier merged commit ad81714 into OP-TEE:master

Contributor

jforissier commented Jun 28, 2017

Thanks @MrVan!

MrVan deleted the imx7d branch

June 28, 2017 12:32

Contributor

jforissier commented Jul 28, 2017

@WANGAlfred English only, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet