Re: [PATCH V2 2/5] arm: mvebu: Added initial support for power managmement service unit

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Mon, Nov 05, 2012 at 02:05:58PM +0000, Will Deacon wrote:
> On Mon, Oct 29, 2012 at 09:11:45PM +0000, Gregory CLEMENT wrote:
> > +#ifdef CONFIG_SMP
> > +int armada_xp_boot_cpu(unsigned int cpu_id, void __iomem *boot_addr)
> > +{
> > +	int reg, hw_cpu;
> > +
> > +	if (!pmsu_mp_base || !pmsu_reset_base) {
> > +		pr_warn("Can't boot CPU. PMSU is uninitialized\n");
> > +		return 1;
> > +	}
> > +
> > +	hw_cpu = cpu_logical_map(cpu_id);
> > +
> > +	writel(virt_to_phys(boot_addr), pmsu_mp_base +
> > +			PMSU_BOOT_ADDR_REDIRECT_OFFSET(hw_cpu));
> 
> virt_to_phys on an __iomem * doesn't feel right to me...

It isn't correct - I suspect the __iomem there is just wrong - and it
needs the callsite also checking.

> > +	/* Make sure value hits memory before reset */
> > +	dsb();
> 
> writel has barrier semantics -- you shouldn't need this dsb.

writel has a barrier before the write (to ensure that DMA agents see data
that was written to memory when they are enabled by the write).  There
isn't a barrier after the write.