[Skiboot] [PATCH 1/2] pci: Add a framework for quirks

[Gavin Shan]

On Fri, Feb 17, 2017 at 04:40:18PM +1100, Benjamin Herrenschmidt wrote:
>From: Russell Currey <ruscur at russell.cc>
>
>In future we may want to be able to do fixups for specific PCI devices in
>skiboot, so add a small framework for doing this.
>
>Signed-off-by: Russell Currey <ruscur at russell.cc>
>Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
>---
> core/Makefile.inc   |  2 +-
> core/pci-quirk.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
> core/pci.c          |  3 +++
> include/pci-quirk.h | 36 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 82 insertions(+), 1 deletion(-)
> create mode 100644 core/pci-quirk.c
> create mode 100644 include/pci-quirk.h
>
>diff --git a/core/Makefile.inc b/core/Makefile.inc
>index 2167044..734fb1b 100644
>--- a/core/Makefile.inc
>+++ b/core/Makefile.inc
>@@ -8,7 +8,7 @@ CORE_OBJS += pci-opal.o fast-reboot.o device.o exceptions.o trace.o affinity.o
> CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o
> CORE_OBJS += console-log.o ipmi.o time-utils.o pel.o pool.o errorlog.o
> CORE_OBJS += timer.o i2c.o rtc.o flash.o sensor.o ipmi-opal.o
>-CORE_OBJS += flash-subpartition.o bitmap.o buddy.o
>+CORE_OBJS += flash-subpartition.o bitmap.o buddy.o pci-quirk.o
> 
> ifeq ($(SKIBOOT_GCOV),1)
> CORE_OBJS += gcov-profiling.o
>diff --git a/core/pci-quirk.c b/core/pci-quirk.c
>new file mode 100644
>index 0000000..708f07f
>--- /dev/null
>+++ b/core/pci-quirk.c
>@@ -0,0 +1,42 @@
>+/* Copyright 2016 IBM Corp.
>+ *
>+ * Licensed under the Apache License, Version 2.0 (the "License");
>+ * you may not use this file except in compliance with the License.
>+ * You may obtain a copy of the License at
>+ *
>+ * 	http://www.apache.org/licenses/LICENSE-2.0
>+ *
>+ * Unless required by applicable law or agreed to in writing, software
>+ * distributed under the License is distributed on an "AS IS" BASIS,
>+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>+ * implied.
>+ * See the License for the specific language governing permissions and
>+ * limitations under the License.
>+ */
>+
>+#include <skiboot.h>
>+#include <pci.h>
>+#include <pci-quirk.h>
>+#include <ast.h>
>+
>+/* Quirks are: {fixup function, vendor ID, (device ID or PCI_ANY_ID)} */
>+static const struct pci_quirk quirk_table[] = {
>+	{0}
>+};
>+
>+void pci_handle_quirk(struct phb *phb,
>+		      struct pci_device *pd,
>+		      struct dt_node *np,
>+		      uint16_t vendor_id,
>+		      uint16_t device_id)
>+{
>+	const struct pci_quirk *quirks = quirk_table;
>+
>+	while (quirks->vendor_id) {
>+		if (vendor_id == quirks->vendor_id &&
>+		    (quirks->device_id == PCI_ANY_ID ||
>+		     device_id == quirks->device_id))
>+			quirks->fixup(phb, pd, np);
>+		quirks++;
>+	}
>+}
>diff --git a/core/pci.c b/core/pci.c
>index 0a9f8cd..7e764a0 100644
>--- a/core/pci.c
>+++ b/core/pci.c
>@@ -19,6 +19,7 @@
> #include <pci.h>
> #include <pci-cfg.h>
> #include <pci-slot.h>
>+#include <pci-quirk.h>
> #include <timebase.h>
> #include <device.h>
> #include <fsp.h>
>@@ -1402,6 +1403,8 @@ static void pci_add_one_device_node(struct phb *phb,
> 	if (intpin)
> 		dt_add_property_cells(np, "interrupts", intpin);
> 
>+	pci_handle_quirk(phb, pd, np, vdid & 0xffff, vdid >> 16);
>+

The device node, vendor and device ID have cached to the PCI device
(struct pci_device). So we needn't pass them to pci_handle_quirk(),
they can be picked from PCI device instead, if we won't pass device
node or vdid other than those are associated with the PCI device.

Also, the memory BAR of one specific PCI device isn't populted. It
means we can't fixup the registers in memory BAR and have to do that
in kernel. With this patch applied, we're going to have some fixups
in skiboot and others in kernel. It's prone to be out of sychronization
between skiboot and kernel. So it's possible to do the quirks in kernel
only?

> 	/* XXX FIXME: Add a few missing ones such as
> 	 *
> 	 *  - devsel-speed (!express)
>diff --git a/include/pci-quirk.h b/include/pci-quirk.h
>new file mode 100644
>index 0000000..c2ceb55
>--- /dev/null
>+++ b/include/pci-quirk.h
>@@ -0,0 +1,36 @@
>+/* Copyright 2016 IBM Corp.
>+ *
>+ * Licensed under the Apache License, Version 2.0 (the "License");
>+ * you may not use this file except in compliance with the License.
>+ * You may obtain a copy of the License at
>+ *
>+ * 	http://www.apache.org/licenses/LICENSE-2.0
>+ *
>+ * Unless required by applicable law or agreed to in writing, software
>+ * distributed under the License is distributed on an "AS IS" BASIS,
>+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>+ * implied.
>+ * See the License for the specific language governing permissions and
>+ * limitations under the License.
>+ */
>+
>+#ifndef __PCI_QUIRK_H
>+#define __PCI_QUIRK_H
>+
>+#include <pci.h>
>+
>+#define PCI_ANY_ID 0xFFFF
>+
>+struct pci_quirk {
>+	void (*fixup)(struct phb *, struct pci_device *, struct dt_node *);

Similar to above, we needn't pass device node which can be picked from
the PCI device instance.

>+	uint16_t vendor_id;
>+	uint16_t device_id;
>+};
>+
>+void pci_handle_quirk(struct phb *phb,
>+		      struct pci_device *pd,
>+		      struct dt_node *np,
>+		      uint16_t vendor_id,
>+		      uint16_t device_id);
>+
>+#endif /* __PCI_QUIRK_H */

Thanks,
Gavin