diff mbox series

[02/17] x86: pci: Allow binding of some devices before relocation

Message ID 20210407043228.2268429-3-sjg@chromium.org
State Superseded
Delegated to: Bin Meng
Headers show
Series misc: Some more misc patches | expand

Commit Message

Simon Glass April 7, 2021, 4:32 a.m. UTC
At present only bridge devices are bound before relocation, to save space
in pre-relocation memory. In some cases we do actually want to bind a
device, e.g. because it provides the console UART. Add a devicetree
binding to support this.

Use the PCI_VENDEV() macro to encode the cell value. This is present in
U-Boot but not used, so move it to the binding header-file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 doc/device-tree-bindings/pci/x86-pci.txt |  7 ++++-
 drivers/pci/pci-uclass.c                 | 33 +++++++++++++++++++++++-
 include/dt-bindings/pci/pci.h            | 12 +++++++++
 include/pci.h                            |  1 -
 4 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 include/dt-bindings/pci/pci.h

Comments

Bin Meng April 8, 2021, 2:16 a.m. UTC | #1
Hi Simon,

On Wed, Apr 7, 2021 at 12:32 PM Simon Glass <sjg@chromium.org> wrote:
>
> At present only bridge devices are bound before relocation, to save space
> in pre-relocation memory. In some cases we do actually want to bind a
> device, e.g. because it provides the console UART. Add a devicetree
> binding to support this.
>
> Use the PCI_VENDEV() macro to encode the cell value. This is present in
> U-Boot but not used, so move it to the binding header-file.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  doc/device-tree-bindings/pci/x86-pci.txt |  7 ++++-
>  drivers/pci/pci-uclass.c                 | 33 +++++++++++++++++++++++-
>  include/dt-bindings/pci/pci.h            | 12 +++++++++
>  include/pci.h                            |  1 -
>  4 files changed, 50 insertions(+), 3 deletions(-)
>  create mode 100644 include/dt-bindings/pci/pci.h
>
> diff --git a/doc/device-tree-bindings/pci/x86-pci.txt b/doc/device-tree-bindings/pci/x86-pci.txt
> index 95e370b3e72..cf4e5ed595a 100644
> --- a/doc/device-tree-bindings/pci/x86-pci.txt
> +++ b/doc/device-tree-bindings/pci/x86-pci.txt
> @@ -20,6 +20,10 @@ For PCI devices the following optional property is available:
>         output to be lost. This should not generally be used in production code,
>         although it is often harmless.
>
> +- u-boot,pci-pre-reloc : List of vendor/device IDs to bind before relocation, even
> +       if they are not bridges. This is useful if the device is needed (e.g. a
> +       UART). The format is 0xvvvvdddd where d is the device ID and v is the
> +       vendor ID.

Can we reuse "u-boot,dm-pre-reloc" to do such thing?

The following is an example from arch/x86/dts/crownbay.dts

                                pciuart0: uart@a,1 {
                                        compatible = "pci8086,8811.00",
                                                        "pci8086,8811",
                                                        "pciclass,070002",
                                                        "pciclass,0700",
                                                        "ns16550";
                                        u-boot,dm-pre-reloc;
                                        reg = <0x00025100 0x0 0x0 0x0 0x0
                                               0x01025110 0x0 0x0 0x0 0x0>;
                                        reg-shift = <0>;
                                        clock-frequency = <1843200>;
                                        current-speed = <115200>;
                                };

Regards,
Bin
Simon Glass April 24, 2021, 4:56 a.m. UTC | #2
Hi Bin,

On Thu, 8 Apr 2021 at 14:17, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, Apr 7, 2021 at 12:32 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > At present only bridge devices are bound before relocation, to save space
> > in pre-relocation memory. In some cases we do actually want to bind a
> > device, e.g. because it provides the console UART. Add a devicetree
> > binding to support this.
> >
> > Use the PCI_VENDEV() macro to encode the cell value. This is present in
> > U-Boot but not used, so move it to the binding header-file.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  doc/device-tree-bindings/pci/x86-pci.txt |  7 ++++-
> >  drivers/pci/pci-uclass.c                 | 33 +++++++++++++++++++++++-
> >  include/dt-bindings/pci/pci.h            | 12 +++++++++
> >  include/pci.h                            |  1 -
> >  4 files changed, 50 insertions(+), 3 deletions(-)
> >  create mode 100644 include/dt-bindings/pci/pci.h
> >
> > diff --git a/doc/device-tree-bindings/pci/x86-pci.txt b/doc/device-tree-bindings/pci/x86-pci.txt
> > index 95e370b3e72..cf4e5ed595a 100644
> > --- a/doc/device-tree-bindings/pci/x86-pci.txt
> > +++ b/doc/device-tree-bindings/pci/x86-pci.txt
> > @@ -20,6 +20,10 @@ For PCI devices the following optional property is available:
> >         output to be lost. This should not generally be used in production code,
> >         although it is often harmless.
> >
> > +- u-boot,pci-pre-reloc : List of vendor/device IDs to bind before relocation, even
> > +       if they are not bridges. This is useful if the device is needed (e.g. a
> > +       UART). The format is 0xvvvvdddd where d is the device ID and v is the
> > +       vendor ID.
>
> Can we reuse "u-boot,dm-pre-reloc" to do such thing?
>
> The following is an example from arch/x86/dts/crownbay.dts
>
>                                 pciuart0: uart@a,1 {
>                                         compatible = "pci8086,8811.00",
>                                                         "pci8086,8811",
>                                                         "pciclass,070002",
>                                                         "pciclass,0700",
>                                                         "ns16550";
>                                         u-boot,dm-pre-reloc;
>                                         reg = <0x00025100 0x0 0x0 0x0 0x0
>                                                0x01025110 0x0 0x0 0x0 0x0>;
>                                         reg-shift = <0>;
>                                         clock-frequency = <1843200>;
>                                         current-speed = <115200>;
>                                 };

Yes but only if we have the correct PCI BDF for it. But the goal of
the coreboot build is to be able to run on any hardware. So if we
require a devicetree (and coreboot doesn't supply it) then it cannot
work. We would need to have many devicetree files, one for each board.
Perhaps that will happen anyway, but for now I am trying to do things
automatically.

Of course this would be a lot easier if coreboot just used devicetree,
but that doesn't seem to be on the cards.

Regards,
Simon
diff mbox series

Patch

diff --git a/doc/device-tree-bindings/pci/x86-pci.txt b/doc/device-tree-bindings/pci/x86-pci.txt
index 95e370b3e72..cf4e5ed595a 100644
--- a/doc/device-tree-bindings/pci/x86-pci.txt
+++ b/doc/device-tree-bindings/pci/x86-pci.txt
@@ -20,6 +20,10 @@  For PCI devices the following optional property is available:
 	output to be lost. This should not generally be used in production code,
 	although it is often harmless.
 
+- u-boot,pci-pre-reloc : List of vendor/device IDs to bind before relocation, even
+	if they are not bridges. This is useful if the device is needed (e.g. a
+	UART). The format is 0xvvvvdddd where d is the device ID and v is the
+	vendor ID.
 
 Example:
 
@@ -32,7 +36,8 @@  pci {
 		0x42000000 0x0 0xb0000000 0xb0000000 0 0x10000000
 		0x01000000 0x0 0x1000 0x1000 0 0xefff>;
 	u-boot,skip-auto-config-until-reloc;
-
+	u-boot,pci-pre-reloc = <
+		PCI_VENDEV(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2)>;
 
 	serial: serial@18,2 {
 		reg = <0x0200c210 0 0 0 0>;
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index dffe5297944..ec3797746ba 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -19,6 +19,7 @@ 
 #if defined(CONFIG_X86) && defined(CONFIG_HAVE_FSP)
 #include <asm/fsp/fsp_support.h>
 #endif
+#include <dt-bindings/pci/pci.h>
 #include <linux/delay.h>
 #include "pci_internal.h"
 
@@ -673,6 +674,34 @@  static bool pci_match_one_id(const struct pci_device_id *id,
 	return false;
 }
 
+/**
+ * pci_need_device_pre_reloc() - Check if a device should be bound
+ *
+ * This checks a list of vendor/device-ID values indicating devices that should
+ * be bound before relocation.
+ *
+ * @bus: Bus to check
+ * @vendor: Vendor ID to check
+ * @device: Device ID to check
+ * @return true if the vendor/device is in the list, false if not
+ */
+static bool pci_need_device_pre_reloc(struct udevice *bus, uint vendor,
+				      uint device)
+{
+	u32 vendev;
+	int index;
+
+	for (index = 0;
+	     !dev_read_u32_index(bus, "u-boot,pci-pre-reloc", index,
+				 &vendev);
+	     index++) {
+		if (vendev == PCI_VENDEV(vendor, device))
+			return true;
+	}
+
+	return false;
+}
+
 /**
  * pci_find_and_bind_driver() - Find and bind the right PCI driver
  *
@@ -761,7 +790,9 @@  static int pci_find_and_bind_driver(struct udevice *parent,
 	 * precious memory space as on some platforms as that space is pretty
 	 * limited (ie: using Cache As RAM).
 	 */
-	if (!(gd->flags & GD_FLG_RELOC) && !bridge)
+	if (!(gd->flags & GD_FLG_RELOC) && !bridge &&
+	    !pci_need_device_pre_reloc(parent, find_id->vendor,
+				       find_id->device))
 		return log_msg_ret("notbr", -EPERM);
 
 	/* Bind a generic driver so that the device can be used */
diff --git a/include/dt-bindings/pci/pci.h b/include/dt-bindings/pci/pci.h
new file mode 100644
index 00000000000..e7290277b90
--- /dev/null
+++ b/include/dt-bindings/pci/pci.h
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * This header provides common constants for PCI bindings.
+ */
+
+#ifndef _DT_BINDINGS_PCI_PCI_H
+#define _DT_BINDINGS_PCI_PCI_H
+
+/* Encode a vendor and device ID into a single cell */
+#define PCI_VENDEV(v, d)	(((v) << 16) | (d))
+
+#endif /* _DT_BINDINGS_PCI_PCI_H */
diff --git a/include/pci.h b/include/pci.h
index f2dfbda5b08..fe7064b2a44 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -578,7 +578,6 @@  typedef int pci_dev_t;
 #define PCI_MASK_BUS(bdf)	((bdf) & 0xffff)
 #define PCI_ADD_BUS(bus, devfn)	(((bus) << 16) | (devfn))
 #define PCI_BDF(b, d, f)	((b) << 16 | PCI_DEVFN(d, f))
-#define PCI_VENDEV(v, d)	(((v) << 16) | (d))
 #define PCI_ANY_ID		(~0)
 
 /* Convert from Linux format to U-Boot format */