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

Scott Wood scottwood at freescale.com
Thu Jan 16 14:15:43 EST 2014


On Tue, 2014-01-14 at 20:57 -0600, Wang Dongsheng-B40534 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, January 15, 2014 7:30 AM
> > To: Wang Dongsheng-B40534
> > Cc: benh at kernel.crashing.org; Zhao Chenhui-B35336; anton at enomsg.org; linuxppc-
> > dev at lists.ozlabs.org
> > Subject: Re: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore
> > registers
> > 
> > On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote:
> > > From: Wang Dongsheng <dongsheng.wang at freescale.com>
> > >
> > > 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?

> > > +
> > > +	/* 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.

-Scott




More information about the Linuxppc-dev mailing list