linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: pci-mvebu driver on km_kirkwood
Date: Wed, 19 Feb 2014 10:26:58 +0100	[thread overview]
Message-ID: <20140219102658.76eec91e@skate> (raw)
In-Reply-To: <53046D98.6020801@keymile.com>

Dear Gerlando Falauto,

On Wed, 19 Feb 2014 09:38:48 +0100, Gerlando Falauto wrote:

> >> pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
> >> pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
> >> pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
> >> pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
> >> pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]
> >
> > So in total, for the device 0000:01:00, the memory region should go
> > from 0xe0000000 to 0xe8804fff. This means that a 256 MB window is
> > needed for this device, because only power of two sizes are possible
> > for MBus windows.
> >
> > Can you give me the output of /sys/kernel/debug/mvebu-mbus/devices ? It
> > will tell us how the MBus windows are configured, as I suspect the
> > problem might be here.
> 
> Here it goes:
> 
> [00] disabled
> [01] disabled
> [02] disabled
> [03] disabled
> [04] 00000000ff000000 - 00000000ff010000 : nand
> [05] 00000000f4000000 - 00000000f8000000 : vpcie
> [06] 00000000fe000000 - 00000000fe010000 : dragonite
> [07] 00000000e0000000 - 00000000ec000000 : pcie0.0
> 
> So there's something wrong: a 256MB window should go all the way up to 
> 0xf0000000, and we have 192MB instead and I don't know how that would be 
> interpreted.

My understanding is that a 192 MB window is illegal, because the window
size should be encoded as a sequence of 1s followed by a sequence of 0s
from the LSB to the MSB. To me, this means that only power of two
window sizes are possible.

> I couldn't figure out where this range comes from though, as in the 
> device tree I now have a size of 256MB (I stupidly set it to 192MB at 
> some point, but I now changed it):
> 
> # hexdump -C /proc/device-tree/ocp at f1000000/pcie-controller/ranges
>   | cut -c1-58
> 00000000  82 00 00 00 00 00 00 00  00 04 00 00 00 04 00 00
> 00000010  00 00 00 00 00 00 20 00  82 00 00 00 00 00 00 00
> 00000020  e0 00 00 00 e0 00 00 00  00 00 00 00 10 00 00 00
>                                                 ^^^^^^^^^^^

Wow, that's an old DT representation that you have here :)

But ok, let me try to explain. The 256 MB value that you define in the
DT is the global PCIe memory aperture: it is the maximum amount of
memory that we allow the PCIe driver to allocate for PCIe windows. But
depending on which PCIe devices you have plugged in, and how large
their BARs are, not necessarily all of these 256 MB will be used.

So, you can very well have a 256 MB global PCIe memory aperture, and
still have only one 1 MB PCIe memory window for PCIe 0.0 and a 256 KB
PCIe memory window for PCIe 1.0, and that's it.

Now, the 192 MB comes from the enumeration of your device. Linux
enumerates the BAR of your device:

pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]

and then concludes that at the emulated bridge level, the memory region
to be created is:

pci 0000:00:01.0:   bridge window [mem 0xe0000000-0xebffffff]

which corresponds to the 192 MB window that we see created.

But I believe a 192 MB memory window cannot work with MBus, it should
be rounded up to the next power of 2. Can you try the below patch (not
tested, not even compiled, might need some tweaks to apply to your 3.10
kernel) :

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 13478ec..002229a 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -372,6 +372,11 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
                (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
                port->memwin_base;
 
+       pr_info("PCIE %d.%d: creating window at 0x%x, size 0x%x rounded up to 0x%x\n",
+               port->port, port->lane, port->memwin_base,
+               port->memwin_size, roundup_pow_of_two(port->memwin_size));
+       port->memwin_size = roundup_pow_of_two(port->memwin_size);
+
        mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
                                    port->memwin_base, port->memwin_size);
 }

I'm obviously interested in seeing the message that gets shown, as well
as the new mvebu-mbus debugfs output.

For good measure, if you could also dump the registers of the PCIe
window. In your case, it was window 7, so dumping 0xf1020070 and
0xf1020074 would be useful.

> But apart from that, what I still don't understand is how that could 
> have anything to do with my problem. The memory area I'm not able to 
> access starts at 0xe4000000.
> BAR0, on the other hand, spawns 0xe8802000-0xe8802fff and seems to work 
> fine.

I am not sure, but since we are configuring an invalid memory size,
maybe the MBus behavior is undefined, and we get some completely funky
behavior, where parts of the 192 MB window are actually work, but parts
of it are not.

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

  reply	other threads:[~2014-02-19  9:26 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-10 16:15 pci-mvebu driver on km_kirkwood Gerlando Falauto
2013-07-10 16:57 ` Thomas Petazzoni
2013-07-10 17:31   ` Gerlando Falauto
2013-07-10 19:56     ` Gerlando Falauto
2013-07-11  7:03     ` Valentin Longchamp
2013-07-12  8:59       ` Thomas Petazzoni
2013-07-15 15:46         ` Valentin Longchamp
2013-07-15 19:51           ` Thomas Petazzoni
2013-07-11 14:32     ` Thomas Petazzoni
2014-02-18 17:29       ` Gerlando Falauto
2014-02-18 20:27         ` Thomas Petazzoni
2014-02-19  8:38           ` Gerlando Falauto
2014-02-19  9:26             ` Thomas Petazzoni [this message]
2014-02-19  9:39               ` Gerlando Falauto
2014-02-19 13:37                 ` Thomas Petazzoni
2014-02-19 21:45                   ` Bjorn Helgaas
2014-02-20  8:55                     ` Thomas Petazzoni
2014-02-20 17:35                       ` Jason Gunthorpe
2014-02-20 20:29                         ` Thomas Petazzoni
2014-02-21  0:32                           ` Jason Gunthorpe
2014-02-21  8:34                             ` Thomas Petazzoni
2014-02-21  8:58                               ` Gerlando Falauto
2014-02-21  9:12                                 ` Thomas Petazzoni
2014-02-21  9:16                                   ` Gerlando Falauto
2014-02-21  9:39                                     ` Thomas Petazzoni
2014-02-21 12:24                                       ` Gerlando Falauto
2014-02-21 13:47                                         ` Thomas Petazzoni
2014-02-21 15:05                                           ` Arnd Bergmann
2014-02-21 15:11                                             ` Thomas Petazzoni
2014-02-21 15:20                                               ` Arnd Bergmann
2014-02-21 15:37                                                 ` Thomas Petazzoni
2014-02-21 16:39                                           ` Jason Gunthorpe
2014-02-21 17:05                                             ` Thomas Petazzoni
2014-02-21 17:31                                               ` Jason Gunthorpe
2014-02-21 18:05                                                 ` Arnd Bergmann
2014-02-21 18:29                                                   ` Gerlando Falauto
2014-02-21 18:18                                           ` Gerlando Falauto
2014-02-21 18:45                                             ` Thomas Petazzoni
2014-02-20 19:18                       ` Bjorn Helgaas
2014-02-21  0:24                         ` Jason Gunthorpe
2014-02-21 19:05                           ` Bjorn Helgaas
2014-02-21 19:21                             ` Thomas Petazzoni
2014-02-21 19:53                             ` Benjamin Herrenschmidt
2014-02-23  3:43                               ` Gavin Shan
2013-07-31  8:03 ` Thomas Petazzoni
2013-07-31  8:26   ` Gerlando Falauto
2013-07-31  9:00     ` Thomas Petazzoni
2013-07-31 20:50       ` Jason Gunthorpe
2013-08-09 14:01         ` Thierry Reding
2013-08-26  9:27           ` Gerlando Falauto
2013-08-26 12:02             ` Thierry Reding
2013-08-26 14:49               ` Gerlando Falauto
2013-08-26 19:16                 ` Jason Gunthorpe
2013-11-04 14:49                   ` Gerlando Falauto
2013-11-05  8:13                     ` Thierry Reding

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=20140219102658.76eec91e@skate \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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).