All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Stefan Roese <sr@denx.de>
Cc: linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	"Russell King" <rmk+kernel@arm.linux.org.uk>,
	"Jisheng Zhang" <jszhang@marvell.com>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Grégory Clement" <gregory.clement@free-electrons.com>,
	"Arnd Bergmann" <arnd@arndb.de>
Subject: Re: [PATCH] PCI: mvebu: Don't try to add an MBus window that already exists
Date: Thu, 2 Jun 2016 15:18:18 +0200	[thread overview]
Message-ID: <20160602151818.0989ee50@free-electrons.com> (raw)
In-Reply-To: <20160602125247.17566-1-sr@denx.de>

Hello,

On Thu,  2 Jun 2016 14:52:47 +0200, Stefan Roese wrote:
> Add a check to mvebu_pcie_add_windows() to detect, if an MBus window is
> already configured. If this is the case (base address, size, target and
> attribute are identical), then this window is not created. This fixes
> a problem I'm currently seeing on a custom Armada XP based board, which
> generates this error upon PCI rescanning (in this case via sysfs):
> 
> $ echo 1 > /sys/bus/pci/rescan
> mvebu_mbus: cannot add window '4:e8', conflicts with another window
> mvebu-pcie soc:pcie-controller: Could not create MBus window at [mem 0x9e000000-0x9e0fffff]: -22

Thanks for the patch. I am not familiar with what happens during a PCI
rescan, but shouldn't the MBus windows be deleted and then re-added?

The pci-mvebu driver deletes the MBus windows for a given PCI device
when invalid memory base/limits are set in the emulated PCI bridge.
Shouldn't this happen over a rescan?

If it doesn't, then the other question is whether the check you're
adding should be done in the PCI driver or in the MBus driver.

In commit b566e782be32145664d96ada3e389f17d32742e5, we already relaxed
the checks done by the MBus driver, and since this commit we allow
different windows to have the same target/attribute.

Should the MBus driver also allow re-creating a	window that
already exists, if all its properties are the same?

Note that I am not asking you to rework your patch in this direction
for the moment, I'm only opening the question. I haven't made up my
mind yet on which solution is the most correct one.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] PCI: mvebu: Don't try to add an MBus window that already exists
Date: Thu, 2 Jun 2016 15:18:18 +0200	[thread overview]
Message-ID: <20160602151818.0989ee50@free-electrons.com> (raw)
In-Reply-To: <20160602125247.17566-1-sr@denx.de>

Hello,

On Thu,  2 Jun 2016 14:52:47 +0200, Stefan Roese wrote:
> Add a check to mvebu_pcie_add_windows() to detect, if an MBus window is
> already configured. If this is the case (base address, size, target and
> attribute are identical), then this window is not created. This fixes
> a problem I'm currently seeing on a custom Armada XP based board, which
> generates this error upon PCI rescanning (in this case via sysfs):
> 
> $ echo 1 > /sys/bus/pci/rescan
> mvebu_mbus: cannot add window '4:e8', conflicts with another window
> mvebu-pcie soc:pcie-controller: Could not create MBus window at [mem 0x9e000000-0x9e0fffff]: -22

Thanks for the patch. I am not familiar with what happens during a PCI
rescan, but shouldn't the MBus windows be deleted and then re-added?

The pci-mvebu driver deletes the MBus windows for a given PCI device
when invalid memory base/limits are set in the emulated PCI bridge.
Shouldn't this happen over a rescan?

If it doesn't, then the other question is whether the check you're
adding should be done in the PCI driver or in the MBus driver.

In commit b566e782be32145664d96ada3e389f17d32742e5, we already relaxed
the checks done by the MBus driver, and since this commit we allow
different windows to have the same target/attribute.

Should the MBus driver also allow re-creating a	window that
already exists, if all its properties are the same?

Note that I am not asking you to rework your patch in this direction
for the moment, I'm only opening the question. I haven't made up my
mind yet on which solution is the most correct one.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  reply	other threads:[~2016-06-02 13:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-02 12:52 [PATCH] PCI: mvebu: Don't try to add an MBus window that already exists Stefan Roese
2016-06-02 12:52 ` Stefan Roese
2016-06-02 13:18 ` Thomas Petazzoni [this message]
2016-06-02 13:18   ` Thomas Petazzoni
2016-06-03  5:57   ` Stefan Roese
2016-06-03  5:57     ` Stefan Roese
2016-06-03  7:30     ` Thomas Petazzoni
2016-06-03  7:30       ` Thomas Petazzoni
2016-06-17 23:36       ` Bjorn Helgaas
2016-06-17 23:36         ` Bjorn Helgaas
2016-06-17 23:46     ` Bjorn Helgaas
2016-06-17 23:46       ` Bjorn Helgaas
2016-06-20 13:42       ` Stefan Roese
2016-06-20 13:42         ` Stefan Roese

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=20160602151818.0989ee50@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=gregory.clement@free-electrons.com \
    --cc=jason@lakedaemon.net \
    --cc=jszhang@marvell.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rmk+kernel@arm.linux.org.uk \
    --cc=sr@denx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.