Re: [PATCH v3 1/6] ARM: davinci: sram: ioremap the davinci_soc_infospecified sram regions

[Sekhar Nori]

On 10/3/2012 8:25 PM, Matt Porter wrote:
> From: Ben Gardiner <bengardiner@xxxxxxxxxxxxxx>
> 
> The current davinci init sets up SRAM in iotables. There has been an observed
> failure to boot a da850 with 128K specified in the iotable.
> 
> Make the davinci sram allocator -- now based on RMK's consolidated SRAM
> support -- do an ioremap of the region specified by the entries in

The part about being based on RMK's consolidated SRAM support should be
dropped.

> davinci_soc_info before registering with gen_pool_add_virt().
> 
> This commit breaks runtime of davinci boards since the regions that
> the sram init is now trying to ioremap have been iomapped by their
> iotable entries. The iotable entries will be removed in the patches
> to come.

I would prefer merging 2/6 into this for this reason.

> 
> Signed-off-by: Ben Gardiner <bengardiner@xxxxxxxxxxxxxx>
> [rebased to mainline as the consolidated SRAM support was dropped]
> Signed-off-by: Matt Porter <mporter@xxxxxx>
> ---
>  arch/arm/mach-davinci/sram.c |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
> index db0f778..0e8ca4f 100644
> --- a/arch/arm/mach-davinci/sram.c
> +++ b/arch/arm/mach-davinci/sram.c
> @@ -10,6 +10,7 @@
>   */
>  #include <linux/module.h>
>  #include <linux/init.h>
> +#include <linux/io.h>
>  #include <linux/genalloc.h>
>  
>  #include <mach/common.h>
> @@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
>  		return NULL;
>  
>  	if (dma)
> -		*dma = dma_base + (vaddr - SRAM_VIRT);
> +		*dma = gen_pool_virt_to_phys(sram_pool, vaddr);
>  	return (void *)vaddr;
>  
>  }
> @@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
>   */
>  static int __init sram_init(void)
>  {
> +	phys_addr_t phys = davinci_soc_info.sram_dma;
>  	unsigned len = davinci_soc_info.sram_len;
>  	int status = 0;
> +	void *addr;
>  
>  	if (len) {
>  		len = min_t(unsigned, len, SRAM_SIZE);
> @@ -62,8 +65,16 @@ static int __init sram_init(void)
>  		if (!sram_pool)
>  			status = -ENOMEM;
>  	}
> -	if (sram_pool)
> -		status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
> +
> +	if (sram_pool) {
> +		addr = ioremap(phys, len);
> +		if (!addr)
> +			return -ENOMEM;
> +		if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
> +					       phys, len, -1)))

Nit: prefer to set status outside of if().

Looks good otherwise. Thanks for reviving this.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/