Dear Gabriel Huau,
In message 1405204264-10922-1-git-send-email-contact@huau-gabriel.fr you wrote:
This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ. For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.
Has this patch really to be specific for the quad core version? Can we not also support the dual core version in the same way?
...
+int cpu_reset(int nr) +{
- uint32_t reg;
- struct src *src = (struct src *)SRC_BASE_ADDR;
- reg = __raw_readl(&src->scr);
- switch (nr) {
- case 1:
reg |= SRC_SCR_CORE_1_RESET_MASK;break;- case 2:
reg |= SRC_SCR_CORE_2_RESET_MASK;break;- case 3:
reg |= SRC_SCR_CORE_3_RESET_MASK;break;- }
I feel this should not be hardwired for 4 cores, and I also think we should avoid using such a switch statement here. All you need is an index into an array.
+int cpu_status(int nr) +{
- uint32_t reg;
- struct src *src = (struct src *)SRC_BASE_ADDR;
- reg = __raw_readl(&src->scr);
- switch (nr) {
- case 1:
printf("core 1: %d\n", !!(reg & SRC_SCR_CORE_1_ENABLE_MASK));break;- case 2:
printf("core 2: %d\n", !!(reg & SRC_SCR_CORE_2_ENABLE_MASK));break;- case 3:
printf("core 3: %d\n", !!(reg & SRC_SCR_CORE_3_ENABLE_MASK));break;- }
Ditto. Such code duplication does not scale. Please rework to avoid the switch.
- switch (nr) {
- case 1:
__raw_writel(boot_addr, &src->gpr3);reg |= SRC_SCR_CORE_1_ENABLE_MASK;break;- case 2:
__raw_writel(boot_addr, &src->gpr5);reg |= SRC_SCR_CORE_2_ENABLE_MASK;break;- case 3:
__raw_writel(boot_addr, &src->gpr7);reg |= SRC_SCR_CORE_3_ENABLE_MASK;break;- }
- /* CPU N is ready to start */
- __raw_writel(reg, &src->scr);
Ditto here.
And can you please explain why you are using __raw_writel() here?
- reg = __raw_readl(&src->scr);
- switch (nr) {
- case 1:
reg &= ~SRC_SCR_CORE_1_ENABLE_MASK;break;- case 2:
reg &= ~SRC_SCR_CORE_2_ENABLE_MASK;break;- case 3:
reg &= ~SRC_SCR_CORE_3_ENABLE_MASK;break;- }
- /* Disable the CPU N */
- __raw_writel(reg, &src->scr);
Again, please avoid the switch.
We have read-modify-write macros which you could use, unless you really have to use the __raw_*() accessors. Why is this needed?
Best regards,
Wolfgang Denk