diff mbox

[1/2] PCI: mvebu - The bridge should obey the MEM and IO command bits

Message ID 1383262380-6984-1-git-send-email-jgunthorpe@obsidianresearch.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Jason Gunthorpe Oct. 31, 2013, 11:32 p.m. UTC
When PCI_COMMAND_MEMORY/PCI_COMMAND_IO are cleared the bridge should not
allocate windows or even look at the window limit/base registers.

Otherwise it can attempt to setup bogus windows that the PCI core code
creates during discovery. The core will leave PCI_COMMAND_IO cleared if
it doesn't need an IO window.

Have mvebu_pcie_handle_*_change respect the bits, and call the change
function whenever the bits changes.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/pci/host/pci-mvebu.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Jason Cooper Nov. 24, 2013, 2:58 a.m. UTC | #1
Jason,

On Thu, Oct 31, 2013 at 05:32:59PM -0600, Jason Gunthorpe wrote:
> When PCI_COMMAND_MEMORY/PCI_COMMAND_IO are cleared the bridge should not
> allocate windows or even look at the window limit/base registers.
> 
> Otherwise it can attempt to setup bogus windows that the PCI core code
> creates during discovery. The core will leave PCI_COMMAND_IO cleared if
> it doesn't need an IO window.
> 
> Have mvebu_pcie_handle_*_change respect the bits, and call the change
> function whenever the bits changes.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/pci/host/pci-mvebu.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

I'd like to get back on track with these small patches being Acked by
mvebu maintainers and then going through Bjorn.  Could you please resend
an updated patchset for this and 2/2?

thx,

Jason.
--
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
Jason Cooper Nov. 24, 2013, 3 a.m. UTC | #2
On Thu, Oct 31, 2013 at 05:32:59PM -0600, Jason Gunthorpe wrote:
> When PCI_COMMAND_MEMORY/PCI_COMMAND_IO are cleared the bridge should not
> allocate windows or even look at the window limit/base registers.
> 
> Otherwise it can attempt to setup bogus windows that the PCI core code
> creates during discovery. The core will leave PCI_COMMAND_IO cleared if
> it doesn't need an IO window.
> 
> Have mvebu_pcie_handle_*_change respect the bits, and call the change
> function whenever the bits changes.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/pci/host/pci-mvebu.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

And a small addendum: I currently have the following in mvebu/drivers

  572bd682145f PCI: mvebu - The bridge secondary status register should be 0
  9503c7fe4d9d PCI: mvebu - Support a bridge with no IO port window
  058100a08be8 PCI: mvebu: return NULL instead of ERR_PTR(ret)

which means they didn't make it in for v3.13-rc1.  I'll hold them there
for reference until you resend.

thx,

Jason.
--
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
Jason Cooper Nov. 24, 2013, 4 a.m. UTC | #3
On Sat, Nov 23, 2013 at 10:00:33PM -0500, Jason Cooper wrote:
> On Thu, Oct 31, 2013 at 05:32:59PM -0600, Jason Gunthorpe wrote:
> > When PCI_COMMAND_MEMORY/PCI_COMMAND_IO are cleared the bridge should not
> > allocate windows or even look at the window limit/base registers.
> > 
> > Otherwise it can attempt to setup bogus windows that the PCI core code
> > creates during discovery. The core will leave PCI_COMMAND_IO cleared if
> > it doesn't need an IO window.
> > 
> > Have mvebu_pcie_handle_*_change respect the bits, and call the change
> > function whenever the bits changes.
> > 
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > ---
> >  drivers/pci/host/pci-mvebu.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> And a small addendum: I currently have the following in mvebu/drivers

Strike that, they are now in mvebu/pci.

>   572bd682145f PCI: mvebu - The bridge secondary status register should be 0
>   9503c7fe4d9d PCI: mvebu - Support a bridge with no IO port window
>   058100a08be8 PCI: mvebu: return NULL instead of ERR_PTR(ret)
> 
> which means they didn't make it in for v3.13-rc1.  I'll hold them there
> for reference until you resend.

thx,

Jason.
--
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
Jason Gunthorpe Nov. 25, 2013, 5:52 a.m. UTC | #4
On Sat, Nov 23, 2013 at 09:58:08PM -0500, Jason Cooper wrote:
> Jason,
> 
> On Thu, Oct 31, 2013 at 05:32:59PM -0600, Jason Gunthorpe wrote:
> > When PCI_COMMAND_MEMORY/PCI_COMMAND_IO are cleared the bridge should not
> > allocate windows or even look at the window limit/base registers.
> > 
> > Otherwise it can attempt to setup bogus windows that the PCI core code
> > creates during discovery. The core will leave PCI_COMMAND_IO cleared if
> > it doesn't need an IO window.
> > 
> > Have mvebu_pcie_handle_*_change respect the bits, and call the change
> > function whenever the bits changes.
> > 
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >  drivers/pci/host/pci-mvebu.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> I'd like to get back on track with these small patches being Acked by
> mvebu maintainers and then going through Bjorn.  Could you please resend
> an updated patchset for this and 2/2?

I recall these two patches are waiting for Thomas to give his Ok. His
testing found problems with the prior versions that I cannot detect on
my hardware..

Once that happens I can send them on with his ack to Bjorn?

Regards,
Jason
--
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
Thomas Petazzoni Nov. 25, 2013, 8:12 a.m. UTC | #5
Dear Jason Gunthorpe,

On Sun, 24 Nov 2013 22:52:22 -0700, Jason Gunthorpe wrote:

> > I'd like to get back on track with these small patches being Acked by
> > mvebu maintainers and then going through Bjorn.  Could you please resend
> > an updated patchset for this and 2/2?
> 
> I recall these two patches are waiting for Thomas to give his Ok. His
> testing found problems with the prior versions that I cannot detect on
> my hardware..

Correct. Sorry for the delay, I'll try to test them today. Thanks for
the reminder!

Best regards,

Thomas
Jason Cooper Nov. 25, 2013, 11:29 a.m. UTC | #6
On Sun, Nov 24, 2013 at 10:52:22PM -0700, Jason Gunthorpe wrote:
> On Sat, Nov 23, 2013 at 09:58:08PM -0500, Jason Cooper wrote:
> > Jason,
> > 
> > On Thu, Oct 31, 2013 at 05:32:59PM -0600, Jason Gunthorpe wrote:
> > > When PCI_COMMAND_MEMORY/PCI_COMMAND_IO are cleared the bridge should not
> > > allocate windows or even look at the window limit/base registers.
> > > 
> > > Otherwise it can attempt to setup bogus windows that the PCI core code
> > > creates during discovery. The core will leave PCI_COMMAND_IO cleared if
> > > it doesn't need an IO window.
> > > 
> > > Have mvebu_pcie_handle_*_change respect the bits, and call the change
> > > function whenever the bits changes.
> > > 
> > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > >  drivers/pci/host/pci-mvebu.c | 14 ++++++++++++--
> > >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > I'd like to get back on track with these small patches being Acked by
> > mvebu maintainers and then going through Bjorn.  Could you please resend
> > an updated patchset for this and 2/2?
> 
> I recall these two patches are waiting for Thomas to give his Ok. His
> testing found problems with the prior versions that I cannot detect on
> my hardware..
> 
> Once that happens I can send them on with his ack to Bjorn?

Please do.

thx,

Jason.
--
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
Thomas Petazzoni Nov. 25, 2013, 5:09 p.m. UTC | #7
Dear Jason Gunthorpe,

On Thu, 31 Oct 2013 17:32:59 -0600, Jason Gunthorpe wrote:
> When PCI_COMMAND_MEMORY/PCI_COMMAND_IO are cleared the bridge should not
> allocate windows or even look at the window limit/base registers.
> 
> Otherwise it can attempt to setup bogus windows that the PCI core code
> creates during discovery. The core will leave PCI_COMMAND_IO cleared if
> it doesn't need an IO window.
> 
> Have mvebu_pcie_handle_*_change respect the bits, and call the change
> function whenever the bits changes.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/pci/host/pci-mvebu.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Thanks!

Thomas
diff mbox

Patch

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 19d77c9..721fca9 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -279,7 +279,8 @@  static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 
 	/* Are the new iobase/iolimit values invalid? */
 	if (port->bridge.iolimit < port->bridge.iobase ||
-	    port->bridge.iolimitupper < port->bridge.iobaseupper) {
+	    port->bridge.iolimitupper < port->bridge.iobaseupper ||
+	    !(port->bridge.command & PCI_COMMAND_IO)) {
 
 		/* If a window was configured, remove it */
 		if (port->iowin_base) {
@@ -316,7 +317,8 @@  static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
 {
 	/* Are the new membase/memlimit values invalid? */
-	if (port->bridge.memlimit < port->bridge.membase) {
+	if (port->bridge.memlimit < port->bridge.membase ||
+	    !(port->bridge.command & PCI_COMMAND_MEMORY)) {
 
 		/* If a window was configured, remove it */
 		if (port->memwin_base) {
@@ -464,8 +466,16 @@  static int mvebu_sw_pci_bridge_write(struct mvebu_pcie_port *port,
 
 	switch (where & ~3) {
 	case PCI_COMMAND:
+	{
+		u32 old = bridge->command;
+
 		bridge->command = value & 0xffff;
+		if ((old ^ bridge->command) & PCI_COMMAND_IO)
+			mvebu_pcie_handle_iobase_change(port);
+		if ((old ^ bridge->command) & PCI_COMMAND_MEMORY)
+			mvebu_pcie_handle_membase_change(port);
 		break;
+	}
 
 	case PCI_BASE_ADDRESS_0 ... PCI_BASE_ADDRESS_1:
 		bridge->bar[((where & ~3) - PCI_BASE_ADDRESS_0) / 4] = value;