On Fri, Dec 04, 2015 at 06:40:40PM -0500, Alex Deucher wrote:

> +void acp_turnonoff_lower_sram_bank(void __iomem *acp_mmio, u16 bank,
> +					bool turnon)

Can we have a name like acp_set_lower_sram_bank_state() or something to
make the boolean a bit more obvious please?  You could probably also
combine this with the equivalent function for the upper bank and select
based on the value which registers to work with, that'd be much clearer
on the user side.

> +	/* If ACP_MEM_SHUT_DOWN_STS_LO is 0xFFFFFFFF, then
> +	 * shutdown sequence is complete.
> +	 */
> +	 while (acp_reg_read(acp_mmio,
> +				      mmACP_MEM_SHUT_DOWN_STS_LO)
> +				      != 0xFFFFFFFF)
> +		cpu_relax();

Strange intentation for the continuation lines (usually you'd align with
the '(' for the matching level) and this needs a timeout for safety.