* Tony Lindgren <tony@atomide.com> [080823 15:16]:
> * Tony Lindgren <tony@atomide.com> [080820 00:36]:
> > * Russell King - ARM Linux <linux@arm.linux.org.uk> [080819 20:03]:
> > > On Fri, Jun 06, 2008 at 07:12:28PM -0700, Tony Lindgren wrote:
> > > > Some register offsets are different for 242x and 243x. This
> > > > will allow compiling sleep code for both chips into the same
> > > > kernel.
> > > > 
> > > > Note that some PM patches are still missing. The PM patches will
> > > > be added later on once the base files are in sync with linux-omap
> > > > tree.
> > > 
> > > Please use git diff -M, since it makes the changes across renames more
> > > obvious.
> > 
> > OK
> > 
> > > > +ENTRY(omap242x_idle_loop_suspend)
> > > > +	stmfd	sp!, {r0, lr}		@ save registers on stack
> > > > +	mov	r0, #0x0		@ clear for mrc call
> > > > +	mcr	p15, 0, r0, c7, c0, 4	@ wait for interrupt
> > > > +	ldmfd	sp!, {r0, pc}		@ restore regs and return
> > > 
> > > What's been lost because of the lack of git diff -M here is the real
> > > change:
> > > 
> > > -ENTRY(omap24xx_idle_loop_suspend)
> > > +ENTRY(omap242x_idle_loop_suspend)
> > >  	stmfd	sp!, {r0, lr}		@ save registers on stack
> > > -	mov	r0, #0			@ clear for mcr setup
> > > +	mov	r0, #0x0		@ clear for mrc call
> > >  	mcr	p15, 0, r0, c7, c0, 4	@ wait for interrupt
> > >  	ldmfd	sp!, {r0, pc}		@ restore regs and return
> > > 
> > > which makes the problem stand out.  That change of the 'mov' line
> > > along with the comment is completely bogus.  In fact, the change to
> > > the comment is clearly wrong.  The same applies to sleep243x.S
> > 
> > Will remove.
> > 
> > > Realistically, the only real difference between the two files are
> > > these lines:
> > > 
> > >  omap2_ocs_sdrc_power:
> > > -       .word OMAP242X_SDRC_REGADDR(SDRC_POWER)
> > > +       .word OMAP243X_SDRC_REGADDR(SDRC_POWER)
> > >  A_SDRC0:
> > >         .word A_SDRC0_V
> > >  omap2_ocs_sdrc_dlla_ctrl:
> > > -       .word OMAP242X_SDRC_REGADDR(SDRC_DLLA_CTRL)
> > > +       .word OMAP243X_SDRC_REGADDR(SDRC_DLLA_CTRL)
> > > 
> > > so is doubling the size of this code really justified?
> > 
> > Yes duplication is a problem. We had code that was dynamically
> > rewriting the addresses but it was not very easy to follow and
> > hard to debug. This code is only compiled in twice if both 242x
> > and 243x are both selected though.
> > 
> > > Looking harder at this code:
> > > 
> > > ENTRY(omap242x_cpu_suspend)
> > >         stmfd   sp!, {r0 - r12, lr}     @ save registers on stack
> > > ...
> > >         mov     r5, #0x2000             @ set delay (DPLL relock + DLL relock)
> > > ...
> > >         nop
> > >         mcr     p15, 0, r2, c7, c0, 4   @ wait for interrupt
> > >         nop
> > > loop:
> > >         subs    r5, r5, #0x1            @ awake, wait just a bit
> > >         bne     loop
> > > 
> > >         ldmfd   sp!, {r0 - r12, pc}     @ restore regs and return
> > > 
> > > it's clear that registers are preserved across the wait-for-interrupt
> > > instruction, so I'm not sure why saving all the registers is really
> > > necessary, but that's only a side point to my main point, which is...
> > > 
> > > ... that you could pass the addresses of these registers into the
> > > function, either directly:
> > > 
> > > void omap24xx_cpu_suspend(u32 dll_ctrl, u32 cpu_revision,
> > > 			  void __iomem *sdrc_pwr,
> > > 			  void __iomem *sdrc_dlla_ctrl);
> > > 
> > > or via a structure, and thereby avoid this duplication.
> > 
> > The structure would have to be also in SRAM then. I guess in this case
> > there are only two addresses, so passing them via into the function
> > should be enough. It's unlikely that this code changes any further
> > to need more addresses.
> > 
> > Will repost.
> 
> Here's this one refreshed. Looks like there was also a bug for boards
> with SDR instead of DDR.

Patch header was missing, here's the same patch with header and
Signed-off-by.

Tony