diff mbox

pci: add kernel config option for disabling common PCI quirks

Message ID 1482306784-29224-1-git-send-email-john@phrozen.org (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

John Crispin Dec. 21, 2016, 7:53 a.m. UTC
From: Gabor Juhos <juhosg@openwrt.org>

These quirks do not effect small small plastic routers. These routers tend
to have very little flash space. This patch adds a new option that allows
us to build a kernel without including the common quirks, thus saving us
valuable flash space.

When building an image for a MIPS/Ralink router with this option enabled
it will reduce the size of an uncompressed uImage by 12KB.

Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
Hi Bjorn,

This patch has been lingering inside the OpenWrt for some years. I am not
sure if this is the best way to remove the quirks from the build. Let me
know if you prefer a different way of solving this.

	John

 drivers/pci/Kconfig  |   10 ++++++++++
 drivers/pci/quirks.c |    6 ++++++
 2 files changed, 16 insertions(+)

Comments

Lukas Wunner Dec. 21, 2016, 10:19 a.m. UTC | #1
On Wed, Dec 21, 2016 at 08:53:04AM +0100, John Crispin wrote:
> From: Gabor Juhos <juhosg@openwrt.org>
> 
> These quirks do not effect small small plastic routers. These routers tend
> to have very little flash space. This patch adds a new option that allows
> us to build a kernel without including the common quirks, thus saving us
> valuable flash space.
> 
> When building an image for a MIPS/Ralink router with this option enabled
> it will reduce the size of an uncompressed uImage by 12KB.

There's already a PCI_QUIRKS config option defined in init/Kconfig,
why isn't this sufficient?  Do you want *some* quirks but not others?

Thanks,

Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Dec. 21, 2016, 12:22 p.m. UTC | #2
On Wed, Dec 21, 2016 at 08:53:04AM +0100, John Crispin wrote:
> This patch has been lingering inside the OpenWrt for some years. I am not
> sure if this is the best way to remove the quirks from the build. Let me
> know if you prefer a different way of solving this.

As a start negative options (_DISABLE_) are very confusing, I would
recommend to turn this into a postive option with a default y.

On what grounds did you decide which quirks to keep and which to
remove?
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Crispin Dec. 21, 2016, 1:11 p.m. UTC | #3
On 21/12/2016 13:22, Christoph Hellwig wrote:
> On Wed, Dec 21, 2016 at 08:53:04AM +0100, John Crispin wrote:
>> This patch has been lingering inside the OpenWrt for some years. I am not
>> sure if this is the best way to remove the quirks from the build. Let me
>> know if you prefer a different way of solving this.
> 
> As a start negative options (_DISABLE_) are very confusing, I would
> recommend to turn this into a postive option with a default y.
> 
> On what grounds did you decide which quirks to keep and which to
> remove?
> 

Hi Christoph,

I can turn it into an enable patch that is selected by default.

The current patch disables all those quirks that are used for x86/PC
style machines and hence are not required in the embedded world.

	John
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Dec. 21, 2016, 2:26 p.m. UTC | #4
On Wed, Dec 21, 2016 at 02:11:25PM +0100, John Crispin wrote:
> I can turn it into an enable patch that is selected by default.
> 
> The current patch disables all those quirks that are used for x86/PC
> style machines and hence are not required in the embedded world.

Maybe we'll just need to reorganize the quirks so that most of them
arch in arch code or the affected drivers?
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Crispin Dec. 22, 2016, 5:54 a.m. UTC | #5
On 21/12/2016 15:26, Christoph Hellwig wrote:
> On Wed, Dec 21, 2016 at 02:11:25PM +0100, John Crispin wrote:
>> I can turn it into an enable patch that is selected by default.
>>
>> The current patch disables all those quirks that are used for x86/PC
>> style machines and hence are not required in the embedded world.
> 
> Maybe we'll just need to reorganize the quirks so that most of them
> arch in arch code or the affected drivers?
> 

Hi Christoph

to be honest i have no opinion on this. I am currently trying to reduce
the amount of patches that we have inside the LEDE tree. the patches
were written by other people and then dumped on us. obviously i am
interested to get this upstream with the least amount of effort. I am
quite aware though that some patches will need an overhaul to be
applicable for upstream. its not really my call if it is enough to make
this an enable patch and review the quirks enabled by it or if the code
needs to be moved.

	John
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas Dec. 22, 2016, 6:45 p.m. UTC | #6
On Thu, Dec 22, 2016 at 06:54:51AM +0100, John Crispin wrote:
> On 21/12/2016 15:26, Christoph Hellwig wrote:
> > On Wed, Dec 21, 2016 at 02:11:25PM +0100, John Crispin wrote:
> >> I can turn it into an enable patch that is selected by default.
> >>
> >> The current patch disables all those quirks that are used for x86/PC
> >> style machines and hence are not required in the embedded world.
> > 
> > Maybe we'll just need to reorganize the quirks so that most of them
> > arch in arch code or the affected drivers?
> 
> to be honest i have no opinion on this. I am currently trying to reduce
> the amount of patches that we have inside the LEDE tree. the patches
> were written by other people and then dumped on us. obviously i am
> interested to get this upstream with the least amount of effort. I am
> quite aware though that some patches will need an overhaul to be
> applicable for upstream. its not really my call if it is enough to make
> this an enable patch and review the quirks enabled by it or if the code
> needs to be moved.

We already have CONFIG_PCI_QUIRKS, which enables everything in
drivers/pci/quirks.c.  That file should contain quirks for devices
that may appear on any architecture, e.g., things on plug-in cards.

Quirks that are only applicable to one arch should be in the arch
directory, e.g., in arch/x86/pci/fixup.c.

If drivers/pci/quirks.c contains arch-specific quirks, we should move
those to the appropriate arch directory.

It looks like arch/x86/pci/fixup.c is currently compiled
unconditionally (on x86 with PCI), but we should probably make it
dependent on CONFIG_PCI_QUIRKS, since I think the infrastructure that
*calls* those quirks is only present when CONFIG_PCI_QUIRKS=y.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 6555eb7..967bcd5 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -71,6 +71,16 @@  config XEN_PCIDEV_FRONTEND
           The PCI device frontend driver allows the kernel to import arbitrary
           PCI devices from a PCI backend to support PCI driver domains.
 
+config PCI_DISABLE_COMMON_QUIRKS
+	bool "PCI disable common quirks"
+	depends on PCI
+        default n
+	help
+	  Say Y here if you do not wish to include the common quirks inside
+	  your kernel This is usefull for devices with scarce memory.
+
+	  If you don't know what to do here, say N.
+
 config HT_IRQ
 	bool "Interrupts on hypertransport devices"
 	default y
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index c232729..afe181e 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -41,6 +41,7 @@  static void quirk_mmio_always_on(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
 				PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);
 
+#ifndef CONFIG_PCI_DISABLE_COMMON_QUIRKS
 /* The Mellanox Tavor device gives false positive parity errors
  * Mark this device with a broken_parity_status, to allow
  * PCI scanning code to "skip" this now blacklisted device.
@@ -3015,6 +3016,7 @@  static void quirk_intel_mc_errata(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x65f9, quirk_intel_mc_errata);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x65fa, quirk_intel_mc_errata);
 
+#endif /* !CONFIG_PCI_DISABLE_COMMON_QUIRKS */
 
 /*
  * Ivytown NTB BAR sizes are misreported by the hardware due to an erratum.  To
@@ -3071,6 +3073,8 @@  static void fixup_debug_report(struct pci_dev *dev, ktime_t calltime,
 	}
 }
 
+#ifndef CONFIG_PCI_DISABLE_COMMON_QUIRKS
+
 /*
  * Some BIOS implementations leave the Intel GPU interrupts enabled,
  * even though no one is handling them (f.e. i915 driver is never loaded).
@@ -3105,6 +3109,8 @@  static void disable_igfx_irq(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x010a, disable_igfx_irq);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0152, disable_igfx_irq);
 
+#endif /* !CONFIG_PCI_DISABLE_COMMON_QUIRKS */
+
 /*
  * PCI devices which are on Intel chips can skip the 10ms delay
  * before entering D3 mode.