[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 05/14] xen/arm: PCI host bridge discovery within XEN on ARM





On 10/09/2021 12:22, Rahul Singh wrote:
Hi Julien

Hi Rahul,
On 7 Sep 2021, at 10:05 am, Julien Grall <julien@xxxxxxx> wrote:
On 19/08/2021 13:02, Rahul Singh wrote:
+#include <xen/rwlock.h>
+#include <xen/sched.h>
+#include <xen/vmap.h>
+
+/*
+ * List for all the pci host bridges.
+ */
+
+static LIST_HEAD(pci_host_bridges);
+
+static atomic_t domain_nr = ATOMIC_INIT(-1);
+
+bool dt_pci_parse_bus_range(struct dt_device_node *dev,
+                            struct pci_config_window *cfg)

Aside, "pci_config_window", the function is not Arm specific. Would it be possible to 
consider to introduce "struct resource" in Xen so this function can be moved in 
common/device_tree.c?

I can introduce the "struct resource” but I am not sure whether "struct 
resource” will be
useful later point in time. What I prefer as of now, we can have this function 
and we can
move this to common/device_tree.c once we have the requirement for "struct 
resource”.

TBH, I wasn't asking about using "struct resource". This was simply a suggestion how to make the code common.

What I am more interested is moving this function in common code so it can be re-used by RISC-v in the future. I don't particularly mind whether this is using "struct resource" or a different structure.

diff --git a/xen/arch/arm/pci/pci-host-generic.c 
b/xen/arch/arm/pci/pci-host-generic.c
new file mode 100644
index 0000000000..13d0f7f999
--- /dev/null
+++ b/xen/arch/arm/pci/pci-host-generic.c
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2021 Arm Ltd.
+ *
+ * Based on Linux drivers/pci/controller/pci-host-common.c
+ * Based on Linux drivers/pci/controller/pci-host-generic.c
+ * Copyright (C) 2014 ARM Limited Will Deacon <will.deacon@xxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <asm/device.h>
+#include <xen/pci.h>
+#include <asm/pci.h>
+
+static const struct dt_device_match gen_pci_dt_match[] = {
+    { .compatible = "pci-host-ecam-generic" },
+    { },
+};
+
+static int gen_pci_dt_init(struct dt_device_node *dev, const void *data)
+{
+    const struct dt_device_match *of_id;
+
+    of_id = dt_match_node(gen_pci_dt_match, dev->dev.of_node);

This seems to be a bit pointless to me as you already know the compatible 
(there is only one possible...).
  As of now we are only implementing the "pci-host-ecam-generic” compatible 
PCI, but in future we might
need to implement the other compatible like Linux see as below.

https://elixir.bootlin.com/linux/latest/source/drivers/pci/controller/pci-host-generic.c#L59

Right... You know the node matched and you don't need any data. So this is a bit pointless to have to walk again the structure for just printing the compatible that matched.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.