Re: [U-Boot] [PATCH] pci: Fix decode regions for memory banks

[Bin Meng]

Hi Bernhard,

On Fri, Mar 23, 2018 at 12:27 AM, Bernhard Messerklinger
 wrote:
> Hi,
>
> Thanks for the report.
> I am answering from my private email. At the moment I can't find any issue
> regarding my patch. It should make no difference since dram_init_banksize in
> quark/dram.c should set the dram bank. I will continue my investigation and
> contact you if I find out something.
>

I've figured out where the bug is. Will send a patch soon.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] pci: Fix decode regions for memory banks

[Bernhard Messerklinger]

Hi,

Thanks for the report.
I am answering from my private email. At the moment I can't find any issue
regarding my patch. It should make no difference since dram_init_banksize in
quark/dram.c should set the dram bank. I will continue my investigation and
contact you if I find out something.

Regards,
Bernhard

On Thu, Mar 22, 2018 at 10:06 AM, Bin Meng  wrote:
> Hi,
>
> On Thu, Feb 15, 2018 at 3:59 PM, Bernhard Messerklinger
>  wrote:
>> Since memory banks may not be located behind each other we need to add
>> them separately.
>>
>> Signed-off-by: Bernhard Messerklinger 
>> 
>> ---
>>
>>  drivers/pci/pci-uclass.c | 17 -
>>  1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
>> index 5a24eb6428..ad43e8a27c 100644
>> --- a/drivers/pci/pci-uclass.c
>> +++ b/drivers/pci/pci-uclass.c
>> @@ -815,7 +815,6 @@ static int decode_regions(struct pci_controller *hose, 
>> ofnode parent_node,
>>   ofnode node)
>>  {
>> int pci_addr_cells, addr_cells, size_cells;
>> -   phys_addr_t base = 0, size;
>> int cells_per_record;
>> const u32 *prop;
>> int len;
>> @@ -874,6 +873,21 @@ static int decode_regions(struct pci_controller *hose, 
>> ofnode parent_node,
>> }
>>
>> /* Add a region for our local memory */
>> +#ifdef CONFIG_NR_DRAM_BANKS
>> +   bd_t *bd = gd->bd;
>> +
>> +   for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
>> +   if (bd->bi_dram[i].size) {
>> +   pci_set_region(hose->regions + hose->region_count++,
>> +  bd->bi_dram[i].start,
>> +  bd->bi_dram[i].start,
>> +  bd->bi_dram[i].size,
>> +  PCI_REGION_MEM | 
>> PCI_REGION_SYS_MEMORY);
>> +   }
>> +   }
>> +#else
>
> Sorry for jumping out. With this commit, Intel Galileo board does not
> boot any more. x86 defines CONFIG_NR_DRAM_BANKS in x86-common.h, so
> this commit forces x86 to use the new logic instead of the old one,
> which breaks things. I have not debugged this on how to fix it. Any
> ideas?
>
>> +   phys_addr_t base = 0, size;
>> +
>> size = gd->ram_size;
>>  #ifdef CONFIG_SYS_SDRAM_BASE
>> base = CONFIG_SYS_SDRAM_BASE;
>> @@ -882,6 +896,7 @@ static int decode_regions(struct pci_controller *hose, 
>> ofnode parent_node,
>> size = gd->pci_ram_top - base;
>> pci_set_region(hose->regions + hose->region_count++, base, base,
>>size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>> +#endif
>>
>> return 0;
>>  }
>> --
>
> Regards,
> Bin
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] pci: Fix decode regions for memory banks

[Bin Meng]

Hi,

On Thu, Feb 15, 2018 at 3:59 PM, Bernhard Messerklinger
 wrote:
> Since memory banks may not be located behind each other we need to add
> them separately.
>
> Signed-off-by: Bernhard Messerklinger 
> 
> ---
>
>  drivers/pci/pci-uclass.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 5a24eb6428..ad43e8a27c 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -815,7 +815,6 @@ static int decode_regions(struct pci_controller *hose, 
> ofnode parent_node,
>   ofnode node)
>  {
> int pci_addr_cells, addr_cells, size_cells;
> -   phys_addr_t base = 0, size;
> int cells_per_record;
> const u32 *prop;
> int len;
> @@ -874,6 +873,21 @@ static int decode_regions(struct pci_controller *hose, 
> ofnode parent_node,
> }
>
> /* Add a region for our local memory */
> +#ifdef CONFIG_NR_DRAM_BANKS
> +   bd_t *bd = gd->bd;
> +
> +   for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> +   if (bd->bi_dram[i].size) {
> +   pci_set_region(hose->regions + hose->region_count++,
> +  bd->bi_dram[i].start,
> +  bd->bi_dram[i].start,
> +  bd->bi_dram[i].size,
> +  PCI_REGION_MEM | 
> PCI_REGION_SYS_MEMORY);
> +   }
> +   }
> +#else

Sorry for jumping out. With this commit, Intel Galileo board does not
boot any more. x86 defines CONFIG_NR_DRAM_BANKS in x86-common.h, so
this commit forces x86 to use the new logic instead of the old one,
which breaks things. I have not debugged this on how to fix it. Any
ideas?

> +   phys_addr_t base = 0, size;
> +
> size = gd->ram_size;
>  #ifdef CONFIG_SYS_SDRAM_BASE
> base = CONFIG_SYS_SDRAM_BASE;
> @@ -882,6 +896,7 @@ static int decode_regions(struct pci_controller *hose, 
> ofnode parent_node,
> size = gd->pci_ram_top - base;
> pci_set_region(hose->regions + hose->region_count++, base, base,
>size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> +#endif
>
> return 0;
>  }
> --

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] pci: Fix decode regions for memory banks

[Hannes Schmelzer]

Bernhard Messerklinger  schrieb 
am 15.02.2018 08:59:53:

> Von: Bernhard Messerklinger 
> An: u-boot@lists.denx.de
> Kopie: hannes.schmel...@br-automation.com, Bernhard Messerklinger 
> , Simon Glass 
, 
> Masahiro Yamada , Tuomas Tynkkynen 
> , Bin Meng , 
"xypron.g...@gmx.de"
> , Hou Zhiqiang 
> Datum: 15.02.2018 09:00
> Betreff: [PATCH] pci: Fix decode regions for memory banks
> 
> Since memory banks may not be located behind each other we need to add
> them separately.
> 
> Signed-off-by: Bernhard Messerklinger 

> ---
> 
>  drivers/pci/pci-uclass.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 5a24eb6428..ad43e8a27c 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -815,7 +815,6 @@ static int decode_regions(struct pci_controller 
*hose, 
> ofnode parent_node,
> ofnode node)
>  {
> int pci_addr_cells, addr_cells, size_cells;
> -   phys_addr_t base = 0, size;
> int cells_per_record;
> const u32 *prop;
> int len;
> @@ -874,6 +873,21 @@ static int decode_regions(struct pci_controller 
*hose, 
> ofnode parent_node,
> }
> 
> /* Add a region for our local memory */
> +#ifdef CONFIG_NR_DRAM_BANKS
> +   bd_t *bd = gd->bd;
> +
> +   for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> +  if (bd->bi_dram[i].size) {
> + pci_set_region(hose->regions + hose->region_count++,
> +   bd->bi_dram[i].start,
> +   bd->bi_dram[i].start,
> +   bd->bi_dram[i].size,
> +   PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> +  }
> +   }
> +#else
> +   phys_addr_t base = 0, size;
> +
> size = gd->ram_size;
>  #ifdef CONFIG_SYS_SDRAM_BASE
> base = CONFIG_SYS_SDRAM_BASE;
> @@ -882,6 +896,7 @@ static int decode_regions(struct pci_controller 
*hose, 
> ofnode parent_node,
>size = gd->pci_ram_top - base;
> pci_set_region(hose->regions + hose->region_count++, base, base,
>   size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> +#endif
> 
> return 0;
>  }
> -- 
> 2.16.1
> 

Reviewed-by: Hannes Schmelzer 


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