[PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers

[Scott Wood]

On Sun, 2014-01-19 at 23:57 -0600, Wang Dongsheng-B40534 wrote:
> > > > > Use fsl_cpu_state_save/fsl_cpu_state_restore to save/restore registers.
> > > > > Use the functions to save/restore registers, so we don't need to
> > > > > maintain the code.
> > > > >
> > > > > Signed-off-by: Wang Dongsheng <dongsheng.wang at freescale.com>
> > > >
> > > > Is there any functional change with this patchset (e.g. suspend
> > > > supported on chips where it wasn't before), or is it just cleanup?  A
> > > > cover letter would be useful to describe the purpose of the overall
> > > > patchset when it isn't obvious.
> > > >
> > >
> > > Yes, just cleanup..
> > 
> > It seems to be introducing complexity rather than removing it.  Is this
> > cleanup needed to prepare for adding new functionality?
> > 
> > Plus, I'm skeptical that this is functionally equivalent.  It looks like
> > the new code saves a lot more than the old code does.  Why?
> > 
> 
> Actually, I want to take a practical example to push the save/restore patches.
> And this is also reasonable for 32bit-hibernation, the code is more clean. :)
> I think I need to change the description of the patch.
> 
> > > > > +
> > > > > +	/* Restore base register */
> > > > > +	li	r4, 0
> > > > > +	bl	fsl_cpu_state_restore
> > > >
> > > > Why are you calling anything with "fsl" in the name from code that is
> > > > supposed to be for all booke?
> > > >
> > > E200, E300 not support.
> > > Support E500, E500v2, E500MC, E5500, E6500.
> > >
> > > Do you have any suggestions about this?
> > 
> > What about non-FSL booke such as 44x?
> > 
> > Or if this file never supported 44x, rename it appropriately.
> > 
> Currently does not support. ok change the name first, if later support, and
> then again to modify the name of this function.
> 
> How about 85xx_cpu_state_restore?

Symbols can't begin with numbers.  booke_cpu_state_restore would be
better (it would still provide a place for 44x to be added if somebody
actually cared about doing so).

I'm still not convinced that asm code is the place to do this, though.

-Scott