linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <bhelgaas@google.com>
To: Gavin Shan <gwshan@linux.vnet.ibm.com>
Cc: clsoto@us.ibm.com, benh@kernel.crashing.org,
	linux-pci@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	mpe@ellerman.id.au
Subject: [PATCH v4 7/7] PCI: Add comments about ROM BAR updating
Date: Mon, 28 Nov 2016 22:16:48 -0600	[thread overview]
Message-ID: <20161129041648.21453.94596.stgit@bhelgaas-glaptop.roam.corp.google.com> (raw)
In-Reply-To: <20161129035218.21453.93986.stgit@bhelgaas-glaptop.roam.corp.google.com>

pci_update_resource() updates a hardware BAR so its address matches the
kernel's struct resource UNLESS it's a disabled ROM BAR.  We only update
those when we enable the ROM.

It's not obvious from the code why ROM BARs should be handled specially.
Apparently there are Matrox devices with defective ROM BARs that read as
zero when disabled.  That means that if pci_enable_rom() reads the disabled
BAR, sets PCI_ROM_ADDRESS_ENABLE (without re-inserting the address), and
writes it back, it would enable the ROM at address zero.

Add comments and references to explain why we can't make the code look more
rational.

The code changes are from 755528c860b0 ("Ignore disabled ROM resources at
setup") and 8085ce084c0f ("[PATCH] Fix PCI ROM mapping").

Link: https://lkml.org/lkml/2005/8/30/138
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/rom.c       |    5 +++++
 drivers/pci/setup-res.c |    6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/pci/rom.c b/drivers/pci/rom.c
index 06663d3..b6edb18 100644
--- a/drivers/pci/rom.c
+++ b/drivers/pci/rom.c
@@ -35,6 +35,11 @@ int pci_enable_rom(struct pci_dev *pdev)
 	if (res->flags & IORESOURCE_ROM_SHADOW)
 		return 0;
 
+	/*
+	 * Ideally pci_update_resource() would update the ROM BAR address,
+	 * and we would only set the enable bit here.  But apparently some
+	 * devices have buggy ROM BARs that read as zero when disabled.
+	 */
 	pcibios_resource_to_bus(pdev->bus, &region, res);
 	pci_read_config_dword(pdev, pdev->rom_base_reg, &rom_addr);
 	rom_addr &= ~PCI_ROM_ADDRESS_MASK;
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 09bdff7..57d7041 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -67,6 +67,12 @@ static void pci_std_update_resource(struct pci_dev *dev, int resno)
 	if (resno < PCI_ROM_RESOURCE) {
 		reg = PCI_BASE_ADDRESS_0 + 4 * resno;
 	} else if (resno == PCI_ROM_RESOURCE) {
+
+		/*
+		 * Apparently some Matrox devices have ROM BARs that read
+		 * as zero when disabled, so don't update ROM BARs unless
+		 * they're enabled.  See https://lkml.org/lkml/2005/8/30/138.
+		 */
 		if (!(res->flags & IORESOURCE_ROM_ENABLE))
 			return;
 

  parent reply	other threads:[~2016-11-29  4:17 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-29  4:13 [PATCH v4 0/7] Disable VF's memory space on updating IOV BARs Bjorn Helgaas
2016-11-29  4:14 ` [PATCH v4 1/7] PCI: Do any VF BAR updates before enabling the BARs Bjorn Helgaas
2016-11-29  4:14 ` [PATCH v4 2/7] PCI: Ignore BAR updates on virtual functions Bjorn Helgaas
2016-11-29  4:45   ` Gavin Shan
2016-11-29  4:15 ` [PATCH v4 3/7] PCI: Separate VF BAR updates from standard BAR updates Bjorn Helgaas
2016-11-29  4:55   ` Gavin Shan
2016-11-29 14:48     ` Bjorn Helgaas
2016-11-29 23:20       ` Gavin Shan
2016-11-30  0:06         ` Bjorn Helgaas
2016-11-30 23:02           ` Gavin Shan
2016-11-30 23:45             ` Bjorn Helgaas
2016-12-01  0:00               ` Gavin Shan
2016-11-29  4:15 ` [PATCH v4 4/7] PCI: Don't update VF BARs while VF memory space is enabled Bjorn Helgaas
2016-11-29  4:57   ` Gavin Shan
2016-11-30 17:56   ` David Laight
2016-11-30 18:52     ` Bjorn Helgaas
2016-11-29  4:15 ` [PATCH v4 5/7] PCI: Remove pci_resource_bar() and pci_iov_resource_bar() Bjorn Helgaas
2016-11-29  5:02   ` Gavin Shan
2016-11-29  4:16 ` [PATCH v4 6/7] PCI: Decouple IORESOURCE_ROM_ENABLE and PCI_ROM_ADDRESS_ENABLE Bjorn Helgaas
2016-11-29  5:03   ` Gavin Shan
2016-11-29  4:16 ` Bjorn Helgaas [this message]
2016-11-29  5:05   ` [PATCH v4 7/7] PCI: Add comments about ROM BAR updating Gavin Shan
2016-12-01 22:21 ` [PATCH v4 0/7] Disable VF's memory space on updating IOV BARs Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161129041648.21453.94596.stgit@bhelgaas-glaptop.roam.corp.google.com \
    --to=bhelgaas@google.com \
    --cc=benh@kernel.crashing.org \
    --cc=clsoto@us.ibm.com \
    --cc=gwshan@linux.vnet.ibm.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).