[RFC][PATCH] Kexec support for PPC440x

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2011-05-31 at 17:15 +0200, Sebastian Andrzej Siewior wrote:
> Suzuki Poulose wrote:
> > Index: powerpc/arch/powerpc/kernel/44x_kexec_mapping.S
> > ===================================================================
> > --- /dev/null
> > +++ powerpc/arch/powerpc/kernel/44x_kexec_mapping.S
> ....
> 
> > + *
> > + */
> > +    bl    nxtins                /* Find our address */
> > +nxtins:    mflr    r5                /* Make it accessible */
> 
> Please don't mix labels and instructions.

With proper indent it's fine as long as he uses numerical relative
labels which should be the case here. For example, the above, it should
look like:

	bl	1f
1:	mflr	r5

> > +    tlbsx    r23,0,r5            /* Find entry we are in */
> 
> using tabs instead of spaces would make it look nice. Please also separate 
> the arguments of the instruction i.e.
> 	tlbsx	r23, 0, r5

That's arguable. If you look at arch/powerpc, we tend not to separate
the arguments ;-)

Actually I used to, others didn't and I changed my own style.

> > Index: powerpc/arch/powerpc/kernel/misc_32.S
> > ===================================================================
> > --- powerpc.orig/arch/powerpc/kernel/misc_32.S
> > +++ powerpc/arch/powerpc/kernel/misc_32.S
> > @@ -736,6 +736,28 @@ relocate_new_kernel:
> >      mr      r5, r31
> >  
> >      li    r0, 0
> > +#elif defined(CONFIG_44x)  && !defined(CONFIG_47x)
> > +
> > +    mr    r29, r3
> > +    mr    r30, r4
> > +    mr    r31, r5
> > +
> > +#include "44x_kexec_mapping.S"
> 
> The way you setup the 1:1 mapping should be close to what you are doing on
> kernel entry. Isn't it possible to include the file here and in the entry
> code?

It should just not be #included, what about branching out instead ?

Cheers,
Ben.





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux