[PATCH v7 06/15] ARM: hisi: enable MCPM implementation

[Nicolas Pitre]

[ I somehow missed this email yesterday.  Sorry if I asked the same 
  questions for which you already had provided answers. ]

On Tue, 20 May 2014, Haojian Zhuang wrote:

> On 16 May 2014 04:01, Nicolas Pitre <nicolas.pitre at linaro.org> wrote:
> > On Thu, 15 May 2014, Haojian Zhuang wrote:
> >
> >> On 14 May 2014 03:43, Nicolas Pitre <nicolas.pitre at linaro.org> wrote:
> >> > On Tue, 13 May 2014, Haojian Zhuang wrote:
> >> >
> >> >> +     data = readl_relaxed(fabric + FAB_SF_MODE);
> >> >> +     if (on)
> >> >> +             data |= 1 << cluster;
> >> >> +     else
> >> >> +             data &= ~(1 << cluster);
> >> >> +     writel_relaxed(data, fabric + FAB_SF_MODE);
> >> >> +     while (1) {
> >> >> +             if (data == readl_relaxed(fabric + FAB_SF_MODE))
> >> >> +                     break;
> >> >> +     }
> >> >> +}
> >> >
> >> > The above could be easily coded in assembly for the power_up_setup
> >> > callback thusly:
> >> >
> >> > hip04_power_up_setup:
> >> >
> >> >         cmp     r0, #0                  @ check affinity level
> >> >         bxeq    lr                      @ nothing to do at CPU level
> >> >
> >> >         mrc     p15, 0, r0, c0, c0, 5   @ get MPIDR
> >> >         ubfx    r0, r0, #8, #8          @ extract cluster number
> >> >
> >> >         adr     r1, .LC0
> >> >         ldmia   r1, {r2, r3}
> >> >         sub     r2, r2, r1              @ virt_addr - phys_addr
> >> >         ldr     r1, [r2, r3]            @ get fabric_phys_addr
> >> >         mov     r2, #1
> >> >         ldr     r3, [r1, #FAB_SF_MODE]  @ read "data"
> >> >         orr     r3, r3, r2, lsl r0      @ set cluster bit
> >> >         str     r3, [r1, #FAB_SF_MODE]  @ write it back
> >> >
> >> > 1:      ldr     r2, [r1, #FAB_SF_MODE]  @ read register content
> >> >         cmp     r2, r3                  @ make sure it matches
> >> >         bne     1b                      @ otherwise retry
> >> >
> >> >         bx      lr
> >> >
> >> > :LC0:   .word   .
> >> >         .word   fabric_phys_addr - .LC0
> >> >
> >> > That should be it.
> >> >
> >>
> >> No. These code should be executed before new CPU on. If I transfer
> >> them to assembler code, it means that code will be executed after
> >> new CPU on.
> >
> > Exact.
> >
> >> Then it results me failing to make new CPU online.
> >
> > The assembly code could be wrong as well.  Are you sure this is not the
> > actual reason?
> >
> > Is there some documentation for this stuff?
> >
> 
> There's no problem in assembly code. I even rewrite your assembly code.
> 
> If I keep my c code with assembly code, new CPU could be online right.
> If I only use assembly code, I only get the kernel panic. So it's not
> caused by assembly code. It's caused by executing code after new CPU
> on.

Beware.  The assembly code, when invoked via the MCPM layer during early 
boot of a CPU, is executing with the MMU still disabled.  That means all 
addresses must be physical addresses.  This is where things myght be 
tricky.  And then that code should not work if invoked from C code 
because it then has to deal with virtual addresses. So if you tested the 
assembly code by calling it from C code and it worked then the assembly 
code is wrong.

To be sure please post the code you tested (mine wasn't complete) so we 
could tell you if it is right.

> cpu_v7_reset() likes to give a reset pulse signal to CPU core logic. 
> The operation on SC_CPU_RESET_REQ register likes make CPU out of reset 
> mode. After system is power on, all CPUs except for CPU0 stay in reset 
> mode.

Sorry, I don't fully understand the above.

I also note in your code that you write the same bits to 
SC_CPU_RESET_REQ in both the power_up and power_down methods.  So if 
this is about sending a reset pulse only, how do you keep a CPU down for 
a long period?


Nicolas