* [U-Boot] [RFC PATCH] driver: pci: Fix regions for local memory
@ 2017-02-09 18:35 York Sun
2017-03-16 22:47 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: York Sun @ 2017-02-09 18:35 UTC (permalink / raw)
To: u-boot
When adding local memory to PCI region, gd->ram_size is correct only
if the memory is in one continuous block. In case memory is split
into several banks, each bank should be added separately.
Signed-off-by: York Sun <york.sun@nxp.com>
CC: Simon Glass <sjg@chromium.org>
---
It was spotted when I was rewriting the code to reserve secure memory
and forgot to reduce gd->ram_size. PCIe resumes working after fixing
gd->ram_size. For my case, the memory is split into two banks. So
base + gd->ram_size is not in memory. I don't know how it worked before.
This change seems reasonable without digging into PCI code.
drivers/pci/pci-uclass.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 3b00e6a..eb80198 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -814,7 +814,22 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
pci_set_region(hose->regions + pos, pci_addr, addr, size, type);
}
- /* Add a region for our local memory */
+ /* Add region(s) for our local memory */
+#ifdef CONFIG_NR_DRAM_BANKS
+ for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+ base = gd->bd->bi_dram[i].start;
+ size = gd->bd->bi_dram[i].size;
+ if (gd->pci_ram_top &&
+ gd->pci_ram_top >= base &&
+ gd->pci_ram_top < base + size)
+ size = gd->pci_ram_top - base;
+ if (size) {
+ pci_set_region(hose->regions + hose->region_count++,
+ base, base, size,
+ PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
+ }
+ }
+#else
size = gd->ram_size;
#ifdef CONFIG_SYS_SDRAM_BASE
base = CONFIG_SYS_SDRAM_BASE;
@@ -823,6 +838,7 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
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.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] driver: pci: Fix regions for local memory
2017-02-09 18:35 [U-Boot] [RFC PATCH] driver: pci: Fix regions for local memory York Sun
@ 2017-03-16 22:47 ` Simon Glass
2017-03-17 3:14 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2017-03-16 22:47 UTC (permalink / raw)
To: u-boot
On 9 February 2017 at 11:35, York Sun <york.sun@nxp.com> wrote:
> When adding local memory to PCI region, gd->ram_size is correct only
> if the memory is in one continuous block. In case memory is split
> into several banks, each bank should be added separately.
>
> Signed-off-by: York Sun <york.sun@nxp.com>
> CC: Simon Glass <sjg@chromium.org>
> ---
> It was spotted when I was rewriting the code to reserve secure memory
> and forgot to reduce gd->ram_size. PCIe resumes working after fixing
> gd->ram_size. For my case, the memory is split into two banks. So
> base + gd->ram_size is not in memory. I don't know how it worked before.
> This change seems reasonable without digging into PCI code.
>
> drivers/pci/pci-uclass.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
Acked-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] driver: pci: Fix regions for local memory
2017-03-16 22:47 ` Simon Glass
@ 2017-03-17 3:14 ` Simon Glass
2017-03-17 3:29 ` york sun
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2017-03-17 3:14 UTC (permalink / raw)
To: u-boot
Hi York,
On 16 March 2017 at 16:47, Simon Glass <sjg@chromium.org> wrote:
> On 9 February 2017 at 11:35, York Sun <york.sun@nxp.com> wrote:
>> When adding local memory to PCI region, gd->ram_size is correct only
>> if the memory is in one continuous block. In case memory is split
>> into several banks, each bank should be added separately.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>> CC: Simon Glass <sjg@chromium.org>
>> ---
>> It was spotted when I was rewriting the code to reserve secure memory
>> and forgot to reduce gd->ram_size. PCIe resumes working after fixing
>> gd->ram_size. For my case, the memory is split into two banks. So
>> base + gd->ram_size is not in memory. I don't know how it worked before.
>> This change seems reasonable without digging into PCI code.
>>
>> drivers/pci/pci-uclass.c | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>
> Acked-by: Simon Glass <sjg@chromium.org>
Unfortunately this breaks chromebook_link (x86).
Do you have any ideas or should I dig into it?
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] driver: pci: Fix regions for local memory
2017-03-17 3:14 ` Simon Glass
@ 2017-03-17 3:29 ` york sun
2017-03-17 11:43 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: york sun @ 2017-03-17 3:29 UTC (permalink / raw)
To: u-boot
On 03/16/2017 08:14 PM, Simon Glass wrote:
> Hi York,
>
> On 16 March 2017 at 16:47, Simon Glass <sjg@chromium.org> wrote:
>> On 9 February 2017 at 11:35, York Sun <york.sun@nxp.com> wrote:
>>> When adding local memory to PCI region, gd->ram_size is correct only
>>> if the memory is in one continuous block. In case memory is split
>>> into several banks, each bank should be added separately.
>>>
>>> Signed-off-by: York Sun <york.sun@nxp.com>
>>> CC: Simon Glass <sjg@chromium.org>
>>> ---
>>> It was spotted when I was rewriting the code to reserve secure memory
>>> and forgot to reduce gd->ram_size. PCIe resumes working after fixing
>>> gd->ram_size. For my case, the memory is split into two banks. So
>>> base + gd->ram_size is not in memory. I don't know how it worked before.
>>> This change seems reasonable without digging into PCI code.
>>>
>>> drivers/pci/pci-uclass.c | 18 +++++++++++++++++-
>>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>
> Unfortunately this breaks chromebook_link (x86).
>
> Do you have any ideas or should I dig into it?
>
Sorry I have no idea. If you can look into it, that will be great.
York
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] driver: pci: Fix regions for local memory
2017-03-17 3:29 ` york sun
@ 2017-03-17 11:43 ` Simon Glass
2017-03-17 14:54 ` york sun
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2017-03-17 11:43 UTC (permalink / raw)
To: u-boot
Hi York,
On 16 March 2017 at 21:29, york sun <york.sun@nxp.com> wrote:
> On 03/16/2017 08:14 PM, Simon Glass wrote:
>> Hi York,
>>
>> On 16 March 2017 at 16:47, Simon Glass <sjg@chromium.org> wrote:
>>> On 9 February 2017 at 11:35, York Sun <york.sun@nxp.com> wrote:
>>>> When adding local memory to PCI region, gd->ram_size is correct only
>>>> if the memory is in one continuous block. In case memory is split
>>>> into several banks, each bank should be added separately.
>>>>
>>>> Signed-off-by: York Sun <york.sun@nxp.com>
>>>> CC: Simon Glass <sjg@chromium.org>
>>>> ---
>>>> It was spotted when I was rewriting the code to reserve secure memory
>>>> and forgot to reduce gd->ram_size. PCIe resumes working after fixing
>>>> gd->ram_size. For my case, the memory is split into two banks. So
>>>> base + gd->ram_size is not in memory. I don't know how it worked before.
>>>> This change seems reasonable without digging into PCI code.
>>>>
>>>> drivers/pci/pci-uclass.c | 18 +++++++++++++++++-
>>>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>>>
>>>
>>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>> Unfortunately this breaks chromebook_link (x86).
>>
>> Do you have any ideas or should I dig into it?
>>
>
> Sorry I have no idea. If you can look into it, that will be great.
The problem is that x86 sets up PCI before relocation, thus before
gd->bd is available.
I think several changes are needed:
1. Something like:
bool use_dram_banks = false;
#ifdef CONFIG_NR_DRAM_BANKS
use_dram_banks = gd->bd != NULL;
#endif
if (use_dram_banks) {
your new code
} else {
old code
}
2. DRAM banks can have a size of 0, so check for that and skip if needed.
3. Check you don't overflow the size of the pci controller regions[] array.
Sorry I didn't notice this when reviewing it. It looked fine to me!
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] driver: pci: Fix regions for local memory
2017-03-17 11:43 ` Simon Glass
@ 2017-03-17 14:54 ` york sun
0 siblings, 0 replies; 6+ messages in thread
From: york sun @ 2017-03-17 14:54 UTC (permalink / raw)
To: u-boot
On 03/17/2017 04:43 AM, Simon Glass wrote:
> Hi York,
>
> On 16 March 2017 at 21:29, york sun <york.sun@nxp.com> wrote:
>> On 03/16/2017 08:14 PM, Simon Glass wrote:
>>> Hi York,
>>>
>>> On 16 March 2017 at 16:47, Simon Glass <sjg@chromium.org> wrote:
>>>> On 9 February 2017 at 11:35, York Sun <york.sun@nxp.com> wrote:
>>>>> When adding local memory to PCI region, gd->ram_size is correct only
>>>>> if the memory is in one continuous block. In case memory is split
>>>>> into several banks, each bank should be added separately.
>>>>>
>>>>> Signed-off-by: York Sun <york.sun@nxp.com>
>>>>> CC: Simon Glass <sjg@chromium.org>
>>>>> ---
>>>>> It was spotted when I was rewriting the code to reserve secure memory
>>>>> and forgot to reduce gd->ram_size. PCIe resumes working after fixing
>>>>> gd->ram_size. For my case, the memory is split into two banks. So
>>>>> base + gd->ram_size is not in memory. I don't know how it worked before.
>>>>> This change seems reasonable without digging into PCI code.
>>>>>
>>>>> drivers/pci/pci-uclass.c | 18 +++++++++++++++++-
>>>>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>>>>
>>>>
>>>> Acked-by: Simon Glass <sjg@chromium.org>
>>>
>>> Unfortunately this breaks chromebook_link (x86).
>>>
>>> Do you have any ideas or should I dig into it?
>>>
>>
>> Sorry I have no idea. If you can look into it, that will be great.
>
> The problem is that x86 sets up PCI before relocation, thus before
> gd->bd is available.
That explains it.
>
> I think several changes are needed:
>
> 1. Something like:
>
> bool use_dram_banks = false;
>
> #ifdef CONFIG_NR_DRAM_BANKS
> use_dram_banks = gd->bd != NULL;
> #endif
> if (use_dram_banks) {
> your new code
> } else {
> old code
> }
>
> 2. DRAM banks can have a size of 0, so check for that and skip if needed.
>
> 3. Check you don't overflow the size of the pci controller regions[] array.
>
> Sorry I didn't notice this when reviewing it. It looked fine to me!
>
Thanks for the explanation.
York
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-17 14:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-09 18:35 [U-Boot] [RFC PATCH] driver: pci: Fix regions for local memory York Sun
2017-03-16 22:47 ` Simon Glass
2017-03-17 3:14 ` Simon Glass
2017-03-17 3:29 ` york sun
2017-03-17 11:43 ` Simon Glass
2017-03-17 14:54 ` york sun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.