On Wednesday 14 October 2015 18:01:46 Ley Foon Tan wrote:

On Wed, Oct 14, 2015 at 5:36 PM, Arnd Bergmann [off-list ref] wrote:

quoted

On Wednesday 14 October 2015 17:28:45 Ley Foon Tan wrote:

quoted

On Wed, Oct 14, 2015 at 5:09 PM, Arnd Bergmann [off-list ref] wrote:

Could we perhaps have a helper function that lets us register
fixups dynamically?

quoted

The linker script keeps all pci fixup callbacks in pci fixup regions
during kernel compile time. So, it needs to be builtin module. Do you
know any way we can update those fixup regions?

The only method I'm aware of at the moment is move the fixups to
drivers/pci/quirks.c and enclose them in an #ifdef if you want them
to not appear in kernels that don't support your SoC.

By looking at the drivers/pci/quirks.c, it looks like it is mainly for
the pci endpoint devices.
Fixups for host controller are in the driver itself.

But if it's for the host itself, there are usually other ways to
do this without needing a fixup: you already have the device structure
present in the driver, so you should just be able to modify it there.


I'm looking at the code in your fixups now:

+static void altera_pcie_retrain(struct pci_dev *dev)
+{
+       u16 linkcap, linkstat;
+
+       /*
+        * Set the retrain bit if the PCIe rootport support > 2.5GB/s, but
+        * current speed is 2.5 GB/s.
+        */
+       pcie_capability_read_word(dev, PCI_EXP_LNKCAP, &linkcap);
+
+       if ((linkcap & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
+               return;
+
+       pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &linkstat);
+       if ((linkstat & PCI_EXP_LNKSTA_CLS) == PCI_EXP_LNKSTA_CLS_2_5GB)
+               pcie_capability_set_word(dev, PCI_EXP_LNKCTL,
+                                        PCI_EXP_LNKCTL_RL);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID, altera_pcie_retrain);

This looks related to the code in pci_set_bus_speed(). What is
missing from that code?

+static void altera_pcie_fixup_res(struct pci_dev *dev)
+{
+       /*
+        * Prevent enumeration of root port.
+        */
+       if (!dev->bus->parent && dev->devfn == 0) {
+               int i;
+
+               for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+                       dev->resource[i].start = 0;
+                       dev->resource[i].end   = 0;
+                       dev->resource[i].flags   = 0;
+               }
+       }
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID,
+                        altera_pcie_fixup_res);

This seems really odd, too. Why is this needed?
I think I've seen similar code in other host drivers, so
it might be time to teach the PCI core about this kind of
device.

	Arnd