On Tue, Mar 31, 2015 at 07:57:49PM -0700, Yinghai Lu wrote:
> We still get "no compatible bridge window" warning on sparc T5-8
> after we add support for 64bit resource parsing for root bus.
>
> PCI: scan_bus[/pci@300/pci@1/pci@0/pci@6] bus no 8
> PCI: Claiming 0000:00:01.0: Resource 15: 0000800100000000..00008004afffffff [220c]
> PCI: Claiming 0000:01:00.0: Resource 15: 0000800100000000..00008004afffffff [220c]
> PCI: Claiming 0000:02:04.0: Resource 15: 0000800100000000..000080012fffffff [220c]
> PCI: Claiming 0000:03:00.0: Resource 15: 0000800100000000..000080012fffffff [220c]
> PCI: Claiming 0000:04:06.0: Resource 14: 0000800100000000..000080010fffffff [220c]
> PCI: Claiming 0000:05:00.0: Resource 0: 0000800100000000..0000800100001fff [204]
> pci 0000:05:00.0: can't claim BAR 0 [mem 0x800100000000-0x800100001fff]: no compatible bridge window
>
> All the bridges 64-bit resource have pref bit, but the device resource does not
> have pref set, then we can not find parent for the device resource,
> as we can not put non-pref mem under pref mem.
>
> According to pcie spec errta
> https://www.pcisig.com/specifications/pciexpress/base2/PCIe_Base_r2.1_Errata_08Jun10.pdf
> page 13, in some case it is ok to mark some as pref.
That implementation note is included in the PCIe spec r3.0, sec 7.5.2.1. I
think that's a better reference to cite.
The most straightforward way to read the implementation note is that it is
safe for the *device*, i.e., the hardware, to set the Prefetchable bit in
some BARs, even if those BARs don't actually support prefetching. But I
don't think the device can really tell when it would be safe, because the
device can't tell whether the entire path leading to it is over PCIe.
I don't think it makes sense for *software* to set the Prefetchable bit in
a BAR, because the spec says bits 0-3 (including Prefetchable) are supposed
to be read-only (conventional PCI spec r3.0, sec 6.2.5.1, p226).
If the intent were to suggest that system software could *treat* a
non-prefetchable BAR as though it were prefetchable, that would make sense,
and it would have been easy to write the implementation note to actually
say that.
I guess I'm OK with doing this as in your patch below even though it really
doesn't match the language in the spec, because I can't see any other
useful way to interpret the spec.
But this is a general change that affects all platforms, and it's late in
the cycle for something as invasive as this. I'd rather include your patch
in the v4.1 merge window, and revert d63e2e1f3df9 ("sparc/PCI: Clip bridge
windows to fit in upstream windows") for v4.0.
Bjorn
> Only set pref for 64bit mmio when the entire path from the host to the adapter is
> over PCI Express.
>
> Fixes: commit d63e2e1f3df9 ("sparc/PCI: Clip bridge windows to fit in upstream windows")
> Link: http://lkml.kernel.org/r/CAE9FiQU1gJY1LYrxs+ma5LCTEEe4xmtjRG0aXJ9K_Tsu+m9Wuw@mail.gmail.com
> Reported-by: David Ahern <[email protected]>
> Tested-by: David Ahern <[email protected]>
> Signed-off-by: Yinghai Lu <[email protected]>
> Cc: <[email protected]> #3.19
> ---
> drivers/pci/probe.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index f71cb7c..4e0cd46 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1507,6 +1507,53 @@ static void pci_init_capabilities(struct pci_dev *dev)
> pci_enable_acs(dev);
> }
>
> +static bool pci_up_path_over_pcie(struct pci_bus *bus)
> +{
> + if (!bus)
> + return true;
> +
> + if (bus->self && !pci_is_pcie(bus->self))
> + return false;
> +
> + return pci_up_path_over_pcie(bus->parent);
> +}
> +
> +/*
> + * According to
> + * https://www.pcisig.com/specifications/pciexpress/base2/PCIe_Base_r2.1_Errata_08Jun10.pdf
> + * page 13, system firmware could put some 64bit non-pref under 64bit pref,
> + * on some cases.
> + * Let's set pref bit for 64bit mmio when entire path from the host to
> + * the adapter is over PCI Express.
> + */
> +static void set_pcie_64bit_pref(struct pci_dev *dev)
> +{
> + int i;
> +
> + if (!pci_is_pcie(dev))
> + return;
> +
> + if (!pci_up_path_over_pcie(dev->bus))
> + return;
> +
> + for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
> + struct resource *res = &dev->resource[i];
> + enum pci_bar_type type;
> + int reg;
> +
> + if (!(res->flags & IORESOURCE_MEM_64))
> + continue;
> +
> + if (res->flags & IORESOURCE_PREFETCH)
> + continue;
> +
> + reg = pci_resource_bar(dev, i, &type);
> + dev_printk(KERN_DEBUG, &dev->dev, "reg 0x%x %pR + pref\n",
> + reg, res);
> + res->flags |= IORESOURCE_PREFETCH;
> + }
> +}
> +
> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> {
> int ret;
> @@ -1536,6 +1583,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> /* Initialize various capabilities */
> pci_init_capabilities(dev);
>
> + /* After pcie_cap is assigned and sriov bar is probed */
> + set_pcie_64bit_pref(dev);
> +
> /*
> * Add the device to our list of discovered devices
> * and the bus list for fixup functions, etc.
> --
> 1.8.4.5
>
On Mon, Apr 6, 2015 at 3:06 PM, Bjorn Helgaas <[email protected]> wrote:
> But this is a general change that affects all platforms, and it's late in
> the cycle for something as invasive as this. I'd rather include your patch
> in the v4.1 merge window, and revert d63e2e1f3df9 ("sparc/PCI: Clip bridge
> windows to fit in upstream windows") for v4.0.
How about applying first two in this patchset for v4.0, and leaving
this one for v4.1?
Thanks
Yinghai
On Mon, Apr 06, 2015 at 03:35:40PM -0700, Yinghai Lu wrote:
> On Mon, Apr 6, 2015 at 3:06 PM, Bjorn Helgaas <[email protected]> wrote:
>
> > But this is a general change that affects all platforms, and it's late in
> > the cycle for something as invasive as this. I'd rather include your patch
> > in the v4.1 merge window, and revert d63e2e1f3df9 ("sparc/PCI: Clip bridge
> > windows to fit in upstream windows") for v4.0.
>
> How about applying first two in this patchset for v4.0, and leaving
> this one for v4.1?
For "[PATCH 1/3] PCI: Introduce pci_bus_addr_t", I'm waiting for an updated
version with Kconfig tweaks so we don't break other arches.
For "[PATCH 2/3] sparc/PCI: Add mem64 resource parsing for root bus", I'm
waiting for a version that fixes the other of_bus_pci_get_flags() and
pci_parse_of_flags() implementations at the same time (or an explanation
about why we should fix only the arch/sparc version). I don't want to fix
one place and leave the same bug in other places.
Bjorn
From: Bjorn Helgaas <[email protected]>
Date: Mon, 6 Apr 2015 17:06:38 -0500
> But this is a general change that affects all platforms, and it's late in
> the cycle for something as invasive as this. I'd rather include your patch
> in the v4.1 merge window, and revert d63e2e1f3df9 ("sparc/PCI: Clip bridge
> windows to fit in upstream windows") for v4.0.
I would kindly ask that we not proceed this way and use the change
which implements the fix properly.
On Mon, Apr 6, 2015 at 3:49 PM, Bjorn Helgaas <[email protected]> wrote:
>
> For "[PATCH 1/3] PCI: Introduce pci_bus_addr_t", I'm waiting for an updated
> version with Kconfig tweaks so we don't break other arches.
I was thinking that you will update it manually.
>
> For "[PATCH 2/3] sparc/PCI: Add mem64 resource parsing for root bus", I'm
> waiting for a version that fixes the other of_bus_pci_get_flags() and
> pci_parse_of_flags() implementations at the same time (or an explanation
> about why we should fix only the arch/sparc version). I don't want to fix
> one place and leave the same bug in other places.
I don't even know if other arch like powerpc support 64-bit bus address.
No one from powerpc reported a problem, why should we mess it up now?
I would like to see someone get access those three kinds of machine that support
of to unify of support code.
Thanks
Yinghai
On Mon, Apr 6, 2015 at 8:13 PM, Yinghai Lu <[email protected]> wrote:
> On Mon, Apr 6, 2015 at 3:49 PM, Bjorn Helgaas <[email protected]> wrote:
>>
>> For "[PATCH 1/3] PCI: Introduce pci_bus_addr_t", I'm waiting for an updated
>> version with Kconfig tweaks so we don't break other arches.
>
> I was thinking that you will update it manually.
I asked you for an updated version, incorporating the documentation
updates, to make sure I got everything you intended. But I did go
ahead and do it manually for you.
>> For "[PATCH 2/3] sparc/PCI: Add mem64 resource parsing for root bus", I'm
>> waiting for a version that fixes the other of_bus_pci_get_flags() and
>> pci_parse_of_flags() implementations at the same time (or an explanation
>> about why we should fix only the arch/sparc version). I don't want to fix
>> one place and leave the same bug in other places.
>
> I don't even know if other arch like powerpc support 64-bit bus address.
>
> No one from powerpc reported a problem, why should we mess it up now?
>
> I would like to see someone get access those three kinds of machine that support
> of to unify of support code.
Of course changes there should be tested on all the affected machines.
I opened https://bugzilla.kernel.org/show_bug.cgi?id=96241 and
assigned it to you as a reminder that there is nearly identical code
in several other places that may have the same issue.
I pushed these two changes to for-linus. I'll work on the third
tomorrow. The current changelog is very sparc64-centric, and it needs
to be much more explicit about how the change will affect every arch.
Bjorn
On Mon, Apr 6, 2015 at 8:43 PM, Bjorn Helgaas <[email protected]> wrote:
> On Mon, Apr 6, 2015 at 8:13 PM, Yinghai Lu <[email protected]> wrote:
>> On Mon, Apr 6, 2015 at 3:49 PM, Bjorn Helgaas <[email protected]> wrote:
>>>
>>> For "[PATCH 1/3] PCI: Introduce pci_bus_addr_t", I'm waiting for an updated
>>> version with Kconfig tweaks so we don't break other arches.
>>
>> I was thinking that you will update it manually.
>
> I asked you for an updated version, incorporating the documentation
> updates, to make sure I got everything you intended. But I did go
> ahead and do it manually for you.
Good.
http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=f70899ff889a38f9697d3c153aaacaed25f501c3
Please consider to split that into two patches.
First one include changes in : include/linux/types.h and
Documentation/DMA-API-HOWTO.txt.
and you should be author for that.
>
>>> For "[PATCH 2/3] sparc/PCI: Add mem64 resource parsing for root bus", I'm
>>> waiting for a version that fixes the other of_bus_pci_get_flags() and
>>> pci_parse_of_flags() implementations at the same time (or an explanation
>>> about why we should fix only the arch/sparc version). I don't want to fix
>>> one place and leave the same bug in other places.
>>
>> I don't even know if other arch like powerpc support 64-bit bus address.
>>
>> No one from powerpc reported a problem, why should we mess it up now?
>>
>> I would like to see someone get access those three kinds of machine that support
>> of to unify of support code.
>
> Of course changes there should be tested on all the affected machines.
> I opened https://bugzilla.kernel.org/show_bug.cgi?id=96241 and
> assigned it to you as a reminder that there is nearly identical code
> in several other places that may have the same issue.
ok, will on that to have three patches cover them.
>
> I pushed these two changes to for-linus. I'll work on the third
> tomorrow. The current changelog is very sparc64-centric, and it needs
> to be much more explicit about how the change will affect every arch.
Sure. That will affect all platform.
Thanks
Yinghai
On Tue, Apr 7, 2015 at 12:23 AM, Yinghai Lu <[email protected]> wrote:
> On Mon, Apr 6, 2015 at 8:43 PM, Bjorn Helgaas <[email protected]> wrote:
>> On Mon, Apr 6, 2015 at 8:13 PM, Yinghai Lu <[email protected]> wrote:
>>> On Mon, Apr 6, 2015 at 3:49 PM, Bjorn Helgaas <[email protected]> wrote:
>>>>
>>>> For "[PATCH 1/3] PCI: Introduce pci_bus_addr_t", I'm waiting for an updated
>>>> version with Kconfig tweaks so we don't break other arches.
>>>
>>> I was thinking that you will update it manually.
>>
>> I asked you for an updated version, incorporating the documentation
>> updates, to make sure I got everything you intended. But I did go
>> ahead and do it manually for you.
>
> Good.
>
> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=f70899ff889a38f9697d3c153aaacaed25f501c3
>
> Please consider to split that into two patches.
> First one include changes in : include/linux/types.h and
> Documentation/DMA-API-HOWTO.txt.
> and you should be author for that.
If you want something different, send patches. I don't have time to
try to figure out your intent and do the work for you. That's why I
asked you for an updated series in the first place.
Bjorn
On Mon, Apr 6, 2015 at 7:35 PM, David Miller <[email protected]> wrote:
> From: Bjorn Helgaas <[email protected]>
> Date: Mon, 6 Apr 2015 17:06:38 -0500
>
>> But this is a general change that affects all platforms, and it's late in
>> the cycle for something as invasive as this. I'd rather include your patch
>> in the v4.1 merge window, and revert d63e2e1f3df9 ("sparc/PCI: Clip bridge
>> windows to fit in upstream windows") for v4.0.
>
> I would kindly ask that we not proceed this way and use the change
> which implements the fix properly.
I reworked the changelog and pushed this to my for-linus branch for
v4.0. I'll ask Linus to pull it tomorrow.
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=1a3ec5e7b00dcd9cac24efe3d65bfccf82597ce5
Thanks for your help.
Bjorn
[+cc Ben]
On Tue, Apr 07, 2015 at 11:48:44AM -0500, Bjorn Helgaas wrote:
> On Mon, Apr 6, 2015 at 7:35 PM, David Miller <[email protected]> wrote:
> > From: Bjorn Helgaas <[email protected]>
> > Date: Mon, 6 Apr 2015 17:06:38 -0500
> >
> >> But this is a general change that affects all platforms, and it's late in
> >> the cycle for something as invasive as this. I'd rather include your patch
> >> in the v4.1 merge window, and revert d63e2e1f3df9 ("sparc/PCI: Clip bridge
> >> windows to fit in upstream windows") for v4.0.
> >
> > I would kindly ask that we not proceed this way and use the change
> > which implements the fix properly.
>
> I reworked the changelog and pushed this to my for-linus branch for
> v4.0. I'll ask Linus to pull it tomorrow.
>
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=1a3ec5e7b00dcd9cac24efe3d65bfccf82597ce5
Based on Ben's concerns, I dropped Yinghai's three patches for now:
PCI: Add pci_bus_addr_t
sparc/PCI: Parse 64-bit host bridge windows from OF
PCI: Allow non-prefetchable PCIe BARs in prefetchable windows
and reverted d63e2e1f3df9 ("sparc/PCI: Clip bridge windows to fit in
upstream windows") instead.
I pushed this to my for-linus branch, which I intend to merge for v4.0.
You can browse it here to see if you agree:
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=for-linus
Dave, I know you weren't happy with this proposal the first time around,
but I'm not sure why. If you have a better idea, let me know.
Bjorn
From: Bjorn Helgaas <[email protected]>
Date: Wed, 8 Apr 2015 10:47:59 -0500
> Dave, I know you weren't happy with this proposal the first time around,
> but I'm not sure why. If you have a better idea, let me know.
At this point getting things working again is more important I guess,
so I'm fine with this for now.
On Wed, 2015-04-08 at 10:47 -0500, Bjorn Helgaas wrote:
> Based on Ben's concerns, I dropped Yinghai's three patches for now:
>
> PCI: Add pci_bus_addr_t
> sparc/PCI: Parse 64-bit host bridge windows from OF
> PCI: Allow non-prefetchable PCIe BARs in prefetchable windows
>
> and reverted d63e2e1f3df9 ("sparc/PCI: Clip bridge windows to fit in
> upstream windows") instead.
>
> I pushed this to my for-linus branch, which I intend to merge for v4.0.
> You can browse it here to see if you agree:
Thanks Bjorn. We can fix Yinghai patch for 4.2, it would be indeed handy
even for us to be able to support putting 64-bit NP BARs in prefetch
windows (For some SR-IOV adapters for example) too, but we need to do it
right.
Cheers,
Ben.
On Wed, Apr 8, 2015 at 2:12 PM, Benjamin Herrenschmidt
<[email protected]> wrote:
> Thanks Bjorn. We can fix Yinghai patch for 4.2, it would be indeed handy
> even for us to be able to support putting 64-bit NP BARs in prefetch
> windows (For some SR-IOV adapters for example) too, but we need to do it
> right.
Please check if you are ok with attached.
Yinghai
On Wed, 2015-04-08 at 17:06 -0700, Yinghai Lu wrote:
> On Wed, Apr 8, 2015 at 2:12 PM, Benjamin Herrenschmidt
> <[email protected]> wrote:
>
> > Thanks Bjorn. We can fix Yinghai patch for 4.2, it would be indeed handy
> > even for us to be able to support putting 64-bit NP BARs in prefetch
> > windows (For some SR-IOV adapters for example) too, but we need to do it
> > right.
>
> Please check if you are ok with attached.
I'll let Bjorn be the final judge here but I am not fan of the way you
set/clear/set/clear the IORESOURCE_PREFETCH bit with
pci_set_pref_under_pref(). It's error prone and confusing, the code is
already barely readable as it is ...
I would rather you replace those various masks compares with a helper
that does something like pci_resource_compatible(parent_res, child_res),
which has the logic to test. That or a helper that does something like
pci_calc_compatible_res_flags() which returns a "flags" that has
PREFETCH set, which you use in place of res->flags in the various
allocation path.
As-is, your patch looks like a band-aid and smells like a hack :-)
Ben.
On Wed, Apr 8, 2015 at 8:17 PM, Benjamin Herrenschmidt
<[email protected]> wrote:
> On Wed, 2015-04-08 at 17:06 -0700, Yinghai Lu wrote:
> I'll let Bjorn be the final judge here but I am not fan of the way you
> set/clear/set/clear the IORESOURCE_PREFETCH bit with
> pci_set_pref_under_pref(). It's error prone and confusing, the code is
> already barely readable as it is ...
We only use them to wrapping final pci_claim*, sizing, and assigning.
that is limited places.
>
> I would rather you replace those various masks compares with a helper
> that does something like pci_resource_compatible(parent_res, child_res),
> which has the logic to test. That or a helper that does something like
> pci_calc_compatible_res_flags() which returns a "flags" that has
> PREFETCH set, which you use in place of res->flags in the various
> allocation path.
That way should be more intrusive to current code, as we are using
type_mask checking to share the code among
parent(pref)/child(pref), parent(no-pref)/child(pref), and
parent(no-pref)/child(pref)
Yinghai
On Wed, Apr 8, 2015 at 10:17 PM, Benjamin Herrenschmidt
<[email protected]> wrote:
> On Wed, 2015-04-08 at 17:06 -0700, Yinghai Lu wrote:
>> On Wed, Apr 8, 2015 at 2:12 PM, Benjamin Herrenschmidt
>> <[email protected]> wrote:
>>
>> > Thanks Bjorn. We can fix Yinghai patch for 4.2, it would be indeed handy
>> > even for us to be able to support putting 64-bit NP BARs in prefetch
>> > windows (For some SR-IOV adapters for example) too, but we need to do it
>> > right.
>>
>> Please check if you are ok with attached.
>
> I'll let Bjorn be the final judge here but I am not fan of the way you
> set/clear/set/clear the IORESOURCE_PREFETCH bit with
> pci_set_pref_under_pref(). It's error prone and confusing, the code is
> already barely readable as it is ...
>
> I would rather you replace those various masks compares with a helper
> that does something like pci_resource_compatible(parent_res, child_res),
> which has the logic to test. That or a helper that does something like
> pci_calc_compatible_res_flags() which returns a "flags" that has
> PREFETCH set, which you use in place of res->flags in the various
> allocation path.
I'm not planning to review this until after the merge window opens,
but I took a quick glance, and I agree with Ben. I don't want to add
a new IORESOURCE_ flag. I think a pci_resource_compatible() helper is
a great idea.
I am absolutely not in favor of "minimally intrusive" as a goal.
"Minimally intrusive" sounds good but it is often used to justify
clever hacks which end up being an anti-maintainer strategy in the
long term. "Maximum readability" is what I'm looking for.
Bjorn
On Wed, 2015-04-08 at 23:26 -0500, Bjorn Helgaas wrote:
> I'm not planning to review this until after the merge window opens,
> but I took a quick glance, and I agree with Ben. I don't want to add
> a new IORESOURCE_ flag. I think a pci_resource_compatible() helper is
> a great idea.
So the new resource flag was handy here still regardless of the
implementation choice because otherwise, we have to do the whole tree
walk to check for "PCI Express only path".
*But*, this is a property of the device as a whole, not of the resource,
so we could instead have a pci_dev flag established at probe time that
indicates that the path to a given device is PCIe only.
The cool thing is that can be very easily done by simply propagating
from the parent. If the parent has it and the device is PCIe, then set
it, the only break would happen on PCIe to PCI bridges, which is exactly
what we are after.
That way, you avoid the special resource flag alltogether.
> I am absolutely not in favor of "minimally intrusive" as a goal.
> "Minimally intrusive" sounds good but it is often used to justify
> clever hacks which end up being an anti-maintainer strategy in the
> long term. "Maximum readability" is what I'm looking for.
Yup, I agree. The code is too hard to understand already which makes it
bug prone in the long run.
Cheers,
Ben.
On Wed, 2015-04-08 at 21:11 -0700, Yinghai Lu wrote:
> That way should be more intrusive to current code, as we are using
> type_mask checking to share the code among
> parent(pref)/child(pref), parent(no-pref)/child(pref), and
> parent(no-pref)/child(pref)
That's fine, as long as the helper knows which one is the parent and
which one is the child.
In fact, as "cute" as the mask trick is, I think it would generally make
the code more self explanatory if it explicitly tested for the
combinations that are supported, ie something like
if (parent_is_pref && !child_is_pref && child->pcie_only)
...
else if (!parent_is_pref)
...
etc...
The impact in performance would be in the noise and the logic of the
algorithm a LOT more explicit.
Cheers,
Ben.
On Thu, Apr 9, 2015 at 1:54 AM, Benjamin Herrenschmidt
<[email protected]> wrote:
> On Wed, 2015-04-08 at 23:26 -0500, Bjorn Helgaas wrote:
>> I'm not planning to review this until after the merge window opens,
>> but I took a quick glance, and I agree with Ben. I don't want to add
>> a new IORESOURCE_ flag. I think a pci_resource_compatible() helper is
>> a great idea.
>
> So the new resource flag was handy here still regardless of the
> implementation choice because otherwise, we have to do the whole tree
> walk to check for "PCI Express only path".
>
> *But*, this is a property of the device as a whole, not of the resource,
> so we could instead have a pci_dev flag established at probe time that
> indicates that the path to a given device is PCIe only.
> That way, you avoid the special resource flag alltogether.
in the assign path: pci_bus_alloc_resource() does not take dev pointer.
should we make
int pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
to
int pci_bus_alloc_resource(struct pci_bus *bus, struct pci_dev *dev,
struct resource *res,
?
On Thu, 2015-04-09 at 11:31 -0700, Yinghai Lu wrote:
> On Thu, Apr 9, 2015 at 1:54 AM, Benjamin Herrenschmidt
> <[email protected]> wrote:
> > On Wed, 2015-04-08 at 23:26 -0500, Bjorn Helgaas wrote:
> >> I'm not planning to review this until after the merge window opens,
> >> but I took a quick glance, and I agree with Ben. I don't want to add
> >> a new IORESOURCE_ flag. I think a pci_resource_compatible() helper is
> >> a great idea.
> >
> > So the new resource flag was handy here still regardless of the
> > implementation choice because otherwise, we have to do the whole tree
> > walk to check for "PCI Express only path".
> >
> > *But*, this is a property of the device as a whole, not of the resource,
> > so we could instead have a pci_dev flag established at probe time that
> > indicates that the path to a given device is PCIe only.
>
> > That way, you avoid the special resource flag alltogether.
>
> in the assign path: pci_bus_alloc_resource() does not take dev pointer.
>
> should we make
>
> int pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
>
> to
>
> int pci_bus_alloc_resource(struct pci_bus *bus, struct pci_dev *dev,
> struct resource *res,
Do you need to pass bus if you have dev ?
Do we have any caller that doesn't have dev available ?
Cheers,
Ben.
> ?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Thu, Apr 9, 2015 at 4:31 PM, Benjamin Herrenschmidt
<[email protected]> wrote:
>> should we make
>>
>> int pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
>>
>> to
>>
>> int pci_bus_alloc_resource(struct pci_bus *bus, struct pci_dev *dev,
>> struct resource *res,
>
> Do you need to pass bus if you have dev ?
in following path:
_pci_assign_resource only take dev, and it will go up for parent bus
while ((ret = __pci_assign_resource(bus, dev, resno, size,
min_align, fit))) {
if (!bus->parent || !bus->self->transparent)
break;
bus = bus->parent;
}
and __pci_assign_resource will call pci_bus_alloc_resource.
so it is could go up several levels to use upper bus there.