Hi Marek,

On 30.07.20 17:16, Stefan Roese wrote:
Hi Simon,

On 28.07.20 21:01, Simon Glass wrote:
Hi Stefan,

On Fri, 24 Jul 2020 at 04:09, Stefan Roese <s...@denx.de> wrote:

Instead of using a fixed length pre-allocated array of regions, this
patch moves to dynamically allocating the regions based on the number
of available regions plus the necessary regions for DRAM banks.

Since MAX_PCI_REGIONS is not needed any more, its removed completely
with this patch.

Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Bin Meng <bmeng...@gmail.com>
Cc: Thierry Reding <tred...@nvidia.com>
Cc: Marek Vasut <marek.vasut+rene...@gmail.com>

---

Changes in v1:
- New patch, replaces increase of MAX_PCI_REGIONS to 10

  board/renesas/rcar-common/common.c | 10 +++++-----
  drivers/pci/pci-uclass.c           | 14 ++++++++------
  include/pci.h                      |  4 +---
  3 files changed, 14 insertions(+), 14 deletions(-)


Can you please split out the generic PCI changes into a separate patch?

Okay, will do.

diff --git a/board/renesas/rcar-common/common.c b/board/renesas/rcar-common/common.c
index 83dd288847..83440c11ef 100644
--- a/board/renesas/rcar-common/common.c
+++ b/board/renesas/rcar-common/common.c
@@ -58,12 +58,12 @@ int ft_board_setup(void *blob, struct bd_info *bd)
         uclass_foreach_dev(dev, uc) {
                 struct pci_controller hose = { 0 };

-               for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
-                       if (hose.region_count == MAX_PCI_REGIONS) {
-                               printf("maximum number of regions parsed, aborting\n");
-                               break;
-                       }
+               /* Dynamically allocate the regions array */

Why is the driver allocating this? Shouldn't it happen in pci-uclass.c ?

I'm not sure, if the PCI init code has been run before this function
is called in all cases. Marek, do you have an answer on this?

Marek, again could you please take a look and let us know, why this
code is necessary here? And if it is okay to allocate the structs here
as well then?

Thanks,
Stefan


Thanks,
Stefan


+               hose.regions = (struct pci_region *)
+                       calloc(1, CONFIG_NR_DRAM_BANKS *
+                              sizeof(struct pci_region));

+               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,
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 69fb46d3f4..0fbbef70c8 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -874,6 +874,7 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
         struct bd_info *bd = gd->bd;
         int cells_per_record;
         const u32 *prop;
+       int max_regions;
         int len;
         int i;

@@ -893,7 +894,13 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
         hose->region_count = 0;
         debug("%s: len=%d, cells_per_record=%d\n", __func__, len,
               cells_per_record);
-       for (i = 0; i < MAX_PCI_REGIONS; i++, len -= cells_per_record) {
+
+       /* Dynamically allocate the regions array */
+       max_regions = len / cells_per_record + CONFIG_NR_DRAM_BANKS;
+       hose->regions = (struct pci_region *)
+               calloc(1, max_regions * sizeof(struct pci_region));
+
+       for (i = 0; i < max_regions; i++, len -= cells_per_record) {
                 u64 pci_addr, addr, size;
                 int space_code;
                 u32 flags;
@@ -943,11 +950,6 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
                 return;

         for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
-               if (hose->region_count == MAX_PCI_REGIONS) {
-                       pr_err("maximum number of regions parsed, aborting\n");
-                       break;
-               }
-
                 if (bd->bi_dram[i].size) {
                         pci_set_region(hose->regions + hose->region_count++,
                                        bd->bi_dram[i].start,
diff --git a/include/pci.h b/include/pci.h
index 281f353916..53f1386fd4 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -590,8 +590,6 @@ extern void pci_cfgfunc_do_nothing(struct pci_controller* hose, pci_dev_t dev,   extern void pci_cfgfunc_config_device(struct pci_controller* hose, pci_dev_t dev,
                                       struct pci_config_table *);

-#define MAX_PCI_REGIONS                7
-
  #define INDIRECT_TYPE_NO_PCIE_LINK     1

  /**
@@ -632,7 +630,7 @@ struct pci_controller {
          * for PCI controllers and a separate UCLASS (or perhaps
          * UCLASS_PCI_GENERIC) is used for bridges.
          */
-       struct pci_region regions[MAX_PCI_REGIONS];
+       struct pci_region *regions;
         int region_count;

         struct pci_config_table *config_table;
--
2.27.0



Viele Grüße,
Stefan



Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

Reply via email to