All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gerlando Falauto <gerlando.falauto@keymile.com>
To: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: "Jason Gunthorpe" <jgunthorpe@obsidianresearch.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Longchamp, Valentin" <Valentin.Longchamp@keymile.com>,
	"Ezequiel Garcia" <ezequiel.garcia@free-electrons.com>,
	"Lior Amsalem" <alior@marvell.com>,
	"Gregory Clément" <gregory.clement@free-electrons.com>
Subject: Re: pci-mvebu driver on km_kirkwood
Date: Fri, 21 Feb 2014 19:18:25 +0100	[thread overview]
Message-ID: <53079871.8080801@keymile.com> (raw)
In-Reply-To: <20140221144708.48559045@skate>

Dear Thomas,

On 02/21/2014 02:47 PM, Thomas Petazzoni wrote:
> Dear Gerlando Falauto,
>
> On Fri, 21 Feb 2014 13:24:36 +0100, Gerlando Falauto wrote:
>
>> So I restored the total aperture size to 192MB.
>> I had to rework your patch a bit because:
>>
>> a) I'm running an older kernel and driver
>> b) sizes are actually 1-byte offset
>
> Hum, right. This is a bit weird, maybe I should change that, I don't
> think the mvebu-mbus driver should accept 1-byte offset sizes.

I don't know anything about this, I only know the size dumped is of the 
form 0x...ffff, that's all.

>> Here's the assignment (same as before):
>>
>> pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xebffffff]
>> 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 here's the output I get from:
>>
>> # cat /sys/kernel/debug/mvebu-mbus/devices
>> [00] 00000000e8000000 - 00000000ec000000 : pcie0.0 (remap 00000000e8000000)
>> [01] disabled
>> [02] disabled
>> [03] disabled
>> [04] 00000000ff000000 - 00000000ff010000 : nand
>> [05] 00000000f4000000 - 00000000f8000000 : vpcie
>> [06] 00000000fe000000 - 00000000fe010000 : dragonite
>> [07] 00000000e0000000 - 00000000e8000000 : pcie0.0
>
> This seems correct: we have two windows pointing to the same device,
> and they have consecutive addresses.

I don't know how to interpret the (remap ... ) bit, but yes, this looks 
right to me as well. I just don't know why mbus window 7 gets picked 
before 0, but apart from that, it looks nice.

>> I did not get to test the whole address space thoroughly, but all the
>> BARs are still accessible (mainly BAR0 which contains the control space
>> and is mapped on the "new" MBUS window, and BAR1 which is the "big"
>> one). So at least, the issues we had before are now gone.
>
> Did you check that what you read from BAR0 (which is mapped on the new
> MBUS window) is really what you expect, and not just the same thing as
> BAR1 accessible for the big window? I just want to make sure that the
> hardware indeed properly handles two windows for the same device.

Yes, there's no way the two BARs could be aliased. It's a fairly complex 
FPGA design, where BAR1 is the huge address space for a PCI-to-localbus 
bridge (whose connected devices are recognized correctly) and BAR0 is 
the control BAR (and its registers are read and written without a problem).


>> So I'd say this looks like a very promising approach. :-)
>
> Indeed. However, I don't think this approach solves the entire problem,
> for two reasons:
>
>   *) For small BARs that are not power-of-two sized, we may not want to
>      consume two windows, but instead consume a little bit more address
>      space. Using two windows to map a 96 KB BAR would be a waste of
>      windows: using a single 128 KB window is much more efficient.
>
>   *) I don't know if the algorithm to split the BAR into multiple
>      windows is going to be trivial.

I see others have already replied and I pretty much agree with them.

Thanks,
Gerlando

WARNING: multiple messages have this Message-ID (diff)
From: gerlando.falauto@keymile.com (Gerlando Falauto)
To: linux-arm-kernel@lists.infradead.org
Subject: pci-mvebu driver on km_kirkwood
Date: Fri, 21 Feb 2014 19:18:25 +0100	[thread overview]
Message-ID: <53079871.8080801@keymile.com> (raw)
In-Reply-To: <20140221144708.48559045@skate>

Dear Thomas,

On 02/21/2014 02:47 PM, Thomas Petazzoni wrote:
> Dear Gerlando Falauto,
>
> On Fri, 21 Feb 2014 13:24:36 +0100, Gerlando Falauto wrote:
>
>> So I restored the total aperture size to 192MB.
>> I had to rework your patch a bit because:
>>
>> a) I'm running an older kernel and driver
>> b) sizes are actually 1-byte offset
>
> Hum, right. This is a bit weird, maybe I should change that, I don't
> think the mvebu-mbus driver should accept 1-byte offset sizes.

I don't know anything about this, I only know the size dumped is of the 
form 0x...ffff, that's all.

>> Here's the assignment (same as before):
>>
>> pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xebffffff]
>> 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 here's the output I get from:
>>
>> # cat /sys/kernel/debug/mvebu-mbus/devices
>> [00] 00000000e8000000 - 00000000ec000000 : pcie0.0 (remap 00000000e8000000)
>> [01] disabled
>> [02] disabled
>> [03] disabled
>> [04] 00000000ff000000 - 00000000ff010000 : nand
>> [05] 00000000f4000000 - 00000000f8000000 : vpcie
>> [06] 00000000fe000000 - 00000000fe010000 : dragonite
>> [07] 00000000e0000000 - 00000000e8000000 : pcie0.0
>
> This seems correct: we have two windows pointing to the same device,
> and they have consecutive addresses.

I don't know how to interpret the (remap ... ) bit, but yes, this looks 
right to me as well. I just don't know why mbus window 7 gets picked 
before 0, but apart from that, it looks nice.

>> I did not get to test the whole address space thoroughly, but all the
>> BARs are still accessible (mainly BAR0 which contains the control space
>> and is mapped on the "new" MBUS window, and BAR1 which is the "big"
>> one). So at least, the issues we had before are now gone.
>
> Did you check that what you read from BAR0 (which is mapped on the new
> MBUS window) is really what you expect, and not just the same thing as
> BAR1 accessible for the big window? I just want to make sure that the
> hardware indeed properly handles two windows for the same device.

Yes, there's no way the two BARs could be aliased. It's a fairly complex 
FPGA design, where BAR1 is the huge address space for a PCI-to-localbus 
bridge (whose connected devices are recognized correctly) and BAR0 is 
the control BAR (and its registers are read and written without a problem).


>> So I'd say this looks like a very promising approach. :-)
>
> Indeed. However, I don't think this approach solves the entire problem,
> for two reasons:
>
>   *) For small BARs that are not power-of-two sized, we may not want to
>      consume two windows, but instead consume a little bit more address
>      space. Using two windows to map a 96 KB BAR would be a waste of
>      windows: using a single 128 KB window is much more efficient.
>
>   *) I don't know if the algorithm to split the BAR into multiple
>      windows is going to be trivial.

I see others have already replied and I pretty much agree with them.

Thanks,
Gerlando

  parent reply	other threads:[~2014-02-21 18:18 UTC|newest]

Thread overview: 90+ 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
2014-02-19  9:39               ` Gerlando Falauto
2014-02-19 13:37                 ` Thomas Petazzoni
2014-02-19 13:37                   ` Thomas Petazzoni
2014-02-19 21:45                   ` Bjorn Helgaas
2014-02-19 21:45                     ` Bjorn Helgaas
2014-02-20  8:55                     ` Thomas Petazzoni
2014-02-20  8:55                       ` Thomas Petazzoni
2014-02-20 17:35                       ` Jason Gunthorpe
2014-02-20 17:35                         ` Jason Gunthorpe
2014-02-20 20:29                         ` Thomas Petazzoni
2014-02-20 20:29                           ` Thomas Petazzoni
2014-02-21  0:32                           ` Jason Gunthorpe
2014-02-21  0:32                             ` Jason Gunthorpe
2014-02-21  8:34                             ` Thomas Petazzoni
2014-02-21  8:34                               ` Thomas Petazzoni
2014-02-21  8:58                               ` Gerlando Falauto
2014-02-21  8:58                                 ` Gerlando Falauto
2014-02-21  9:12                                 ` Thomas Petazzoni
2014-02-21  9:12                                   ` Thomas Petazzoni
2014-02-21  9:16                                   ` Gerlando Falauto
2014-02-21  9:16                                     ` Gerlando Falauto
2014-02-21  9:39                                     ` Thomas Petazzoni
2014-02-21  9:39                                       ` Thomas Petazzoni
2014-02-21 12:24                                       ` Gerlando Falauto
2014-02-21 12:24                                         ` Gerlando Falauto
2014-02-21 13:47                                         ` Thomas Petazzoni
2014-02-21 13:47                                           ` Thomas Petazzoni
2014-02-21 15:05                                           ` Arnd Bergmann
2014-02-21 15:05                                             ` Arnd Bergmann
2014-02-21 15:11                                             ` Thomas Petazzoni
2014-02-21 15:11                                               ` Thomas Petazzoni
2014-02-21 15:20                                               ` Arnd Bergmann
2014-02-21 15:20                                                 ` Arnd Bergmann
2014-02-21 15:37                                                 ` Thomas Petazzoni
2014-02-21 15:37                                                   ` Thomas Petazzoni
2014-02-21 16:39                                           ` Jason Gunthorpe
2014-02-21 16:39                                             ` Jason Gunthorpe
2014-02-21 17:05                                             ` Thomas Petazzoni
2014-02-21 17:05                                               ` Thomas Petazzoni
2014-02-21 17:31                                               ` Jason Gunthorpe
2014-02-21 17:31                                                 ` Jason Gunthorpe
2014-02-21 18:05                                                 ` Arnd Bergmann
2014-02-21 18:05                                                   ` Arnd Bergmann
2014-02-21 18:29                                                   ` Gerlando Falauto
2014-02-21 18:29                                                     ` Gerlando Falauto
2014-02-21 18:18                                           ` Gerlando Falauto [this message]
2014-02-21 18:18                                             ` Gerlando Falauto
2014-02-21 18:45                                             ` Thomas Petazzoni
2014-02-21 18:45                                               ` Thomas Petazzoni
2014-02-20 19:18                       ` Bjorn Helgaas
2014-02-20 19:18                         ` Bjorn Helgaas
2014-02-21  0:24                         ` Jason Gunthorpe
2014-02-21  0:24                           ` Jason Gunthorpe
2014-02-21 19:05                           ` Bjorn Helgaas
2014-02-21 19:05                             ` Bjorn Helgaas
2014-02-21 19:21                             ` Thomas Petazzoni
2014-02-21 19:21                               ` Thomas Petazzoni
2014-02-21 19:53                             ` Benjamin Herrenschmidt
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  9:27             ` Gerlando Falauto
2013-08-26 12:02             ` Thierry Reding
2013-08-26 12:02               ` Thierry Reding
2013-08-26 14:49               ` Gerlando Falauto
2013-08-26 14:49                 ` Gerlando Falauto
2013-08-26 19:16                 ` Jason Gunthorpe
2013-08-26 19:16                   ` Jason Gunthorpe
2013-11-04 14:49                   ` Gerlando Falauto
2013-11-04 14:49                     ` Gerlando Falauto
2013-11-05  8:13                     ` Thierry Reding
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=53079871.8080801@keymile.com \
    --to=gerlando.falauto@keymile.com \
    --cc=Valentin.Longchamp@keymile.com \
    --cc=alior@marvell.com \
    --cc=andrew@lunn.ch \
    --cc=bhelgaas@google.com \
    --cc=ezequiel.garcia@free-electrons.com \
    --cc=gregory.clement@free-electrons.com \
    --cc=jason@lakedaemon.net \
    --cc=jgunthorpe@obsidianresearch.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    /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.