Message ID | 20170224053646.6496-1-ruscur@russell.cc |
---|---|
State | Accepted |
Headers | show |
On 24/02/17 16:36, 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@russell.cc> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > v2 [ruscur]: Drop dt_node from pci_handle_quirk() thanks to Gavin Looks fairly sensible to me. Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > +#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} This adds a slightly silly sparse warning: core/pci-quirk.c:71:10: warning: Using plain integer as NULL pointer
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@russell.cc> >Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Reviewed-by: Gavin Shan <gwshan@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 >
Russell Currey <ruscur@russell.cc> writes: > 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@russell.cc> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > v2 [ruscur]: Drop dt_node from pci_handle_quirk() thanks to Gavin > --- Series merged to master as of fcb5114c426119dedb0226137d4a3468462f979f
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) +{ + 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. + * + * 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 */