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

[Gavin Shan]

On Fri, Feb 24, 2017 at 04:36:45PM +1100, Russell Currey wrote:
>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.
>
>This is not intended for the same purposes as quirks in the Linux kernel,
>as the PCI devices that quirks can match for in skiboot are not properly
>configured.  This is intended to enable having a custom path to make
>changes that don't directly interact with the PCI device, for example
>adding device tree entries.
>
>Signed-off-by: Russell Currey <ruscur at russell.cc>
>Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>

Reviewed-by: Gavin Shan <gwshan at linux.vnet.ibm.com>

With minor comments fixed as below, or to be fixed later. Not a
big deal.

>---
>v2 [ruscur]: Drop dt_node from pci_handle_quirk() thanks to Gavin
>---
> core/Makefile.inc   |  2 +-
> core/pci-quirk.c    | 41 +++++++++++++++++++++++++++++++++++++++++
> core/pci.c          |  3 +++
> include/pci-quirk.h | 35 +++++++++++++++++++++++++++++++++++
> 4 files changed, 80 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 ae3c2979..b09c30c0 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 00000000..93f613eb
>--- /dev/null
>+++ b/core/pci-quirk.c
>@@ -0,0 +1,41 @@
>+/* 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,
>+		      uint16_t vendor_id,
>+		      uint16_t device_id)
>+{

The vendor_id and device_id can be got from struct pci_device::vdid
as mentioned in the comments for v1. So you needn't pass them via
arguments, but nothing is wrong here.

>+	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);
>+		quirks++;
>+	}
>+}
>diff --git a/core/pci.c b/core/pci.c
>index 9889dbf8..fe4e73d8 100644
>--- a/core/pci.c
>+++ b/core/pci.c
>@@ -20,6 +20,7 @@
> #include <pci-cfg.h>
> #include <pci-iov.h>
> #include <pci-slot.h>
>+#include <pci-quirk.h>
> #include <timebase.h>
> #include <device.h>
> #include <fsp.h>
>@@ -1430,6 +1431,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, vdid & 0xffff, vdid >> 16);
>+
> 	/* 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 00000000..c7669733
>--- /dev/null
>+++ b/include/pci-quirk.h
>@@ -0,0 +1,35 @@
>+/* Copyright 2016 IBM Corp.

2017 maybe?

>+ *
>+ * 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 *);
>+	uint16_t vendor_id;
>+	uint16_t device_id;
>+};
>+
>+void pci_handle_quirk(struct phb *phb,
>+		      struct pci_device *pd,
>+		      uint16_t vendor_id,
>+		      uint16_t device_id);
>+
>+#endif /* __PCI_QUIRK_H */
>-- 
>2.11.1
>