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: b...@kernel.crashing.org; Zhao Chenhui-B35336; an...@enomsg.org; > > linuxppc- > > d...@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.w...@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.w...@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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
