Re: [U-Boot] [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load

2016-11-09 Thread york sun
On 11/09/2016 04:03 AM, Ashish Kumar wrote:
>>
>> Do you have substantial change beside the changing name from mc_ram_addr to 
>> mc_ram_aligned_base_addr?
>> [Ashish Kumar] It is not exactly name change. Here intent is to use 
>> userdefine memory size for MC  before this only 512MB of memory can be 
>> allocated to MC since it incorrectly used mc_ram_addr in place of 
>> mc_aligned_base_addr.
>> Ok, Name changes are there only in the function parameters to avoid 
>> confusion and retain the function signatures.
>
> Actually your change is more confusing. Let us try another way, for example 
> not changing the name, shall we?
> [Ashish Kumar] If we do not rename function params "mc_ram_addr" to this 
> "mc_ram_aligned_base_addr", we will be actually using aligned_base_addr but 
> in function params  it will be denoted as mc_ram_addr will that be correct?
>

I think the only substantial change you have is in mc_init() function, 
where you moved the local variable mc_ram_aligned_base_addr to global. 
You started to use this global variable below and passed it as function 
parameters several times. For those passed as parameters, you don't need 
to rename them. If you take out those, you don't have to reformat the 
line wrap. So most of your changes can be avoided. Try that, you will 
have very small change and easy to review. Maybe you can find a better 
way to deal with alignment.

York
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load

2016-11-09 Thread Ashish Kumar
Hello York,

Please see inline.

Regards

-Original Message-
From: york sun 
Sent: Tuesday, November 08, 2016 11:38 PM
To: Ashish Kumar ; Priyanka Jain ; 
u-boot@lists.denx.de
Cc: Prabhakar Kushwaha 
Subject: Re: [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load

On 11/07/2016 09:37 PM, Ashish Kumar wrote:
> Hello York,
>
> Please see inline.
>
> Regards
> Ashish
>
> -Original Message-
> From: york sun
> Sent: Tuesday, November 08, 2016 12:23 AM
> To: Priyanka Jain ; u-boot@lists.denx.de
> Cc: Prabhakar Kushwaha ; Ashish Kumar 
> 
> Subject: Re: [PATCH] driver: net: fsl-mc: Use aligned address for MC 
> FW load
>
> On 10/24/2016 01:05 AM, Priyanka Jain wrote:
>> Firmware of Management Complex (MC) should be loaded at 512MB aligned 
>> address.
>> So use aligned address for firmware load.
>>
>> Signed-off-by: Priyanka Jain 
>> Signed-off-by: Prabhakar Kushwaha 
>> Signed-off-by: Ashish Kumar 
>> ---
>>  drivers/net/fsl-mc/mc.c |   62 
>> ++
>>  1 files changed, 35 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 
>> 1811b0f..c66340b 100644
>> --- a/drivers/net/fsl-mc/mc.c
>> +++ b/drivers/net/fsl-mc/mc.c
>> @@ -29,6 +29,7 @@
>>  DECLARE_GLOBAL_DATA_PTR;
>>  static int mc_boot_status = -1;
>>  static int mc_dpl_applied = -1;
>> +static u64 mc_ram_aligned_base_addr;
>
> Why do you need this static variable? You didn't use it.
> [Ashish Kumar]  following two function uses global static variable
> -357,7 +369,6 @@ static unsigned long get_mc_boot_timeout_ms(void)  
> #ifdef CONFIG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
>  static int load_mc_aiop_img(u64 aiop_fw_addr)  {
> -   u64 mc_ram_addr = mc_get_dram_addr();
>  #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
> void *aiop_img;
>  #endif
>
>
> @@ -440,7 +451,6 @@ int mc_init(u64 mc_fw_addr, u64 mc_dpc_addr)
> size_t raw_image_size = 0;
>  #endif
> struct mc_version mc_ver_info;
> -   u64 mc_ram_aligned_base_addr;
> u8 mc_ram_num_256mb_blocks;
> size_t mc_ram_size = mc_get_dram_block_size();
>

You showed the deletion of local variable. You added a static variable, but 
also use local variable within function. The only place where this static 
variable gets assigned is by a pointer in calculate_mc_private_ram_params(). I 
don't see the need to declare it as a global variable.
[Ashish Kumar] Yes true, it is first set here then used from global instance in 
func: mc_apply_dpl(64 mc_dpl_addr)  { ... error = 
load_mc_dpl(mc_ram_aligned_base_addr, mc_ram_size, mc_dpl_addr);... } and

Then in func: load_mc_aiop_img((u64 aiop_fw_addr) {... mc_copy_image(...)  ...}

>>  #ifdef CONFIG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
>>  static int mc_aiop_applied = -1;
>>  #endif
>> @@ -87,12 +88,15 @@ void dump_mc_ccsr_regs(struct mc_ccsr_registers 
>> __iomem *mc_ccsr_regs)
>>  /**
>>   * Copying MC firmware or DPL image to DDR
>>   */
>> -static int mc_copy_image(const char *title,
>> - u64 image_addr, u32 image_size, u64 mc_ram_addr)
>> +static int mc_copy_image(const char *title, u64 image_addr,
>> + u32 image_size, u64 mc_ram_aligned_base_addr)
>>  {
>> -debug("%s copied to address %p\n", title, (void *)mc_ram_addr);
>> -memcpy((void *)mc_ram_addr, (void *)image_addr, image_size);
>> -flush_dcache_range(mc_ram_addr, mc_ram_addr + image_size);
>> +debug("%s copied to address %p\n", title,
>> +  (void *)mc_ram_aligned_base_addr);
>> +memcpy((void *)mc_ram_aligned_base_addr,
>> +   (void *)image_addr, image_size);
>> +flush_dcache_range(mc_ram_aligned_base_addr,
>> +   mc_ram_aligned_base_addr + image_size);
>>  return 0;
>>  }
>>
>> @@ -224,7 +228,8 @@ static int mc_fixup_dpc(u64 dpc_addr)
>>  return 0;
>>  }
>>
>> -static int load_mc_dpc(u64 mc_ram_addr, size_t mc_ram_size, u64
>> mc_dpc_addr)
>> +static int load_mc_dpc(u64 mc_ram_aligned_base_addr, size_t mc_ram_size,
>> +   u64 mc_dpc_addr)
>>  {
>>  u64 mc_dpc_offset;
>>  #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
>> @@ -246,7 +251,8 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
>> mc_ram_size, u64 mc_dpc_addr)
>>   * Load the MC DPC blob in the MC private DRAM block:
>>   */
>>  #ifdef CONFIG_SYS_LS_MC_DPC_IN_DDR
>> -printf("MC DPC is preloaded to %#llx\n", mc_ram_addr + mc_dpc_offset);
>> +printf("MC DPC is preloaded to %#llx\n",
>> +   mc_ram_aligned_base_addr + mc_dpc_offset);
>>  #else
>>  /*
>>   * Get address and size of the DPC blob stored in flash:
>> @@ -270,18 +276,20 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
>> mc_ram_size, u64 mc_dpc_addr)
>>  return -EINVAL;
>>  }
>>
>> -mc_copy_image("MC DPC blob",
>> -  (u64)dpc_fdt_hdr, dpc_size, mc_ram_addr + mc_dpc_offset);
>> +mc_copy_image("MC DPC blob", (u64)dpc_fdt_hdr, dpc_size,
>> +  mc_ram_alig

Re: [U-Boot] [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load

2016-11-08 Thread york sun
On 11/07/2016 09:37 PM, Ashish Kumar wrote:
> Hello York,
>
> Please see inline.
>
> Regards
> Ashish
>
> -Original Message-
> From: york sun
> Sent: Tuesday, November 08, 2016 12:23 AM
> To: Priyanka Jain ; u-boot@lists.denx.de
> Cc: Prabhakar Kushwaha ; Ashish Kumar 
> 
> Subject: Re: [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load
>
> On 10/24/2016 01:05 AM, Priyanka Jain wrote:
>> Firmware of Management Complex (MC) should be loaded at 512MB aligned
>> address.
>> So use aligned address for firmware load.
>>
>> Signed-off-by: Priyanka Jain 
>> Signed-off-by: Prabhakar Kushwaha 
>> Signed-off-by: Ashish Kumar 
>> ---
>>  drivers/net/fsl-mc/mc.c |   62 
>> ++
>>  1 files changed, 35 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index
>> 1811b0f..c66340b 100644
>> --- a/drivers/net/fsl-mc/mc.c
>> +++ b/drivers/net/fsl-mc/mc.c
>> @@ -29,6 +29,7 @@
>>  DECLARE_GLOBAL_DATA_PTR;
>>  static int mc_boot_status = -1;
>>  static int mc_dpl_applied = -1;
>> +static u64 mc_ram_aligned_base_addr;
>
> Why do you need this static variable? You didn't use it.
> [Ashish Kumar]  following two function uses global static variable
> -357,7 +369,6 @@ static unsigned long get_mc_boot_timeout_ms(void)
>  #ifdef CONFIG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
>  static int load_mc_aiop_img(u64 aiop_fw_addr)
>  {
> -   u64 mc_ram_addr = mc_get_dram_addr();
>  #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
> void *aiop_img;
>  #endif
>
>
> @@ -440,7 +451,6 @@ int mc_init(u64 mc_fw_addr, u64 mc_dpc_addr)
> size_t raw_image_size = 0;
>  #endif
> struct mc_version mc_ver_info;
> -   u64 mc_ram_aligned_base_addr;
> u8 mc_ram_num_256mb_blocks;
> size_t mc_ram_size = mc_get_dram_block_size();
>

You showed the deletion of local variable. You added a static variable, 
but also use local variable within function. The only place where this 
static variable gets assigned is by a pointer in 
calculate_mc_private_ram_params(). I don't see the need to declare it as 
a global variable.

>>  #ifdef CONFIG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
>>  static int mc_aiop_applied = -1;
>>  #endif
>> @@ -87,12 +88,15 @@ void dump_mc_ccsr_regs(struct mc_ccsr_registers
>> __iomem *mc_ccsr_regs)
>>  /**
>>   * Copying MC firmware or DPL image to DDR
>>   */
>> -static int mc_copy_image(const char *title,
>> - u64 image_addr, u32 image_size, u64 mc_ram_addr)
>> +static int mc_copy_image(const char *title, u64 image_addr,
>> + u32 image_size, u64 mc_ram_aligned_base_addr)
>>  {
>> -debug("%s copied to address %p\n", title, (void *)mc_ram_addr);
>> -memcpy((void *)mc_ram_addr, (void *)image_addr, image_size);
>> -flush_dcache_range(mc_ram_addr, mc_ram_addr + image_size);
>> +debug("%s copied to address %p\n", title,
>> +  (void *)mc_ram_aligned_base_addr);
>> +memcpy((void *)mc_ram_aligned_base_addr,
>> +   (void *)image_addr, image_size);
>> +flush_dcache_range(mc_ram_aligned_base_addr,
>> +   mc_ram_aligned_base_addr + image_size);
>>  return 0;
>>  }
>>
>> @@ -224,7 +228,8 @@ static int mc_fixup_dpc(u64 dpc_addr)
>>  return 0;
>>  }
>>
>> -static int load_mc_dpc(u64 mc_ram_addr, size_t mc_ram_size, u64
>> mc_dpc_addr)
>> +static int load_mc_dpc(u64 mc_ram_aligned_base_addr, size_t mc_ram_size,
>> +   u64 mc_dpc_addr)
>>  {
>>  u64 mc_dpc_offset;
>>  #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
>> @@ -246,7 +251,8 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
>> mc_ram_size, u64 mc_dpc_addr)
>>   * Load the MC DPC blob in the MC private DRAM block:
>>   */
>>  #ifdef CONFIG_SYS_LS_MC_DPC_IN_DDR
>> -printf("MC DPC is preloaded to %#llx\n", mc_ram_addr + mc_dpc_offset);
>> +printf("MC DPC is preloaded to %#llx\n",
>> +   mc_ram_aligned_base_addr + mc_dpc_offset);
>>  #else
>>  /*
>>   * Get address and size of the DPC blob stored in flash:
>> @@ -270,18 +276,20 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
>> mc_ram_size, u64 mc_dpc_addr)
>>  return -EINVAL;
>>  }
>>
>> -mc_copy_image("MC DPC blob",
>> -  (u64)dpc_fdt_hdr, dpc_size, mc_ram_addr + mc_dpc_offset);
>> +mc_copy_image("MC DPC blob", (u64)dpc_fdt_hdr, dpc_size,
>> +  mc_ram_aligned_base_addr + mc_dpc_offset);
>>  #endif /* not defined CONFIG_SYS_LS_MC_DPC_IN_DDR */
>>
>> -if (mc_fixup_dpc(mc_ram_addr + mc_dpc_offset))
>> +if (mc_fixup_dpc(mc_ram_aligned_base_addr + mc_dpc_offset))
>>  return -EINVAL;
>>
>> -dump_ram_words("DPC", (void *)(mc_ram_addr + mc_dpc_offset));
>> +dump_ram_words("DPC",
>> +   (void *)(mc_ram_aligned_base_addr + mc_dpc_offset));
>>  return 0;
>>  }
>>
>> -static int load_mc_dpl(u64 mc_ram_addr, size_t mc_ram_size, u64
>> mc_dpl_addr)
>> +static int load_mc_dpl(u64 mc_ram_al

Re: [U-Boot] [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load

2016-11-08 Thread Ashish Kumar
Hello York,

Please see inline.

Regards
Ashish

-Original Message-
From: york sun 
Sent: Tuesday, November 08, 2016 12:23 AM
To: Priyanka Jain ; u-boot@lists.denx.de
Cc: Prabhakar Kushwaha ; Ashish Kumar 

Subject: Re: [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load

On 10/24/2016 01:05 AM, Priyanka Jain wrote:
> Firmware of Management Complex (MC) should be loaded at 512MB aligned 
> address.
> So use aligned address for firmware load.
>
> Signed-off-by: Priyanka Jain 
> Signed-off-by: Prabhakar Kushwaha 
> Signed-off-by: Ashish Kumar 
> ---
>  drivers/net/fsl-mc/mc.c |   62 ++
>  1 files changed, 35 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 
> 1811b0f..c66340b 100644
> --- a/drivers/net/fsl-mc/mc.c
> +++ b/drivers/net/fsl-mc/mc.c
> @@ -29,6 +29,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  static int mc_boot_status = -1;
>  static int mc_dpl_applied = -1;
> +static u64 mc_ram_aligned_base_addr;

Why do you need this static variable? You didn't use it.
[Ashish Kumar]  following two function uses global static variable 
-357,7 +369,6 @@ static unsigned long get_mc_boot_timeout_ms(void)
 #ifdef CONFIG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
 static int load_mc_aiop_img(u64 aiop_fw_addr)
 {
-   u64 mc_ram_addr = mc_get_dram_addr();
 #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
void *aiop_img;
 #endif


@@ -440,7 +451,6 @@ int mc_init(u64 mc_fw_addr, u64 mc_dpc_addr)
size_t raw_image_size = 0;
 #endif
struct mc_version mc_ver_info;
-   u64 mc_ram_aligned_base_addr;
u8 mc_ram_num_256mb_blocks;
size_t mc_ram_size = mc_get_dram_block_size();

>  #ifdef CONFIG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
>  static int mc_aiop_applied = -1;
>  #endif
> @@ -87,12 +88,15 @@ void dump_mc_ccsr_regs(struct mc_ccsr_registers 
> __iomem *mc_ccsr_regs)
>  /**
>   * Copying MC firmware or DPL image to DDR
>   */
> -static int mc_copy_image(const char *title,
> -  u64 image_addr, u32 image_size, u64 mc_ram_addr)
> +static int mc_copy_image(const char *title, u64 image_addr,
> +  u32 image_size, u64 mc_ram_aligned_base_addr)
>  {
> - debug("%s copied to address %p\n", title, (void *)mc_ram_addr);
> - memcpy((void *)mc_ram_addr, (void *)image_addr, image_size);
> - flush_dcache_range(mc_ram_addr, mc_ram_addr + image_size);
> + debug("%s copied to address %p\n", title,
> +   (void *)mc_ram_aligned_base_addr);
> + memcpy((void *)mc_ram_aligned_base_addr,
> +(void *)image_addr, image_size);
> + flush_dcache_range(mc_ram_aligned_base_addr,
> +mc_ram_aligned_base_addr + image_size);
>   return 0;
>  }
>
> @@ -224,7 +228,8 @@ static int mc_fixup_dpc(u64 dpc_addr)
>   return 0;
>  }
>
> -static int load_mc_dpc(u64 mc_ram_addr, size_t mc_ram_size, u64 
> mc_dpc_addr)
> +static int load_mc_dpc(u64 mc_ram_aligned_base_addr, size_t mc_ram_size,
> +u64 mc_dpc_addr)
>  {
>   u64 mc_dpc_offset;
>  #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
> @@ -246,7 +251,8 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
> mc_ram_size, u64 mc_dpc_addr)
>* Load the MC DPC blob in the MC private DRAM block:
>*/
>  #ifdef CONFIG_SYS_LS_MC_DPC_IN_DDR
> - printf("MC DPC is preloaded to %#llx\n", mc_ram_addr + mc_dpc_offset);
> + printf("MC DPC is preloaded to %#llx\n",
> +mc_ram_aligned_base_addr + mc_dpc_offset);
>  #else
>   /*
>* Get address and size of the DPC blob stored in flash:
> @@ -270,18 +276,20 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
> mc_ram_size, u64 mc_dpc_addr)
>   return -EINVAL;
>   }
>
> - mc_copy_image("MC DPC blob",
> -   (u64)dpc_fdt_hdr, dpc_size, mc_ram_addr + mc_dpc_offset);
> + mc_copy_image("MC DPC blob", (u64)dpc_fdt_hdr, dpc_size,
> +   mc_ram_aligned_base_addr + mc_dpc_offset);
>  #endif /* not defined CONFIG_SYS_LS_MC_DPC_IN_DDR */
>
> - if (mc_fixup_dpc(mc_ram_addr + mc_dpc_offset))
> + if (mc_fixup_dpc(mc_ram_aligned_base_addr + mc_dpc_offset))
>   return -EINVAL;
>
> - dump_ram_words("DPC", (void *)(mc_ram_addr + mc_dpc_offset));
> + dump_ram_words("DPC",
> +(void *)(mc_ram_aligned_base_addr + mc_dpc_offset));
>   return 0;
>  }
>
> -static int load_mc_dpl(u64 mc_ram_addr, size_t mc_ram_size, u64 
> mc_dpl_addr)
> +static int load_mc_dpl(u64 mc_ram_aligned_base_addr, size_t mc_ram_size,
> +u64 mc_dpl_addr)
>  {
>   u64 mc_dpl_offset;
>  #ifndef CONFIG_SYS_LS_MC_DPL_IN_DDR
> @@ -303,7 +311,8 @@ static int load_mc_dpl(u64 mc_ram_addr, size_t 
> mc_ram_size, u64 mc_dpl_addr)
>* Load the MC DPL blob in the MC private DRAM block:
>*/
>  #ifdef CONFIG_SYS_LS_MC_DPL_IN_DDR
> - printf("MC DPL is preloaded to %#llx\n", mc_ram_addr + mc_dpl_offset);
> + prin

Re: [U-Boot] [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load

2016-11-07 Thread york sun
On 10/24/2016 01:05 AM, Priyanka Jain wrote:
> Firmware of Management Complex (MC) should be loaded at 512MB aligned
> address.
> So use aligned address for firmware load.
>
> Signed-off-by: Priyanka Jain 
> Signed-off-by: Prabhakar Kushwaha 
> Signed-off-by: Ashish Kumar 
> ---
>  drivers/net/fsl-mc/mc.c |   62 ++
>  1 files changed, 35 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c
> index 1811b0f..c66340b 100644
> --- a/drivers/net/fsl-mc/mc.c
> +++ b/drivers/net/fsl-mc/mc.c
> @@ -29,6 +29,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  static int mc_boot_status = -1;
>  static int mc_dpl_applied = -1;
> +static u64 mc_ram_aligned_base_addr;

Why do you need this static variable? You didn't use it.

>  #ifdef CONFIG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
>  static int mc_aiop_applied = -1;
>  #endif
> @@ -87,12 +88,15 @@ void dump_mc_ccsr_regs(struct mc_ccsr_registers __iomem 
> *mc_ccsr_regs)
>  /**
>   * Copying MC firmware or DPL image to DDR
>   */
> -static int mc_copy_image(const char *title,
> -  u64 image_addr, u32 image_size, u64 mc_ram_addr)
> +static int mc_copy_image(const char *title, u64 image_addr,
> +  u32 image_size, u64 mc_ram_aligned_base_addr)
>  {
> - debug("%s copied to address %p\n", title, (void *)mc_ram_addr);
> - memcpy((void *)mc_ram_addr, (void *)image_addr, image_size);
> - flush_dcache_range(mc_ram_addr, mc_ram_addr + image_size);
> + debug("%s copied to address %p\n", title,
> +   (void *)mc_ram_aligned_base_addr);
> + memcpy((void *)mc_ram_aligned_base_addr,
> +(void *)image_addr, image_size);
> + flush_dcache_range(mc_ram_aligned_base_addr,
> +mc_ram_aligned_base_addr + image_size);
>   return 0;
>  }
>
> @@ -224,7 +228,8 @@ static int mc_fixup_dpc(u64 dpc_addr)
>   return 0;
>  }
>
> -static int load_mc_dpc(u64 mc_ram_addr, size_t mc_ram_size, u64 mc_dpc_addr)
> +static int load_mc_dpc(u64 mc_ram_aligned_base_addr, size_t mc_ram_size,
> +u64 mc_dpc_addr)
>  {
>   u64 mc_dpc_offset;
>  #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
> @@ -246,7 +251,8 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
> mc_ram_size, u64 mc_dpc_addr)
>* Load the MC DPC blob in the MC private DRAM block:
>*/
>  #ifdef CONFIG_SYS_LS_MC_DPC_IN_DDR
> - printf("MC DPC is preloaded to %#llx\n", mc_ram_addr + mc_dpc_offset);
> + printf("MC DPC is preloaded to %#llx\n",
> +mc_ram_aligned_base_addr + mc_dpc_offset);
>  #else
>   /*
>* Get address and size of the DPC blob stored in flash:
> @@ -270,18 +276,20 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
> mc_ram_size, u64 mc_dpc_addr)
>   return -EINVAL;
>   }
>
> - mc_copy_image("MC DPC blob",
> -   (u64)dpc_fdt_hdr, dpc_size, mc_ram_addr + mc_dpc_offset);
> + mc_copy_image("MC DPC blob", (u64)dpc_fdt_hdr, dpc_size,
> +   mc_ram_aligned_base_addr + mc_dpc_offset);
>  #endif /* not defined CONFIG_SYS_LS_MC_DPC_IN_DDR */
>
> - if (mc_fixup_dpc(mc_ram_addr + mc_dpc_offset))
> + if (mc_fixup_dpc(mc_ram_aligned_base_addr + mc_dpc_offset))
>   return -EINVAL;
>
> - dump_ram_words("DPC", (void *)(mc_ram_addr + mc_dpc_offset));
> + dump_ram_words("DPC",
> +(void *)(mc_ram_aligned_base_addr + mc_dpc_offset));
>   return 0;
>  }
>
> -static int load_mc_dpl(u64 mc_ram_addr, size_t mc_ram_size, u64 mc_dpl_addr)
> +static int load_mc_dpl(u64 mc_ram_aligned_base_addr, size_t mc_ram_size,
> +u64 mc_dpl_addr)
>  {
>   u64 mc_dpl_offset;
>  #ifndef CONFIG_SYS_LS_MC_DPL_IN_DDR
> @@ -303,7 +311,8 @@ static int load_mc_dpl(u64 mc_ram_addr, size_t 
> mc_ram_size, u64 mc_dpl_addr)
>* Load the MC DPL blob in the MC private DRAM block:
>*/
>  #ifdef CONFIG_SYS_LS_MC_DPL_IN_DDR
> - printf("MC DPL is preloaded to %#llx\n", mc_ram_addr + mc_dpl_offset);
> + printf("MC DPL is preloaded to %#llx\n",
> +mc_ram_aligned_base_addr + mc_dpl_offset);
>  #else
>   /*
>* Get address and size of the DPL blob stored in flash:
> @@ -323,11 +332,12 @@ static int load_mc_dpl(u64 mc_ram_addr, size_t 
> mc_ram_size, u64 mc_dpl_addr)
>   return -EINVAL;
>   }
>
> - mc_copy_image("MC DPL blob",
> -   (u64)dpl_fdt_hdr, dpl_size, mc_ram_addr + mc_dpl_offset);
> + mc_copy_image("MC DPL blob", (u64)dpl_fdt_hdr, dpl_size,
> +   mc_ram_aligned_base_addr + mc_dpl_offset);
>  #endif /* not defined CONFIG_SYS_LS_MC_DPL_IN_DDR */
>
> - dump_ram_words("DPL", (void *)(mc_ram_addr + mc_dpl_offset));
> + dump_ram_words("DPL",
> +(void *)(mc_ram_aligned_base_addr + mc_dpl_offset));
>   return 0;
>  }
>
> @@ -364,7 +374,6 @@ __weak bool soc_has_aiop(void)
>
>  static int load_mc_aiop_img(u64 a

[U-Boot] [PATCH] driver: net: fsl-mc: Use aligned address for MC FW load

2016-10-24 Thread Priyanka Jain
Firmware of Management Complex (MC) should be loaded at 512MB aligned
address.
So use aligned address for firmware load.

Signed-off-by: Priyanka Jain 
Signed-off-by: Prabhakar Kushwaha 
Signed-off-by: Ashish Kumar 
---
 drivers/net/fsl-mc/mc.c |   62 ++
 1 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c
index 1811b0f..c66340b 100644
--- a/drivers/net/fsl-mc/mc.c
+++ b/drivers/net/fsl-mc/mc.c
@@ -29,6 +29,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 static int mc_boot_status = -1;
 static int mc_dpl_applied = -1;
+static u64 mc_ram_aligned_base_addr;
 #ifdef CONFIG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
 static int mc_aiop_applied = -1;
 #endif
@@ -87,12 +88,15 @@ void dump_mc_ccsr_regs(struct mc_ccsr_registers __iomem 
*mc_ccsr_regs)
 /**
  * Copying MC firmware or DPL image to DDR
  */
-static int mc_copy_image(const char *title,
-u64 image_addr, u32 image_size, u64 mc_ram_addr)
+static int mc_copy_image(const char *title, u64 image_addr,
+u32 image_size, u64 mc_ram_aligned_base_addr)
 {
-   debug("%s copied to address %p\n", title, (void *)mc_ram_addr);
-   memcpy((void *)mc_ram_addr, (void *)image_addr, image_size);
-   flush_dcache_range(mc_ram_addr, mc_ram_addr + image_size);
+   debug("%s copied to address %p\n", title,
+ (void *)mc_ram_aligned_base_addr);
+   memcpy((void *)mc_ram_aligned_base_addr,
+  (void *)image_addr, image_size);
+   flush_dcache_range(mc_ram_aligned_base_addr,
+  mc_ram_aligned_base_addr + image_size);
return 0;
 }
 
@@ -224,7 +228,8 @@ static int mc_fixup_dpc(u64 dpc_addr)
return 0;
 }
 
-static int load_mc_dpc(u64 mc_ram_addr, size_t mc_ram_size, u64 mc_dpc_addr)
+static int load_mc_dpc(u64 mc_ram_aligned_base_addr, size_t mc_ram_size,
+  u64 mc_dpc_addr)
 {
u64 mc_dpc_offset;
 #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
@@ -246,7 +251,8 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t mc_ram_size, 
u64 mc_dpc_addr)
 * Load the MC DPC blob in the MC private DRAM block:
 */
 #ifdef CONFIG_SYS_LS_MC_DPC_IN_DDR
-   printf("MC DPC is preloaded to %#llx\n", mc_ram_addr + mc_dpc_offset);
+   printf("MC DPC is preloaded to %#llx\n",
+  mc_ram_aligned_base_addr + mc_dpc_offset);
 #else
/*
 * Get address and size of the DPC blob stored in flash:
@@ -270,18 +276,20 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t 
mc_ram_size, u64 mc_dpc_addr)
return -EINVAL;
}
 
-   mc_copy_image("MC DPC blob",
- (u64)dpc_fdt_hdr, dpc_size, mc_ram_addr + mc_dpc_offset);
+   mc_copy_image("MC DPC blob", (u64)dpc_fdt_hdr, dpc_size,
+ mc_ram_aligned_base_addr + mc_dpc_offset);
 #endif /* not defined CONFIG_SYS_LS_MC_DPC_IN_DDR */
 
-   if (mc_fixup_dpc(mc_ram_addr + mc_dpc_offset))
+   if (mc_fixup_dpc(mc_ram_aligned_base_addr + mc_dpc_offset))
return -EINVAL;
 
-   dump_ram_words("DPC", (void *)(mc_ram_addr + mc_dpc_offset));
+   dump_ram_words("DPC",
+  (void *)(mc_ram_aligned_base_addr + mc_dpc_offset));
return 0;
 }
 
-static int load_mc_dpl(u64 mc_ram_addr, size_t mc_ram_size, u64 mc_dpl_addr)
+static int load_mc_dpl(u64 mc_ram_aligned_base_addr, size_t mc_ram_size,
+  u64 mc_dpl_addr)
 {
u64 mc_dpl_offset;
 #ifndef CONFIG_SYS_LS_MC_DPL_IN_DDR
@@ -303,7 +311,8 @@ static int load_mc_dpl(u64 mc_ram_addr, size_t mc_ram_size, 
u64 mc_dpl_addr)
 * Load the MC DPL blob in the MC private DRAM block:
 */
 #ifdef CONFIG_SYS_LS_MC_DPL_IN_DDR
-   printf("MC DPL is preloaded to %#llx\n", mc_ram_addr + mc_dpl_offset);
+   printf("MC DPL is preloaded to %#llx\n",
+  mc_ram_aligned_base_addr + mc_dpl_offset);
 #else
/*
 * Get address and size of the DPL blob stored in flash:
@@ -323,11 +332,12 @@ static int load_mc_dpl(u64 mc_ram_addr, size_t 
mc_ram_size, u64 mc_dpl_addr)
return -EINVAL;
}
 
-   mc_copy_image("MC DPL blob",
- (u64)dpl_fdt_hdr, dpl_size, mc_ram_addr + mc_dpl_offset);
+   mc_copy_image("MC DPL blob", (u64)dpl_fdt_hdr, dpl_size,
+ mc_ram_aligned_base_addr + mc_dpl_offset);
 #endif /* not defined CONFIG_SYS_LS_MC_DPL_IN_DDR */
 
-   dump_ram_words("DPL", (void *)(mc_ram_addr + mc_dpl_offset));
+   dump_ram_words("DPL",
+  (void *)(mc_ram_aligned_base_addr + mc_dpl_offset));
return 0;
 }
 
@@ -364,7 +374,6 @@ __weak bool soc_has_aiop(void)
 
 static int load_mc_aiop_img(u64 aiop_fw_addr)
 {
-   u64 mc_ram_addr = mc_get_dram_addr();
 #ifndef CONFIG_SYS_LS_MC_DPC_IN_DDR
void *aiop_img;
 #endif
@@ -377,13 +386,14 @@ static int load_mc_aiop_img(u64 aiop_fw_addr)
 */
 
 #ifdef C